Skip to content

Add LightMap: entry-less flat open-addressing map primitive (perf toolbox) - #12102

Open
dougqh wants to merge 44 commits into
masterfrom
dougqh/lightmap
Open

Add LightMap: entry-less flat open-addressing map primitive (perf toolbox)#12102
dougqh wants to merge 44 commits into
masterfrom
dougqh/lightmap

Conversation

@dougqh

@dougqh dougqh commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

What Does This Do?

Adds LightMap<K, V>, an entry-less, flat, open-addressed map — an easy-to-use, allocation-light stand-in for a small Map. It deliberately does not implement java.util.Map; the surface is intentionally small (set/get/remove/containsKey/forEach, plus read-only for-each iteration via Iterable<EntryReader>).

Motivation

  • Entry-less design allocates less than a conventional HashMap (no per-entry Node).
  • Dense, cache-friendly storage.
  • AdaptiveSizingHint encourages restricting capacity and appropriate per-site sizing.

Additional Notes

  • Keys and values share a single Object[] spine — keys in [0, numSlots), values in [numSlots, 2·numSlots) — so there is no per-entry object. Power-of-2 sizing, linear probing with two-loop wraparound, tombstone deletion (a dedicated RemovedTombstone sentinel, unmistakable in a heap dump).
  • A single Object[] (rather than two typed K[]/V[] arrays) is deliberate: under erasure typed arrays gain no real type safety, and the single array lets the non-K tombstone live in the array directly and keeps embedding to one field. (Documented in the class.)
  • Read-only for-each iteration. LightMap is Iterable<EntryReader>. The iterator is a single reused flyweight that is both the Iterator and the EntryReader it yields, so a for-each over a concretely-typed map lets escape analysis scalar-replace it — zero allocation (see benchmarks below). It is read-only by design (Iterator.remove() is unsupported); the returned EntryReader is a reused cursor and must not be retained past the next next().
  • The LightMap<K, V> object is the product and stands on its own.
  • A nested EmbeddingSupport exposes the same operations as static functions over a caller-owned array — the opt-in "drop a level for more performance" hatch: a caller can embed the backing Object[] directly in its own fields and skip the wrapper. The benchmarks show this buys back only the wrapper object (~32 B/op) and nothing else, so it is documented as rarely worth reaching for.
  • Per-site sizing: LightMap.createUncappedAdaptiveSizingHint(), LightMap.createCappedAdaptiveSizingHint(maxCapacity), or AdaptiveSizingHint.builder() for an explicit initial capacity. The plain createUncapped()/createCapped(max) factory javadocs steer callers who will hold more than the default seed toward an explicit capacity or a reused hint (a larger map otherwise takes one resize on the way up).

Benchmarks

SingleThreadedMapBenchmark gains LightMap arms across all three scenarios (create / get / iterate), including adaptive-sizing and embedded-spine variants, benchmarked head-to-head against HashMap.

Measured on JDK 17, @Threads(8), -prof gc (throughput is aggregate across 8 threads; allocation is per-op normalized). A 10-entry, String-keyed map; get uses distinct-instance keys so it exercises equals(), not identity. These are indicative single-machine numbers, not a CI gate.

Scenario Arm Throughput (ops/s) Alloc (B/op)
create HashMap ~19.5M* 448
LightMap (uncapped seed) 8.9M 256
LightMap + AdaptiveSizingHint 18.5M 240
LightMap embedded (spine) 8.0M 224
LightMap embedded + hint 19.5M 208
get HashMap 184.8M ≈0
LightMap 178.1M ≈0
LightMap embedded (spine) 190.4M ≈0
iterate HashMap 18.43M ≈0
LightMap 18.40M ≈0
LightMap embedded (spine) 18.42M ≈0

Reading it honestly:

  • create — LightMap's entry-less layout allocates well under HashMap (256 vs 448 B/op, and 208–240 B/op sized/embedded). The plain createUncapped() seed is ~2× slower to build because filling past the small default takes one resize; an AdaptiveSizingHint (or explicit capacity) erases that gap, matching/beating HashMap throughput at roughly half the allocation. This is why the factory javadocs steer toward sizing.
  • get — a wash with HashMap (~178–190M ops/s), all zero-alloc. No lookup win or penalty on a 10-entry map.
  • iterate — identical three ways (~18.4M ops/s), all ~0 B/op. The reused flyweight (and even the embedded iterable() lambda) scalar-replaces under escape analysis, so entry-less iteration matches HashMap's zero-alloc traversal rather than beating it — the value is that being entry-less costs nothing at iteration time.
  • The embedded spine buys back only the wrapper object (~32 B/op on create) and is otherwise a wash — documented as rarely worth reaching for.

* create_hashMap throughput is fork-noisy (±~8.5M across forks); the allocation figure is stable and is the more meaningful comparison here.

LightMapGrowBenchmark separately measures the grow-trigger choice (probe-bound vs load-factor vs full).

Scope

Draft: primitive + tests + benchmarks only. The first real consumer lands in a tick-tock PR behind this one. The full experiment surface is retained as-is (including currently callerless spine methods) so the consumer PR can draw from it without re-plumbing.

Originated from the FastMapBenchmarks experiment. Since the port it has been generalized and hardened on this branch: parameterized to <K, V>, renamed LightStringMapLightMap, the sizing-hint factory surface restructured, a distinct RemovedTombstone sentinel, an identity fast-path before equals() in the probe loops, a fix for a ClassCastException when resizing a non-String-keyed map, and read-only Iterable<EntryReader> iteration.

Test plan

  • ./gradlew :internal-api:compileJava — clean
  • ./gradlew :internal-api:test --tests "datadog.trace.util.LightMapTest"76 tests, all pass
  • JUnit 5 coverage for both the instance API and the EmbeddingSupport spine: grow-and-preserve across resizes (String and non-String keys), equals-fallback lookup, tombstone chains + wraparound probe loops, expand-purges-tombstones, the maxCapacity hard cap + boolean set(), AdaptiveSizingHint seeding/decay/cap clamping, slot math, and for-each iteration (single reused flyweight, skips tombstones/empties, read-only).
  • Regression: growsAndPreservesAllEntriesWithNonStringKeys throws pre-fix (the resize (String) cast) and passes post-fix.

🤖 Generated with Claude Code

dougqh and others added 2 commits July 29, 2026 11:57
LightMap is a flat, entry-less String-keyed map in the flat-collections
family (alongside Hashtable / ConcurrentHashtable / FlatHashtable). Keys
and values live in a single Object[] spine (keys in [0, numSlots), values
in [numSlots, 2*numSlots)), power-of-2 sized, linear-probed with
tombstone deletion.

Following StringIndex, the object stands on its own but exposes a nested
EmbeddingSupport class as an opt-in "drop a level" performance hatch:
static functions over a caller-owned array, so callers can embed the
backing array in their own fields and skip the wrapper. (This is the
reverse of the *Hashtable family, where statics are primary for arity.)

Ported from the FastMapBenchmarks experiment; no behavior change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
JUnit 5 coverage for both surfaces: instance set/get/remove/forEach with
grow-and-preserve across resizes, equals-fallback lookup for non-literal
keys, and remove-then-reinsert; the EmbeddingSupport statics for empty-map
probes, tombstone chains, expand-purges-tombstones, copy independence,
setAll merge/clone, keyAt/valueAt/prevValueAt, findSlot/findInsertionSlot,
insertAt/removeAt/getAndRemoveAt, both forEach variants, and the slot math
(roundUpToPow2, preferredSlot, isRemoved sentinel).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dougqh dougqh added comp: core Tracer core tag: ai generated Largely based on code generated by an AI or LLM tag: no release notes Changes to exclude from release notes type: refactoring labels Jul 29, 2026
The two-pass findSlot ran a ref-equality pre-pass (optimized for interned
keys) that did not terminate at null -- it scanned the whole key array on
every miss -- before an equals pass that did terminate. Dropping the
assumption that keys are interned, the pre-pass is pure cost: String.equals
already short-circuits on `this == other`, so a literal-key hit still
resolves on a pointer compare inside the single equals pass, while a miss
now touches only the probe chain instead of the entire array.

Pure deletion of the pre-pass; the equals loops already were the correct
single terminating probe. No behavior change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@datadog-prod-us1-3

datadog-prod-us1-3 Bot commented Jul 29, 2026

Copy link
Copy Markdown

🎯 Code Coverage (details)
Patch Coverage: 90.94%
Overall Coverage: 57.45% (-0.34%)

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 34741d0 | Docs | Datadog PR Page | Give us feedback!

dougqh and others added 8 commits July 29, 2026 12:25
Frees the LightMap name for a future LightClassMap sibling and makes the
String-key specialization explicit. Pure rename (file, class, constructor,
test); no behavior change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…llability

Front door: reject null values in set() so a null get() unambiguously means
"absent"; add containsKey() for explicit presence probes. Rename key params
literal -> key (interned keys are not assumed) and reframe the class doc.
Add @Nonnull/@nullable across the front door and EmbeddingSupport hatch, and
guard valueAt() against a negative slot index (mirroring prevValueAt()).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add a per-construction-site SizingHint: a caller declares one static final
hint via LightStringMap.sizingHint(), passes it to the constructor, and never
touches it again. The map seeds itself from the hint and tunes it back on its
own -- monotonic-max on grow (with one power-of-two class of headroom) and a
periodic step-down decay so a stale high-water self-corrects.

Also add object-tier growth-at-threshold: the map now tracks its live entry
count and grows once a new key would exceed a 0.75 load factor, instead of
filling to a load factor of 1.0 before doubling -- keeping linear-probe chains
short. The static EmbeddingSupport spine has no live count to consult, so it
keeps its grow-only-when-physically-full behavior; the threshold is purely an
object-tier policy.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… a thin delegate

Replace the object-tier 0.75 load-factor grow threshold with a stateless
maximum-probe-distance trigger in the EmbeddingSupport spine: an insertion that
would land MAX_PROBES (8) or more slots from its home slot forces a resize.

Why max-probes over load factor (measured, LightStringMapGrowBenchmark, -f 5):
a load factor is blind to local clustering -- colliding keys pile into one probe
chain long before global occupancy is high -- so it gives no worst-case bound.
On adversarial (all-colliding) keys a load-factor table reads at 31.9 ns/op,
identical to grow-when-full; the probe bound reads at 6.4 ns/op (~5x). On the
tiny, interned-key consumer profile the trigger is provably inert (a probe can
never travel 8 slots in an 8-slot table), so that case is byte-for-byte
unchanged -- build/get within noise across all three triggers.

Because the trigger is derived entirely from the probe walk, it needs no live
count, so it moves into the static spine and the object collapses to a thin
delegate: set() delegates to EmbeddingSupport.set and only teaches the sizing
hint on a genuine grow. Drops the size field, growThreshold, and
LOAD_FACTOR_RESERVE_SHIFT; size() reverts to the spine's O(n) scan (its original
behavior -- the field existed only to serve the load-factor threshold).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replace growsBeforeReachingFullLoad (asserted the removed 0.75 load-factor
threshold) with two tests for the new behavior:

- tinyMapGrowsOnlyWhenPhysicallyFull: a seed-8 map fills all 8 slots before it
  grows, locking in the guarantee that the trigger is inert on tiny maps (the
  consumer-neutral property).
- clusteredKeysGrowEarlierThanWellSpreadKeys: the same key count, all colliding
  onto one home slot, drives a strictly larger table than a well-spread set --
  proving the probe bound fires on clustering (a load factor would not have),
  while both sets stay fully retrievable.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add LightStringMapGrowBenchmark, the measurement backing the probe-bound trigger
choice. Implements all three candidate triggers (FULL / LOAD_FACTOR / MAX_PROBES)
over the real EmbeddingSupport spine across three regimes (tiny/interned,
medium/well-spread, adversarial/all-colliding). @benchmark methods answer the
hot-path-tax question; a main() prints a timing-free probe-length distribution
and grow count per arm.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dougqh

dougqh commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@datadog-prod-us1-3

datadog-prod-us1-3 Bot commented Jul 29, 2026

Copy link
Copy Markdown

Bits has a CI fix ready

🟢 Investigated · 🟢 Fix prepared · ⚪ Validation skipped · 🟠 Ready

LightMapGrowBenchmark prints its standalone analysis through System.out, which the JMH forbidden-API check rejects. Added a class-level @SuppressForbidden limited to that benchmark reporting code.

Commit fix to this PR


View in Datadog | Reviewed commit 507e49e · Any feedback? Reach out in #deveng-pr-agent

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7b0eba1e13

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread internal-api/src/main/java/datadog/trace/util/LightStringMap.java Outdated
Comment thread internal-api/src/main/java/datadog/trace/util/LightStringMap.java Outdated
Comment thread internal-api/src/main/java/datadog/trace/util/LightStringMap.java Outdated
Comment thread internal-api/src/main/java/datadog/trace/util/LightStringMap.java Outdated
dougqh and others added 4 commits July 29, 2026 14:45
Pairs the probe-bound grow trigger with a hard slot cap so a capped map's
worst-case memory is bounded even under clustering. The cap rides on the
SizingHint via buildSizingHint().maxCapacity(...), so every map at a
construction site shares one bound.

set() now returns boolean: true when the mapping was stored (or overwrote an
existing one), false only when a capped map is physically full at its cap and
the key is genuinely new. The rejection is non-fatal (the map is unchanged) and
uncapped maps always return true, keeping the hot path a thin spine delegate.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Covers: uncapped set always stores; a capped map grows up to the cap then
rejects a new key when physically full (leaving the map unchanged) while still
allowing overwrites; rejection is non-fatal (freeing a slot readmits a key);
maxCapacity rounds up to a power of two; build() rejects initCapacity >
maxCapacity; and the learned seed is clamped to the cap.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds create(), create(int capacity), and create(SizingHint) as the single front
door and makes the constructors private. create() is a new zero-config default
(seeds at DEFAULT_CAPACITY) so call sites no longer name a magic capacity, and
the factory set mirrors the existing sizingHint()/buildSizingHint() static idiom.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dougqh

dougqh commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

@dougqh dougqh changed the title Add LightMap: entry-less flat open-addressing map primitive Add LightMap: entry-less flat open-addressing map primitive (perf toolbox) Jul 29, 2026
prevValueAt was byte-identical to valueAt (same guards, same return) and had no
caller -- a leftover from earlier experimentation. Drop it and its test line.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7fe8e0aa81

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment thread internal-api/src/main/java/datadog/trace/util/LightStringMap.java Outdated
Comment thread internal-api/src/main/java/datadog/trace/util/LightStringMap.java Outdated
dougqh and others added 3 commits July 29, 2026 15:21
The uncapped spine set(int,Object[],...) and the capped object-tier
set(String,V) were the same probe -> overwrite/fill/grow sequence with
different tails. Extract a single EmbeddingSupport.setOrReject core that
takes maxSlots: NO_MAX_SLOTS makes the cap check inert (spine path, never
rejects, result never null), a finite cap lets a would-be grow past the
cap become a non-fatal null rejection (object-tier path). Both set()
methods become thin delegates. No behavior change.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Migration counterpart of the object-tier LightStringMap.set for a map
dropped to the embedded spine: seeds a fresh table from hint.seedSlots()
and teaches the hint back on a genuine grow, mirroring the object tier
so graduating a map to the spine keeps its self-tuning (a strict
superset). The hint's maxCapacity cap does NOT follow down -- an Object[]
return can't signal rejection -- so the spine always stores; taking the
spine means owning your own growth bound.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Covers the four behaviors that make dropping a map to the spine a strict
superset of the object tier: seeds a fresh table from the hint, teaches
the hint (with one class of headroom) on a genuine grow, and leaves the
hint untouched on an in-place overwrite -- plus the deliberate
discontinuity that the hint's maxCapacity cap does NOT follow to the
spine (it grows past the cap and always stores).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dougqh and others added 2 commits July 29, 2026 18:56
…-value guard

Rename the two EmbeddingSupport sentinels so each names its own return
contract, since findSlot and findInsertionSlot live in different numeric
spaces:
  - findSlot follows the String.indexOf idiom -> SLOT_NOT_FOUND (-1) on any
    miss (both null map and absent key; previously the null-map case returned
    Integer.MIN_VALUE while a genuine miss returned -1 -- an inconsistency the
    public NOT_FOUND constant invited callers to trip over).
  - findInsertionSlot follows the Arrays.binarySearch idiom (flip()-encoded
    free slot) -> SLOT_CAPACITY_REACHED when physically full.
Document the flip() encoding by naming the Arrays.binarySearch parallel.

Enforce the no-null-value invariant centrally in setOrReject (the shared
insert core every set path flows through) and in insertAt (a separate spine
write path), instead of only at the object-tier front door.

Fill in the @Nonnull/@nullable annotations across the EmbeddingSupport spine
helpers (findSlot, findInsertionSlot, newMapData, newMapUncheckedInsert,
forEach ctx, str).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Plain int reads/writes are atomic (JLS 17.7), so the racy tuning state can
never expose a half-written value -- "torn" was the wrong hazard to name. The
real races are lost updates and stale reads, both benign (they only mis-size a
future array by a class, self-correcting on the next grow). Reword the class
doc and the seedSlots() inline comment accordingly. Doc-only.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dougqh dougqh changed the title Add LightMap: entry-less flat open-addressing map primitive (perf toolbox) Add LightStringMap: entry-less flat open-addressing map primitive (perf toolbox) Jul 29, 2026
dougqh and others added 8 commits July 30, 2026 08:04
The deletion tombstone was a deliberately un-interned magic String
(NUL bytes). Replace it with a private singleton class RemovedTombstone
so it is unmistakable in a heap dump or debugger and can never collide
with a real key of any type -- it is compared only by identity.

Because the sentinel is no longer a String, every slot read that could
land on a tombstone now reads Object and identity-checks REMOVED before
any String cast (keyAt, containsKey, expandMapData, both forEach spine
helpers, toInternalString, and the object-tier forEach). The str()
cast helper is thus unused and removed, and isRemoved widens from
String to Object (its callers all widen cleanly).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Check curKey == key before falling back to key.equals(curKey) in the
lookup probes (findInsertionSlot already did this). Interned or reused
keys -- the common case for this String-keyed map -- then resolve on a
pointer compare with no virtual equals() call. Per the EqGuardBenchmark
result the guard is a wash when equals() inlines and a win when it does
not; that "does not" case is now reachable because keys erase to Object,
so equals() is not guaranteed monomorphic across callers of the shared
spine. A live key is never the REMOVED sentinel, so the identity hit
needs no tombstone check. This retires the stale "not bothering with
literal checks" / "check whether fast literal search is worth it" notes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The String key specialization only ever bought the interned-literal
reference-equality fast path, which has already been removed. Since keys
and values already erase to Object through the single backing array,
generalizing to <K, V> costs almost nothing and widens the applicable
use cases.

The object-tier facade carries the <K, V> types for front-door safety;
the EmbeddingSupport spine takes Object keys, since it is the
type-agnostic sharp-edges tier. Spine forEach consumers are typed
? super K / ? super V.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Noun-returns-builder naming, consistent with TagMap.ledger and the
future SpanPrototype.builder.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Now that the map is parameterized on key type, the "String" in the name
is misleading. Also documents why the backing store is a single Object[]
rather than two typed arrays.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
AdaptiveSizingHint.builder() returns a nested AdaptiveSizingHint.Builder,
replacing LightMap.adaptiveSizingHintBuilder(). The builder belongs with
the type it builds.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Replaces the no-arg LightMap.adaptiveSizingHint() with two descriptive
factories that mirror the map-side createUncapped / createCapped naming:

  LightMap.createUncappedAdaptiveSizingHint()
  LightMap.createCappedAdaptiveSizingHint(maxCapacity)

Both delegate to AdaptiveSizingHint.builder(), which stays for the case
that also needs an explicit initial capacity. A little verbose, but the
common capped/uncapped choice reads at the call site without a builder.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
expandMapData copied keys back with a leftover (String) cast that the
parameterization to <K, V> missed. newMapUncheckedInsert already takes an
Object key, so the cast was both unnecessary and a latent
ClassCastException for any non-String key type once the table grows.

The existing grow/resize tests all used String keys, so it never
surfaced. Adds growsAndPreservesAllEntriesWithNonStringKeys (Integer
keys, several forced resizes), which throws pre-fix and passes post-fix.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dougqh dougqh changed the title Add LightStringMap: entry-less flat open-addressing map primitive (perf toolbox) Add LightMap: entry-less flat open-addressing map primitive (perf toolbox) Jul 30, 2026
dougqh and others added 3 commits July 30, 2026 10:22
Two coverage-gap tests, no production change:
- noArgCreateUncappedProducesUsableMap exercises the parameterless
  front door, which no other test touched.
- removedTombstoneRendersDistinctlyForHeapInspection pins the deletion
  sentinel's "--REMOVED--" toString(). That method has no code-path
  caller (it exists purely for heap-dump/debugger inspection), so it was
  the one class below the 80% per-class instruction gate; the test both
  lifts it to 100% and locks in the debug rendering.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The second (wraparound) loop in findSlot/containsKey/findInsertionSlot --
the path where a probe runs off the end of the table and resumes at slot
0 -- was untested; it was also where the resize (String)-cast CCE hid.
Add three EmbeddingSupportTests that force wraparound with Integer keys
whose home slot is the last slot: a hit located after wrap, a miss that
terminates at a null in the wraparound loop, and a tombstone reclaimed in
the wraparound loop on insert.

Test-only; lifts EmbeddingSupport branch coverage 78% -> 85%.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dd-octo-sts

dd-octo-sts Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

🟢 Java Benchmark SLOs — All performance SLOs passed

Suite Status
Startup 🟢 pass

SLO thresholds are defined here based on automatically generated metrics. A warning is raised when results are within 5% of the threshold.

PR vs. master results
Scenario Candidate master Δ (95% CI of mean)
startup:insecure-bank:iast:Agent 14.03 s 13.99 s [-0.7%; +1.2%] (no difference)
startup:insecure-bank:tracing:Agent 12.86 s 12.95 s [-1.3%; -0.0%] (maybe better)
startup:petclinic:appsec:Agent 15.87 s 16.84 s [-11.5%; +0.0%] (unstable)
startup:petclinic:iast:Agent 16.97 s 17.02 s [-1.2%; +0.6%] (no difference)
startup:petclinic:profiling:Agent 16.67 s 16.94 s [-2.8%; -0.4%] (maybe better)
startup:petclinic:sca:Agent 16.88 s 16.57 s [+0.7%; +3.0%] (maybe worse)
startup:petclinic:tracing:Agent 16.06 s 16.10 s [-1.3%; +0.8%] (no difference)

Commit: 34741d0f · CI Pipeline · Benchmarking Platform UI


Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion.

dougqh and others added 5 commits July 30, 2026 13:04
LightMap now implements Iterable<EntryReader<K,V>> so it works in a
for-each loop, with the spine exposing the same capability as static
functions over a caller-owned array: EmbeddingSupport.iterator(data) and
iterable(data).

The iterator is a flyweight that is *both* the Iterator and the
EntryReader it yields -- next() advances to the next live slot (skipping
nulls and tombstones) and returns this. There is no per-entry object, so
a for-each loop that does not let the cursor escape scalar-replaces the
whole thing away; its fields are one array reference plus two ints for
exactly that reason. This preserves the entry-less, allocation-light
intent rather than leaving a gap a naive Iterator<Map.Entry> would later
fill with a per-entry allocation.

Read-only by design: Iterator.remove() stays unsupported (the JDK 8
default throws); mutate via remove(Object). The returned EntryReader is a
reused cursor valid only until the next next() -- documented on the
interface so it is not retained or collected.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Covers both tiers: the instance Iterable and the spine
EmbeddingSupport.iterator(data) / iterable(data). Asserts live entries
are visited, tombstones and empty slots are skipped, empty/null data
yields nothing (NoSuchElementException on next()), the spine iterable is
re-iterable, remove() throws UnsupportedOperationException, and -- pinning
the zero-allocation contract -- that next() hands back the one reused
flyweight (the iterator itself) each step.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Adds create_lightMap, get_lightMap, and iterate_lightMap alongside the
HashMap/TagMap/TreeMap arms. iterate_lightMap exercises the entry-less
for-each: the flyweight iterator (Iterator + EntryReader in one object)
stays non-escaping, so escape analysis scalar-replaces it -- measured
~0 B/op under -prof gc, matching iterate_hashMap's zero-alloc traversal
rather than paying to materialize per-entry objects.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Extends the LightMap comparison with:
  - create_lightMap_adaptive: create seeded from a held self-tuning hint,
    isolating resize churn from the small createUncapped() seed.
  - create/get/iterate _embedded (+ create _embedded_adaptive): the raw
    Object[] spine driven via EmbeddingSupport, no wrapper object, to
    measure the LightMap wrapper's own overhead.

Measured (JDK 17, -prof gc, -t 1):
  create: adaptive sizing lifts throughput ~8.9M -> ~18.5M ops/s (wrapper)
    and ~8.0M -> ~19.5M (embedded) -- the uncapped-seed deficit was all
    resize churn. Allocation: 256 (wrapper) / 224 (embedded) uncapped;
    240 / 208 adaptive -- entry-less stays well under HashMap's 448 B/op,
    and the wrapper itself costs ~32 B/op.
  get: all ~0 B/op; embedded ~190M, wrapper ~178M, HashMap ~185M ops/s.
  iterate: all ~0 B/op, ~18.4M ops/s -- the embedded iterable() lambda
    scalar-replaces alongside the flyweight, so even that indirection is
    free under escape analysis.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The plain createUncapped()/createCapped(max) seed is sized for a handful
of entries; a larger map takes one resize on the way up, which roughly
halves build throughput (createUncapped) with no allocation win. Point
callers who will hold more than the default toward an explicit capacity
or a reused AdaptiveSizingHint, and cite SingleThreadedMapBenchmark
(create_lightMap vs create_lightMap_adaptive) as the proof.

Also document that the EmbeddingSupport hatch buys back only the wrapper
object (~32 B/op) and nothing else -- lookup and iteration are identical
-- so it is rarely worth reaching for (the *_embedded benchmark arms).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dougqh

dougqh commented Jul 30, 2026

Copy link
Copy Markdown
Contributor Author

@codex review

All three findInsertionSlot overloads declared a <T> that appears nowhere
in their signature or body -- no caller binds it. Pure leftover cruft;
removing it is behavior-neutral and tidies the spine's public surface.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Another round soon, please!

Reviewed commit: ee09c377fd

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

forbiddenApisJmh flags System.out on the benchmark's main()/reportRow --
but those are the deterministic CLI analysis report (grow frequency and
probe-length distribution), not agent logging. Annotate the two methods
with @SuppressForbidden, matching the check's own guidance.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dougqh
dougqh marked this pull request as ready for review July 30, 2026 18:33
@dougqh
dougqh requested a review from a team as a code owner July 30, 2026 18:33
@dougqh
dougqh requested a review from amarziali July 30, 2026 18:33

@datadog-prod-us1-3 datadog-prod-us1-3 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.

Datadog Autotest: FAIL

The new capacity rounding overflows for a very large capped capacity: createCapped(Integer.MAX_VALUE) silently becomes a one-slot map, so the first insertion succeeds and the second is rejected. Normal growth, collision handling, tombstones, capped maps, and randomized operations behaved correctly; no additional tests recommended because the existing suite already covers those paths and production telemetry is unavailable.

View proposed fix
📊 Validated against 5 scenarios · Open Bits AI session

🤖 Datadog Autotest · Commit 34741d0 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest

}

static int roundUpToPow2(int n) {
return n <= 1 ? 1 : Integer.highestOneBit(n - 1) << 1;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Reject capacities whose rounded slot count overflows

A caller supplying a large configured capacity gets silently incorrect storage limits and data loss through false set() results instead of a clear configuration error.

Assertion details
  • Input: Call LightMap.createCapped(Integer.MAX_VALUE), then insert two distinct keys.
  • Expected: The API should reject an unrepresentable capacity with IllegalArgumentException, or otherwise preserve the requested cap semantics.
  • Actual: roundUpToPow2(Integer.MAX_VALUE) shifts 1 << 30 left into -2147483648. The resulting map accepts the first key using a one-slot allocation and rejects the second key, silently behaving as a one-slot map.

Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest

Suggested change
return n <= 1 ? 1 : Integer.highestOneBit(n - 1) << 1;
if (n >= (1 << 30)) {
throw new IllegalArgumentException("capacity too large: " + n);
}
return n <= 1 ? 1 : Integer.highestOneBit(n - 1) << 1;

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 34741d0fcd

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

*/
@Nonnull
public static AdaptiveSizingHint createCappedAdaptiveSizingHint(int maxCapacity) {
return AdaptiveSizingHint.builder().maxCapacity(maxCapacity).build();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Clamp seed for small capped hints

When maxCapacity rounds below the default seed of 8, this convenience factory keeps the builder's default initCapacity, so createCappedAdaptiveSizingHint(4) throws IllegalArgumentException instead of creating a 4-slot capped hint. That makes the adaptive capped API unusable for the same small caps that createCapped(4) explicitly supports; clamp the seed to the rounded cap or pass an explicit init capacity here.

Useful? React with 👍 / 👎.

Comment on lines +746 to +747
mapData[insertionSlot] = key;
mapData[insertionSlot + numSlots(mapData)] = value;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve probe-bound growth in insertAt

When an embedded caller uses the public findInsertionSlot/insertAt pair, the flipped insertion slot can be at distance >= MAX_PROBES; setOrReject would grow before storing in that case, but this branch writes directly, so repeated inserts through this API can build probe chains the rest of LightMap is designed to avoid. Fresh evidence: this public insertAt path still bypasses setOrReject and stores the key/value directly.

Useful? React with 👍 / 👎.

Comment on lines +681 to +683
if (distance >= MAX_PROBES
&& numSlots < maxSlots
&& (long) numSlots < (long) size(mapData) * MAX_SLOTS_PER_LIVE_ENTRY) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Don't suppress useful probe-bound grows

For maps seeded much larger than their live count, this density guard disables the probe-bound grow even for keys that only collide at the current table size and would spread after doubling; for example, with a 1024-slot seed, keys sharing the low 10 spread bits keep inserting into a single chain past MAX_PROBES until the live count exceeds 128. That breaks the advertised bounded-probe behavior for non-identical hashes and makes sparse, hint-sized maps pay long miss probes that a resize would have avoided.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp: core Tracer core tag: ai generated Largely based on code generated by an AI or LLM tag: no release notes Changes to exclude from release notes type: refactoring

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant