Skip to content

Encapsulate ConcurrentHashtable.SizeManager and relocate createBounded - #12453

Draft
dougqh wants to merge 1 commit into
masterfrom
chore/concurrenthashtable-api-cleanup
Draft

Encapsulate ConcurrentHashtable.SizeManager and relocate createBounded#12453
dougqh wants to merge 1 commit into
masterfrom
chore/concurrenthashtable-api-cleanup

Conversation

@dougqh

@dougqh dougqh commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

What Does This Do

Two small, uncontroversial ConcurrentHashtable API cleanups surfaced during review of #12367 (the first real external consumer of ConcurrentHashtable):

  1. Encapsulate SizeManager. State.sizeManager was a public field, so a caller with a State reference could reach into SizeManager directly instead of going through the State-accepting static helpers (estimateSize, isFull, tryReserveOrEvict, evictOne, evictAll). That's now package-private. It was also missing a lock-free tryReserve(State) wrapper — the one operation an external caller doing custom find/reserve/insert (like LogCollector in Use ConcurrentHashtable for telemetry log #12367) actually needs — so callers no longer have to bypass encapsulation just to call it.
  2. Relocate createBounded. Moved from the nested ConcurrentHashtable.State.createBounded(...) onto ConcurrentHashtable.createBounded(...) directly for a nicer call site. D1.createBounded/D2.createBounded now delegate to it.

Pure refactor — no behavior change. Existing tests updated only to call ConcurrentHashtable.createBounded instead of ConcurrentHashtable.State.createBounded.

Motivation

See the review thread on #12367 (comments from @dougqh):

  • "I think there was an oversight on my part. I didn't really intend to expose anything on SizeManager directly. I guess I should fix that."
  • "I'm wondering if it would look nicer to move createBounded onto ConcurrentHashtable." (bric3 agreed)

These are the least controversial items from that discussion; a couple of other, more design-heavy follow-ups (a lock-free find/matcher primitive avoiding boxed composite keys, and a doc note on the bucketAt/isFull composability hazard) are being tracked separately and are not part of this PR.

Contributor Checklist

  • Format the title according to the contribution guidelines
  • Assign the type: and comp: or inst: labels in addition to any other useful labels
  • Avoid using close, fix, or linking keywords when referencing an issue; use solves instead
  • Update CODEOWNERS on source file addition, migration, or deletion
  • Update public documentation with any new configuration flags or behaviors
  • Once approved, use merge queue to merge the PR

🤖 Generated with Claude Code

State.sizeManager was a public field, letting callers outside this
class reach into SizeManager directly instead of going through the
State-accepting static helpers. Narrow it to package-private and add
the missing tryReserve(State) wrapper so external callers (e.g. an
upcoming ConcurrentHashtable consumer) have a sanctioned lock-free
reservation entry point instead of needing sizeManager exposed.

Also move createBounded from the nested State class onto
ConcurrentHashtable directly for a nicer call site, per review
discussion on PR #12367.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@dougqh dougqh added tag: ai generated Largely based on code generated by an AI or LLM comp: api Tracer public API type: refactoring tag: no release notes Changes to exclude from release notes labels Sep 10, 2026
@datadog-prod-us1-4

This comment has been minimized.

@dd-octo-sts

dd-octo-sts Bot commented Sep 10, 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.78 s 14.64 s [+0.1%; +1.8%] (maybe worse)
startup:insecure-bank:tracing:Agent 13.59 s 13.72 s [-1.7%; -0.2%] (maybe better)
startup:petclinic:appsec:Agent 16.97 s 16.87 s [-0.3%; +1.5%] (no difference)
startup:petclinic:iast:Agent 16.40 s 16.86 s [-7.2%; +1.7%] (no difference)
startup:petclinic:profiling:Agent 16.47 s 16.86 s [-3.4%; -1.3%] (significantly better)
startup:petclinic:sca:Agent 16.87 s 16.59 s [+0.6%; +2.8%] (maybe worse)
startup:petclinic:tracing:Agent 16.05 s 16.01 s [-0.8%; +1.2%] (no difference)

Commit: 9d2f878b · 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.

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

Labels

comp: api Tracer public API 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