Repository navigation
ItemData#clean! is O(attributes^2) in Marshal round-trips (populate_default_values called inside the per-attribute loop) #158
Description
Summary
ItemData#clean! calls populate_default_values inside its per-attribute loop, and Attribute#default_value deep-copies via Marshal unconditionally — including when the attribute has no default at all. Together these make record hydration O(attributes²) in Marshal round-trips.
clean! runs for every row of every query (item_collection.rb calls model_class.new + data.clean! per item), so the cost is paid on every read.
The two lines
lib/aws-record/record/item_data.rb (v2.15.0):
def clean!
@dirty_flags = {}
@model_attributes.attributes.each_key do |name|
populate_default_values # <-- inside the loop
value = get_attribute(name)
@clean_copies[name] = @track_mutations ? _deep_copy(value) : value
end
endpopulate_default_values itself iterates every attribute:
def populate_default_values
@model_attributes.attributes.each do |name, attribute|
next if (default_value = attribute.default_value).nil?
next unless @data[name].nil?
@data[name] = default_value
end
endand lib/aws-record/record/attribute.rb:
def default_value
if _is_lambda?(@default_value_or_lambda)
type_cast(@default_value_or_lambda.call)
else
_deep_copy(@default_value_or_lambda) # Marshal round-trip even when nil
end
end
def _deep_copy(obj)
Marshal.load(Marshal.dump(obj))
endAn attribute declared without default_value: leaves @default_value_or_lambda as nil, and _is_lambda?(nil) is false — so Marshal.load(Marshal.dump(nil)) runs once per attribute, per attribute, per record.
Note the ordering in populate_default_values: attribute.default_value is evaluated before the .nil? guard, so the copy is paid even for attributes that are then skipped.
Net ≈ A² + 3A Marshal dump/load pairs per hydrated record, where A is the attribute count.
Reproduction
Standalone — no Rails, no AWS, no network:
require 'benchmark'
def dc(o) = Marshal.load(Marshal.dump(o))
# current: populate_default_values INSIDE the loop
def current(attrs) = attrs.each_key { attrs.each { |_k, d| dc(d) }; dc(nil) }
# hoisted: called once
def hoisted(attrs) = (attrs.each { |_k, d| dc(d) }; attrs.each_key { dc(nil) })
[59, 66, 100].each do |a|
attrs = (1..a).to_h { |i| ["attr_#{i}", nil] } # realistic: most attrs have no default
cur = Benchmark.realtime { 100.times { current(attrs) } } * 10
hoi = Benchmark.realtime { 100.times { hoisted(attrs) } } * 10
printf("A=%3d current=%7.3f ms/record hoisted=%6.3f ms/record %5.1fx\n", a, cur, hoi, cur / hoi)
endA= 59 current= 5.815 ms/record hoisted= 0.197 ms/record 29.5x
A= 66 current= 7.206 ms/record hoisted= 0.211 ms/record 34.1x
A=100 current= 16.455 ms/record hoisted= 0.324 ms/record 50.8x
Measured on a real application model with 59 attributes (many with mutable Hash/Set defaults, which Marshal far more expensively than nil), hydration went from 43.5 ms to 3.75 ms per record — 11.6× — with the hoist plus an immutable-default short-circuit applied as prepend patches.
Suggested fix
Hoisting populate_default_values out of the loop is behaviour-preserving: it is idempotent with respect to @data (next unless @data[name].nil? makes every pass after the first a no-op), and get_attribute never writes to @data.
def clean!
@dirty_flags = {}
populate_default_values
@model_attributes.attributes.each_key do |name|
value = get_attribute(name)
@clean_copies[name] = @track_mutations ? _deep_copy(value) : value
end
endOne caveat worth documenting either way: the current code re-evaluates a callable default once per attribute per outer iteration, whereas the hoisted version evaluates it once. That only differs for a stateful callable default (one returning nil first and a value later). Pure callable defaults — -> { {} }, -> { [] }, -> { Set.new } — are unaffected.
A second, independent improvement: Attribute#default_value could skip _deep_copy when the stored default is immutable and frozen (nil, true, false, Symbol, and frozen Numeric). That removes the round-trip for every attribute with no default, which is the common case.
Frozen String/Array/Hash must not be short-circuited: Marshal currently returns an unfrozen copy, so callers may legitimately mutate what they receive, and a frozen container can still hold mutable descendants.
Versions
aws-record2.15.0 (code path identical in 2.14.0)- Ruby 3.4.8
Activity
sichanyoo commentedon Aug 6, 2026
Hello, thanks for the GitHub issue! We'll look into this and keep you posted on the work.
sichanyoo commentedon Aug 7, 2026
Just merged optimizations as suggested in the github issue. We'll manually release the gem early next week, thank you again for the detailed report!
jterapin commentedon Aug 10, 2026
Hi @thomaswitt! We released a new version of the gem that contains the fix: https://rubygems.org/gems/aws-record/versions/2.15.1
Let us know if you have any questions.
github-actions commentedon Aug 10, 2026
This issue is now closed. Comments on closed issues are hard for our team to see.
If you need more assistance, please open a new issue that references this one.
thomaswitt commentedon Aug 11, 2026
@jterapin — confirmed fixed in 2.15.1. We've upgraded and deleted both local workarounds (the
ItemData#clean!hoist and the immutable-default fast path), and found no behavioural differences across our model set. Thanks for the fast turnaround on this one.One note purely for the record, not a bug report: upstream's
_immutable?usesvalue.is_a?(Numeric), which is broader than the exact-class check our workaround used (Integer/Float/Rational/BigDecimal). ANumericsubclass default is therefore now returned by identity and shared across every record. No stdlib numeric is mutable, andAttribute#initializetype-casts non-callable defaults anyway, so reaching a difference would take a custom marshaler plus a mutableNumericsubclass — we could not construct a realistic case. Flagging it only because the sharing is per-process and would be hard to diagnose if someone ever did hit it.jterapin commentedon Aug 12, 2026
Thanks for the feedback! Yeah, I agree that Numeric by itself is too broad. Realistically hitting it is very rare, but the cost of tightening is basically zero, so I went ahead and opened a quick PR for it.
jterapin commentedon Aug 17, 2026
New version: https://rubygems.org/gems/aws-record/versions/2.15.2
Feel free let us know if you have any questions. Thanks again!