Skip to content

MOD-16608 Object memory improvement - pack header, u32 hash table, elide table for small objects#20

Merged
AvivDavid23 merged 13 commits into
masterfrom
object_perf
Jul 6, 2026
Merged

MOD-16608 Object memory improvement - pack header, u32 hash table, elide table for small objects#20
AvivDavid23 merged 13 commits into
masterfrom
object_perf

Conversation

@AvivDavid23

@AvivDavid23 AvivDavid23 commented Jul 1, 2026

Copy link
Copy Markdown

Object memory reduction (~20–40% per object container)

  • Pack IObject header 16B → 8B: {len:usize, cap:usize} → single u64 (32b len / 32b cap). Flat −8B per object.
  • Hash-table index usizeu32: table slots hold item indices (≤cap < 2^32), so 4B suffices. Halves table: 10·cap5·cap bytes.
  • Small-object no-table: objects with cap ≤ 8 store no hash table; lookups linear-scan the items array. Drops the table entirely for the common small object.
  • Net: −40% on tiny objects (table elided), −20% on larger ones (table halved), applied per-container down the whole tree.

API — fallible allocation

  • IObject mutators (with_capacity, insert, reserve, entry, …) now return Result<_, IJsonError>, mirroring IArray. Allocation failure and the 32-bit limit are recoverable instead of panicking.
  • Extend/FromIterator/From<HashMap|BTreeMap>TryExtend/TryFromIterator/TryFrom.
  • Moved the shared TryExtend/TryFromIterator/TryCollect traits into new src/convert.rs; pub use re-exports keep existing paths (ijson::array::TryExtend) working.
  • Infallible-context .unwrap()s (Clone, Drop/dealloc, layout, defrag, obj[k]=v) - they preserve the prior panic-on-OOM contract.

Compatibility

  • Breaking: a single IObject is now capped at 2^32−1 (~4.29B) entries (was usize) — 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
IObject memory layout is reworked for smaller containers: the header is a single 8-byte u64 (32-bit len/cap), hash buckets store u32 item indices instead of usize, and objects with cap ≤ 8 skip the hash table and use linear scan. Layout, resize, and Robin-Hood insert/remove paths are updated for both modes.

Breaking API: IObject allocation and mutation (with_capacity, insert, reserve, entry, …) now return Result<_, IJsonError>; Extend/FromIterator/From<HashMap|BTreeMap> become TryExtend/TryFromIterator/TryFrom. Shared fallible iterator traits move to src/convert.rs and are re-exported from the crate root (and from array.rs).

Call-site updates: JSON/CBOR deserialize, serde serialize, macros, and CBOR map building propagate allocation errors instead of ignoring them. IArray internal len/cap stay u32 with casts at public usize boundaries; fuzz_object_ops is added to CI to fuzz insert/remove/get against a HashMap oracle 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.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.

Fix All in Cursor

❌ 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.

Comment thread src/value.rs
Comment thread src/object.rs
Comment thread src/object.rs Outdated
…, 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.
@AvivDavid23 AvivDavid23 changed the title Object perf MOD-16608 Object memory improvement - pack header, u32 hash table, elide table for small objects Jul 2, 2026
Comment thread src/object.rs
Comment thread src/object.rs Outdated
cap > SMALL_OBJECT_THRESHOLD
}

fn hash_capacity(cap: usize) -> usize {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that all sizes and capacities are 32 bits why keep functions with usizes (in args and/or return value) around at all?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will require a bigger change that we might can change in future,, but I'm against since:

  1. In some places we can't change the usize(serde, other std/public interfaces)
  2. Changing to u32 might cause a lot more casting changes in the upstream usage

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe so. But anyway I prefer to do so in a separate PR(which will have more breaking API changes also in array.rs

Comment thread src/object.rs Outdated
…blic API which cinsumers are expecting usize(serde, etc)
@AvivDavid23

Copy link
Copy Markdown
Author

@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)

@galcohen-redislabs galcohen-redislabs left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Beautiful

@AvivDavid23 AvivDavid23 merged commit 711bdce into master Jul 6, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants