Memoize nested attribute classes - #552
Conversation
`NestedAttribute#value` built a fresh subclass, reset its attributes, applied the key transformation and evaluated the block on every object it serialized, so a nested attribute got more expensive the more objects a resource serialized. None of that work depends on the object, so the class is built once and memoized on the attribute. The `key_transformation` setter, which `transform_keys!` calls after class definition, drops the memo so the next serialization picks up the new transformation.
WalkthroughNested attribute resource classes are lazily memoized and invalidated when key transformations change. Tests cover evaluation behavior, while benchmarks compare manual and nested serialization scenarios. ChangesNested attribute memoization
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@benchmark/nested_attributes.rb`:
- Around line 3-4: Update the benchmark resource definitions so each flat case
and its nested counterpart serialize the same output shape, including the
details wrapper and placement of body/comments. Keep the comparison pairs
aligned throughout the benchmark rather than measuring root-level versus nested
representations; update the description only if equivalent shapes cannot be
preserved.
In `@lib/alba/nested_attribute.rb`:
- Around line 21-23: Synchronize key_transformation= and resource_class with the
same lock so updating the transformation and invalidating the cached resource
class occur atomically. Protect lazy resource_class construction as well,
ensuring concurrent first access evaluates the transformation block only once
and never caches a class built from an outdated transformation. Add a concurrent
regression test covering transformation updates during lazy construction.
In `@sig/alba/nested_attribute.rbs`:
- Around line 22-24: Update the RBS declarations for
NestedAttribute#resource_class and `#build_resource_class` by adding private
visibility before both methods, matching their visibility in the Ruby
implementation and preventing typed callers from invoking them publicly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0a55cd4f-be24-45e4-826c-5793e7057451
📒 Files selected for processing (5)
CHANGELOG.mdbenchmark/nested_attributes.rblib/alba/nested_attribute.rbsig/alba/nested_attribute.rbstest/usecases/nested_attribute_test.rb
The benchmark compared a nested attribute against a resource that emitted the same values at the root, so the two sides produced different JSON and part of what it measured was the wrapper hash rather than nesting itself. The baseline now builds the nested hash by hand in a plain attribute, and the script asserts both sides serialize the same string.
Problem
NestedAttribute#valuebuilt a fresh subclass, reset its attributes, applied the key transformation and evaluated the block for every object it serialized. None of that depends on the object, so a nested attribute got more expensive the more objects a resource serialized.Solution
The class is built once and memoized on the attribute. The
key_transformationsetter, whichtransform_keys!calls after class definition, drops the memo so the next serialization picks up the new transformation.Behavioural change
A nested attribute body is evaluated once instead of once per serialized object, so side effects or runtime conditionals inside it no longer run for every object.
Benchmark
benchmark/nested_attributes.rb(added in this PR) serializes 100 posts and compares the same JSON produced with the nested hash built by hand in a plain attribute vs produced bynested_attribute, for a plain attribute and for an association with 20 comments per post. The script asserts both sides serialize the same string. Both runs used it on Ruby 4.0.5 (arm64, YJIT). Manual is the baseline, is unaffected by this PR, and each run carries its own manual figure — a nested figure is only meaningful next to the manual one from the same run.Speed (higher is better):
Allocated memory per serialization (lower is better):
End to end: a nested attribute carrying a plain attribute goes from 1071 i/s to 13 214 i/s (12.3x) and from 369 kB to 81 kB per serialization, one carrying an association from 416 i/s to 1031 i/s (2.5x) and from 1.06 MB to 703 kB.
A nested attribute still costs 1.69x the hand-built hash: a nested resource is instantiated per object and its hash merged, so this removes the cost of building the class, not the cost of the instance.
Tests
New tests cover the nested attribute body being evaluated once however many objects are serialized, and
transform_keys!after a first serialization still taking effect.Summary by CodeRabbit