Uh oh!
There was an error while loading. Please reload this page.
Remove LegacyTagMap; OptimizedTagMap is the sole TagMap implementation - #11678
Conversation
With one implementor, the optimized/legacy split is dead weight. Delete LegacyTagMap and LegacyTagMapFactory; collapse the whole TagMapFactory abstraction (TagMap.create()/create(int)/EMPTY now construct OptimizedTagMap directly via a lazy EmptyHolder that avoids a TagMap<->OptimizedTagMap static-init cycle). OptimizedTagMap is the sole TagMap implementor, satisfying CHA's unique-implementor precondition. Delete the now-trivial TagMapType/TagMapTypePair test enums and de-parameterize their tests to plain @test; TagMapScenario (size-based) stays. Drop stale "legacy and optimized" references from javadoc. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The flag only chose between the optimized and legacy TagMap impls; with Legacy gone it controls nothing. Remove the Config field/getter, the GeneralConfig constant, and the metadata/supported-configurations.json entry. Per the Config team, removing the registry entry is detected automatically at the next release (FPD + public docs get a "Max Version: v1.xx.0" marker), so the hard delete is the correct process. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This comment has been minimized.
This comment has been minimized.
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
| /* | ||
| * Using a class, so class hierarchy analysis kicks in | ||
| * That will allow all of the calls to create methods to be devirtualized without a guard | ||
| */ |
There was a problem hiding this comment.
question: For my own information, do the hierarchy analysis do not work with interfaces ?
There was a problem hiding this comment.
There are two flavors of hierarchy analysis.
There's single unique method which I believe does work for interfaces
-- and allows method call to be devirtualized
And there's single implementor which allows the reference to be refined to the more specific type
-- which in turn, then allows the method call to be devirtualized
And devirtualization can lead to inlining
There's also profile guided devirtualization which would work here, too, but that has a slightly higher execution cost
There was a problem hiding this comment.
But then wouldn't that analysis also work if the factory was an interface ?
And there's single implementor which allows the reference to be refined to the more specific type
-- which in turn, then allows the method call to be devirtualized
| "DD_OPTIMIZED_MAP_ENABLED": [ | ||
| { | ||
| "version": "A", | ||
| "type": "boolean", | ||
| "default": "false", | ||
| "aliases": [] | ||
| } | ||
| ], |
There was a problem hiding this comment.
question: Might need some updated on the configuration portal
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Sarah Chen <sarah.chen@datadoghq.com>
/merge |
View all feedbacks in Devflow UI.
The expected merge time in
|
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
What This Does
OptimizedTagMaphas been the default for several months (theoptimized.map.enabledflag defaulted totrue). With Legacy effectively dead, this removes it soOptimizedTagMapis the soleTagMapimplementor — satisfying CHA's unique-implementor precondition (a prerequisite for upcoming read-through / dense-store work).Motivation
The removal of
LegacyTagMapmakes future changes toTagMapeasier.Possible future work includes...
Changes
LegacyTagMapand the wholeTagMapFactoryabstraction (the abstract class + its loneOptimizedTagMapFactory);TagMap.create()/create(int)/EMPTYnow constructOptimizedTagMapdirectly.TagMapType/TagMapTypePairtest enums and de-parameterize their tests to plain@Test.TagMapScenario(parameterizes by size) is kept.optimized.map.enabledflag —Configfield/getter,GeneralConfigconstant, and themetadata/supported-configurations.jsonentry. Per the Config team, removing the registry entry is detected automatically at the next release (FPD + public docs get aMax Version: v1.xx.0marker).Notable
TagMap(interface with default methods) ↔OptimizedTagMapcould leaveTagMap.EMPTYnull whenOptimizedTagMapinitialized first. A lazyEmptyHolderbuilds the empty singleton via the constructor instead of reading a mid-<clinit>static.isOptimized()is intentionally left in place — it now constant-folds totrueand is DCE'd at its call site (DDSpanContext); simplifying it away is a trivial follow-up.Validation
internal-apiTagMap suite green (TagMapTest,TagMapLedgerTest,TagMapFuzzTest);internal-api/dd-trace-api/dd-trace-corecompile clean.dd-trace-coreintegration run before marking ready.Benchmark — overhead-neutral (parity)
TagMapAccessBenchmark(threading-correct,@Threads(8)) run on the pre-kill tree vs. this branch — the two differ by exactly these two commits (verified byte-identical pre-image). Throughput, 1 fork, 3 warmup + 5 measurement iters:insertgetObjectgetEntryAll deltas are within run-to-run error (single fork) → no measurable change, as expected for a dead-code removal.
🤖 Generated with Claude Code