Hi, Johannes! > Peak memory allocated inside toast_flatten_tuple(): > > cols plain stored master patched cut > ---- -------- --------- ---------- ---------- ------ > 1 96 MB 28 MB 57 MB 57 MB 0.0% > 2 96 MB 28 MB 57 MB 43 MB 25.0% > 4 96 MB 28 MB 57 MB 36 MB 37.5% > 8 96 MB 28 MB 57 MB 32 MB 43.7% > 8 896 MB 265 MB 530 MB 298 MB 43.7% > > That is your claim confirmed, to three decimals: master is exactly 2x the > tuple in all five cases, and v1 is exactly 1x plus the one detoasted value > it is holding. > > Whole-backend peak RSS: > > cols plain stored master patched cut > ---- -------- --------- ---------- ---------- ------ > 1 96 MB 28 MB 89 MB 88 MB 0.4% > 2 96 MB 28 MB 88 MB 83 MB 6.4% > 4 96 MB 28 MB 89 MB 75 MB 15.0% > 8 96 MB 28 MB 88 MB 72 MB 18.8% > 8 896 MB 265 MB 803 MB 580 MB 27.8% > > One small thing on the code, since I was reading it anyway. > > toast_flatten_tuple() now open-codes what heap_form_tuple() does: the > hasnull scan, len and hoff, heap_compute_data_size(), the palloc0 and the > HeapTupleHeaderSet* calls. It has to, because it needs the allocation and > the fill separated. > > You already split fill_val() and heap_fill_tuple() into _attr variants with > thin wrappers. The same move one level up would avoid the duplication > in toast_flatten_tuple(): > heap_form_tuple() becomes a wrapper around heap_form_tuple_attr(), which > takes skip_copy and attr_data_out. toast_flatten_tuple() could then call > that and drop the block entirely. > > Worth it for the single source of truth, I think.
Thank you very much for your independent verification! I also split heap_form_tuple_attr encapsulation to avoid code duplication, as you suggested. See attached patch v2. Kind regards, Pavel Borisov Supabase
v2-0001-Halve-peak-memory-allocation-for-toast_flatten_tu.patch
Description: Binary data
