ICU-23321 ResourceCache: replace synchronized trie with ConcurrentHashMap

[jabagawee]

Replace the custom synchronized trie in ICUResourceBundleReader.ResourceCache with ConcurrentHashMap, eliminating a heavily contended lock on the resource loading path. This is split out from the batch 2 PR (forthcoming) because it replaces a data structure rather than just changing a synchronization idiom, and warrants separate review.

Background

ResourceCache.get() is synchronized and is called on every resource bundle access — getString(), getStringV2(), getAlias(), getArray(), getTable(). On a multi-threaded server, this monitor serializes all resource lookups across all threads.

The custom trie was introduced in ICU 54.1 (June 2014, ICU-10932, b3a6ea1) as a memory optimization for Android. It uses int[]/Object[] arrays to avoid Integer autoboxing and HashMap.Node per-entry overhead. The JIRA discussion focuses on memory optimization; the choice of synchronized vs alternatives is not discussed.

Why the trie loses on memory

The trie's per-entry savings (no autoboxing, no Node objects) are real but small. What kills it is sparse power-of-2 array allocations. Measured via reflection across 44 loaded caches (avg 221 entries each):

Metric	Trie	ConcurrentHashMap
Memory per cache (221 entries)	39 KB	13 KB
Slot utilization	5.2%	~75%

The trie allocates a 128-slot root and ~65 child Levels of 64 slots each, but only 221 slots contain data. 94.8% of allocated slots are empty.

Benchmark results

Full-stack throughput (NumberFormat.getInstance(), which exercises ULocale.getDefault → SimpleCache → ResourceCache):

Threads	synchronized trie	ConcurrentHashMap
1	543 ops/ms	505 ops/ms
32	2,074 ops/ms	4,051 ops/ms

ReadWriteLock was also benchmarked as a middle-ground alternative. It underperforms both — readLock() still does a CAS on a shared state word, making read-read contention on that counter the new bottleneck.

Benchmark code: https://gist.github.com/jabagawee/54710d21ca98c1e6a6d307b5229bbd21

Known tradeoff: autoboxing

ConcurrentHashMap<Integer, Object> autoboxes int keys on every get(). Resource offsets are typically outside the Integer cache range (-128..127), so each lookup allocates a short-lived Integer. Accepted because:

1. Lock contention savings dominate (2x throughput at 32 threads)
2. JIT can optimize escape-analyzed Integer allocations
3. A primitive-keyed map (e.g., Eclipse Collections IntObjectHashMap) would require a new dependency
4. The old trie's memory waste (39 KB vs 13 KB) was worse than the autoboxing overhead

Changes

• Replace ResourceCache internals with ConcurrentHashMap<Integer, Object>
• get() becomes lock-free
• putIfAbsent() uses CHM's compute() to atomically handle cleared SoftReferences (plain putIfAbsent() cannot replace a non-null but dead SoftReference entry)
• Remove synchronized from both methods
• Remove the Level/keys/values trie structure (~170 lines deleted)
• Add ResourceCacheConcurrencyTest — concurrent resource bundle lookups from 16 threads, gated on -DICU.exhaustive=5

References

• Java Concurrency in Practice (Goetz et al.), Section 5.2.1 "ConcurrentHashMap"
• Effective Java (Bloch, 3rd ed.), Item 81 "Prefer concurrency utilities to wait and notify"

Checklist

• Required: Issue filed: ICU-23321
• Required: The PR title must be prefixed with a JIRA Issue number. Example: "ICU-NNNNN Fix xyz"
• Required: Each commit message must be prefixed with a JIRA Issue number. Example: "ICU-NNNNN Fix xyz"
• Issue accepted (done by Technical Committee after discussion)
• Tests included, if applicable
• API docs and/or User Guide docs changed or added, if applicable
• Approver: Feel free to merge on my behalf

[markusicu]

That guy who wrote b3a6ea1 thought he made some clever changes... :-/

[jabagawee]

Yeah I anticipated that feedback, which is why I split it out into its own PR. I'm willing to keep the original implementation?

[jabagawee]

I can still make some improvements to the lock contention if we stick to the trie data structure, so I'd just revive another branch of mine that tinkered with the existing code without changing the implementation wholesale.

[markusicu]

Yeah I anticipated that feedback, which is why I split it out into its own PR. I'm willing to keep the original implementation?

Sorry for my confusing comment. You clearly understand Java multi-threaded performance better than me (that guy...), and have done much more thorough testing. I will review this PR, and I have asked the team for a second pair of eyes.

[markusicu]

This looks very good to me!

PR description:

putIfAbsent() uses CHM's atomic putIfAbsent()

... but it's using CHM.compute(). Please fix.

Anyone else want to chime in? @mihnita @gvictor @vladcioricagoogle ?

[markusicu]

lgtm, tnx!

let's wait a day or so to give others a chance to chime in.

[mihnita]

Thank you!

[markusicu]

Let's do it.
@jabagawee do you want to do the squashing honors?

[jira-pull-request-webhook]

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

[jabagawee]

done!