Unify Hashtable static API with ConcurrentHashtable; deprecate Support - #12101
Unify Hashtable static API with ConcurrentHashtable; deprecate Support#12101dougqh wants to merge 3 commits into
Conversation
Move the static building blocks off the nested Support class onto Hashtable itself, mirroring ConcurrentHashtable's flat layout, and add createFixedBuckets(Class, int) factories on Hashtable/D1/D2 for family symmetry. Support becomes a thin @deprecated facade delegating to the new statics (retaining the scaled create(int, float)/MAX_RATIO helpers, which have no blessed equivalent), so client-side-statistics callers keep compiling untouched. Rename the context type parameter <T> -> <C> on the context-passing forEach overloads, and add D2.Entry.key1()/key2() accessors to match D1/the concurrent variant. No behavior change; pure API relocation + deprecation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Point the tests at the relocated static building blocks on Hashtable (createFixedBuckets, sizeFor, bucketIndex, clear, insertHeadEntry, and the iterator factories) instead of the now-deprecated Support facade. Keep a small DeprecatedSupportTests group covering the deprecated-only scaled create(int, float) + MAX_RATIO, which have no blessed equivalent and remain in use by client-side statistics. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
dougqh
added
comp: core
|
@codex review |
|
🎯 Code Coverage (details) 🔗 Commit SHA: 8c1b05d | Docs | Datadog PR Page | Give us feedback! |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e4643cba67
ℹ️ 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".
| */ | ||
| @Deprecated | ||
| public static final class Support { | ||
| private Support() {} |
There was a problem hiding this comment.
Because Support was a public nested class with an implicit public no-arg constructor, making the constructor private breaks source and binary compatibility for any existing consumer that instantiated the facade, even if only as a namespace: recompilation now fails, and already-compiled bytecode can hit an access error when loading against this version. Since this change is explicitly keeping Support as a deprecated compatibility facade, leave a deprecated public no-op constructor (or omit the explicit constructor) until the facade is actually removed.
Useful? React with 👍 / 👎.
| * size {@code n} should pass {@code create(n, MAX_RATIO)} to leave ~25% headroom in the array. | ||
| */ | ||
| public static final float MAX_RATIO = 4.0f / 3.0f; | ||
| @Deprecated public static final float MAX_RATIO = 4.0f / 3.0f; |
There was a problem hiding this comment.
Right next to the retained MAX_RATIO, the deprecated facade no longer exposes the package-private MAX_BUCKETS constant that Support previously had. Any in-package consumer (including downstream tests or custom table helpers compiled in datadog.trace.util) that sizes or bounds-checks against Hashtable.Support.MAX_BUCKETS now fails to recompile even though the facade is advertised as source-compatible; keep a deprecated Support.MAX_BUCKETS alias to the outer constant.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| /** The first key part this entry was created with. */ | ||
| public K1 key1() { |
There was a problem hiding this comment.
Adding key1()/key2() to the subclassable D2.Entry base class can break existing custom D2 entries that already used those natural helper names with weaker visibility (for example private or package-private accessors): those subclasses compiled against the old API but now fail to recompile because the methods are treated as invalid overrides. If preserving source compatibility for this internal API matters, consider less collision-prone names or another migration path before adding these accessors.
Useful? React with 👍 / 👎.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
What Does This Do?
Unifies the
Hashtablestatic API withConcurrentHashtable's flat layout, ahead of treating the two as one family (see #11675).Supportclass ontoHashtableitself —sizeFor,bucketIndex,bucket,insertHeadEntry,clear,forEach, and the iterator factories (bucketIterator,mutatingBucketIterator,mutatingTableIterator) — mirroringConcurrentHashtable's "static functions over a caller-owned array" shape.createFixedBuckets(Class<TEntry> entryClass, int capacity)factories onHashtable,D1, andD2for family symmetry withConcurrentHashtable/FlatHashtable. OnHashtableit returns a concreteHashtable.Entry[](chain heads stored at the base type — the array is a heterogeneousEntry[], not a reflectively-allocatedTEntry[], soentryClassanchors inference/call-shape but is not consumed to allocate).D1/D2.createFixedBucketsreturn aD1/D2instance.D2.Entry.key1()/key2()accessors to matchD1and the concurrent variant.<T>→<C>on the context-passingforEachoverloads (consistent with the naming applied in Add ConcurrentHashtable (perf toolbox) #11675).Motivation
HashtableandConcurrentHashtableare meant to read as one family — same call shapes, same building-block spine — so a caller can move between single-threaded and concurrent variants without relearning the surface. Today the building blocks live on a nestedSupportclass onHashtablebut flat onConcurrentHashtable. This aligns them.Supportis deprecated, not removedSupportbecomes a thin@Deprecatedfacade that delegates to the new statics, retaining the scaledcreate(int, float)factory andMAX_RATIO(which have no blessed equivalent — size for load-factor headroom explicitly at the call site instead). Client-side statistics (AggregateTable,AggregateEntry,CardinalityLimitReporter) keep compiling untouched — deprecation is warning-only, so this PR introduces zero churn there. Migrating those consumers offSupportis a deliberate follow-up.No behavior change
Pure API relocation + deprecation. Bodies are unchanged;
Supportmembers now delegate.Test plan
./gradlew :internal-api:compileJava :dd-trace-core:compileJava— client-side-stats consumers compile untouched (deprecation warnings only)./gradlew :internal-api:test --tests "datadog.trace.util.Hashtable*"— all passHashtableTestmigrated to the blessed API (separate commit), with a smallDeprecatedSupportTestsgroup retaining coverage of the deprecated-only scaledcreate+MAX_RATIO🤖 Generated with Claude Code