MOD-16608 Object memory improvement - pack header, u32 hash table, elide table for small objects#20
Conversation
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1fface9. Configure here.
…, propagate ijson! insert errors - From<HashMap|BTreeMap|Vec|&[..]> for IValue now panics on allocation failure instead of silently yielding IValue::NULL. Split the shared conversion macro: floats keep null-degradation (non-finite has no JSON representation); containers use a panicking variant, restoring the panic-on-OOM contract they had before the fallible API. TryFrom on the concrete container type remains the recoverable path. - ijson! object arms now .unwrap() insert() instead of `let _ =`, matching the array arms — allocation failure panics rather than building a partial object.
…iner OOM, propagate ijson! insert errors" This reverts commit 899c172.
| cap > SMALL_OBJECT_THRESHOLD | ||
| } | ||
|
|
||
| fn hash_capacity(cap: usize) -> usize { |
There was a problem hiding this comment.
Now that all sizes and capacities are 32 bits why keep functions with usizes (in args and/or return value) around at all?
There was a problem hiding this comment.
This will require a bigger change that we might can change in future,, but I'm against since:
- In some places we can't change the usize(serde, other std/public interfaces)
- Changing to u32 might cause a lot more casting changes in the upstream usage
There was a problem hiding this comment.
Ideally the castings are only done at the edges (e.g., when you must call a function outside the scope of our code, like allocations, etc). Other places should stick to u32 - it will make the code less confusing and will benefit from Rust's nazi compiler's type checks.
There was a problem hiding this comment.
Maybe so. But anyway I prefer to do so in a separate PR(which will have more breaking API changes also in array.rs
…blic API which cinsumers are expecting usize(serde, etc)
|
@galcohen-redislabs check last commit, changed to u32 in most cases(apart for some public APIs whihc are called from serde/etc (to handle the casting in the array/object side) |

Object memory reduction (~20–40% per object container)
IObjectheader 16B → 8B:{len:usize, cap:usize}→ singleu64(32b len / 32b cap). Flat −8B per object.usize→u32: table slots hold item indices (≤cap < 2^32), so 4B suffices. Halves table:10·cap→5·capbytes.cap ≤ 8store no hash table; lookups linear-scan the items array. Drops the table entirely for the common small object.API — fallible allocation
IObjectmutators (with_capacity,insert,reserve,entry, …) now returnResult<_, IJsonError>, mirroringIArray. Allocation failure and the 32-bit limit are recoverable instead of panicking.Extend/FromIterator/From<HashMap|BTreeMap>→TryExtend/TryFromIterator/TryFrom.TryExtend/TryFromIterator/TryCollecttraits into newsrc/convert.rs;pub usere-exports keep existing paths (ijson::array::TryExtend) working..unwrap()s (Clone, Drop/dealloc, layout, defrag,obj[k]=v) - they preserve the prior panic-on-OOM contract.Compatibility
IObjectis now capped at 2^32−1 (~4.29B) entries (wasusize) — the tradeoff for the packed 8B header + halved table.Note
High Risk
Large breaking API surface plus unsafe object hash-table and layout changes; incorrect small-object vs table-mode logic could corrupt maps or lose keys.
Overview
IObjectmemory layout is reworked for smaller containers: the header is a single 8-byteu64(32-bit len/cap), hash buckets storeu32item indices instead ofusize, and objects withcap ≤ 8skip the hash table and use linear scan. Layout, resize, and Robin-Hood insert/remove paths are updated for both modes.Breaking API:
IObjectallocation and mutation (with_capacity,insert,reserve,entry, …) now returnResult<_, IJsonError>;Extend/FromIterator/From<HashMap|BTreeMap>becomeTryExtend/TryFromIterator/TryFrom. Shared fallible iterator traits move tosrc/convert.rsand are re-exported from the crate root (and fromarray.rs).Call-site updates: JSON/CBOR deserialize, serde serialize, macros, and CBOR map building propagate allocation errors instead of ignoring them.
IArrayinternal len/cap stayu32with casts at publicusizeboundaries;fuzz_object_opsis added to CI to fuzz insert/remove/get against aHashMaporacle across small/large table behavior.Reviewed by Cursor Bugbot for commit 23c23af. Bugbot is set up for automated code reviews on this repo. Configure here.