Skip to content

Commit daa3f49

Browse files
jabagaweemarkusicu
authored andcommitted
ICU-23321 ResourceCache: replace synchronized trie with ConcurrentHashMap
Replace the custom synchronized trie in ICUResourceBundleReader.ResourceCache with ConcurrentHashMap, eliminating a heavily contended lock on the resource loading path. ResourceCache.get() was synchronized and called on every resource bundle access (getString, getAlias, getArray, getTable). The custom trie (ICU-10932, 2014) optimized memory via int[]/Object[] arrays to avoid Integer autoboxing, but its sparse power-of-2 allocations waste 94.8% of slots (39KB vs 13KB per cache with ConcurrentHashMap). Benchmark (NumberFormat.getInstance full stack): 1 thread: 543 -> 505 ops/ms (slight regression from autoboxing) 32 threads: 2,074 -> 4,051 ops/ms (2x improvement from lock elimination) Uses ConcurrentHashMap.compute() in the put path to atomically handle cleared SoftReferences (plain putIfAbsent cannot replace a non-null but dead entry).
1 parent be00b6b commit daa3f49

File tree

3 files changed

+184
-211
lines changed

3 files changed

+184
-211
lines changed

icu4j/main/core/src/main/java/com/ibm/icu/impl/ICUResourceBundleReader.java

Lines changed: 49 additions & 211 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
import java.nio.CharBuffer;
2222
import java.nio.IntBuffer;
2323
import java.nio.charset.StandardCharsets;
24-
import java.util.Arrays;
24+
import java.util.concurrent.ConcurrentHashMap;
2525

2626
/**
2727
* This class reads the *.res resource bundle format.
@@ -139,7 +139,7 @@ public boolean isDataVersionAcceptable(byte formatVersion[]) {
139139

140140
private ResourceCache resourceCache;
141141

142-
private static ReaderCache CACHE = new ReaderCache();
142+
private static final ReaderCache CACHE = new ReaderCache();
143143
private static final ICUResourceBundleReader NULL_READER = new ICUResourceBundleReader();
144144

145145
private static class ReaderCacheKey {
@@ -323,7 +323,7 @@ private void init(ByteBuffer inBytes) throws IOException {
323323
}
324324

325325
if (!isPoolBundle || b16BitUnits.length() > 1) {
326-
resourceCache = new ResourceCache(maxOffset);
326+
resourceCache = new ResourceCache();
327327
}
328328

329329
// Reset the position for future .asCharBuffer() etc.
@@ -1167,232 +1167,70 @@ int getContainerResource(ICUResourceBundleReader reader, int index) {
11671167
* mapped. Integers need not and should not be cached. Multiple .res items may share resource
11681168
* offsets (genrb eliminates some duplicates).
11691169
*
1170-
* <p>This cache uses int[] and Object[] arrays to minimize object creation and avoid
1171-
* auto-boxing.
1172-
*
11731170
* <p>Large resource objects are usually stored in SoftReferences.
11741171
*
1175-
* <p>For few resources, a small table is used with binary search. When more resources are
1176-
* cached, then the data structure changes to be faster but also use more memory.
1172+
* <p>This replaces the previous custom trie structure (ICU-10932, 2014) with ConcurrentHashMap
1173+
* for lock-free concurrent reads. Benchmarking shows CHM uses ~3x less memory than the trie at
1174+
* typical ICU bundle sizes (~221 entries/cache) due to the trie's sparse power-of-2 Level
1175+
* arrays (5% utilization), while providing ~2x better throughput at 32 threads by eliminating
1176+
* the synchronized bottleneck on every resource lookup.
11771177
*/
11781178
private static final class ResourceCache {
1179-
// Number of items to be stored in a simple array with binary search and insertion sort.
1180-
private static final int SIMPLE_LENGTH = 32;
1181-
1182-
// When more than SIMPLE_LENGTH items are cached,
1183-
// then switch to a trie-like tree of levels with different array lengths.
1184-
private static final int ROOT_BITS = 7;
1185-
private static final int NEXT_BITS = 6;
1186-
1187-
// Simple table, used when length >= 0.
1188-
private int[] keys = new int[SIMPLE_LENGTH];
1189-
private Object[] values = new Object[SIMPLE_LENGTH];
1190-
private int length;
1191-
1192-
// Trie-like tree of levels, used when length < 0.
1193-
private int maxOffsetBits;
1194-
1195-
/** Number of bits in each level, each stored in a nibble. */
1196-
private int levelBitsList;
1197-
1198-
private Level rootLevel;
1179+
private final ConcurrentHashMap<Integer, Object> map;
11991180

12001181
private static boolean storeDirectly(int size) {
12011182
return size < LARGE_SIZE || CacheValue.futureInstancesWillBeStrong();
12021183
}
12031184

1204-
@SuppressWarnings("unchecked")
1205-
private static final Object putIfCleared(
1206-
Object[] values, int index, Object item, int size) {
1207-
Object value = values[index];
1208-
if (!(value instanceof SoftReference)) {
1209-
// The caller should be consistent for each resource,
1210-
// that is, create equivalent objects of equal size every time,
1211-
// but the CacheValue "strength" may change over time.
1212-
// assert size < LARGE_SIZE;
1213-
return value;
1214-
}
1215-
assert size >= LARGE_SIZE;
1216-
value = ((SoftReference<Object>) value).get();
1217-
if (value != null) {
1218-
return value;
1219-
}
1220-
values[index] =
1221-
CacheValue.futureInstancesWillBeStrong() ? item : new SoftReference<>(item);
1222-
return item;
1223-
}
1224-
1225-
private static final class Level {
1226-
int levelBitsList;
1227-
int shift;
1228-
int mask;
1229-
int[] keys;
1230-
Object[] values;
1231-
1232-
Level(int levelBitsList, int shift) {
1233-
this.levelBitsList = levelBitsList;
1234-
this.shift = shift;
1235-
int bits = levelBitsList & 0xf;
1236-
assert bits != 0;
1237-
int length = 1 << bits;
1238-
mask = length - 1;
1239-
keys = new int[length];
1240-
values = new Object[length];
1241-
}
1242-
1243-
Object get(int key) {
1244-
int index = (key >> shift) & mask;
1245-
int k = keys[index];
1246-
if (k == key) {
1247-
return values[index];
1248-
}
1249-
if (k == 0) {
1250-
Level level = (Level) values[index];
1251-
if (level != null) {
1252-
return level.get(key);
1253-
}
1254-
}
1255-
return null;
1256-
}
1257-
1258-
Object putIfAbsent(int key, Object item, int size) {
1259-
int index = (key >> shift) & mask;
1260-
int k = keys[index];
1261-
if (k == key) {
1262-
return putIfCleared(values, index, item, size);
1263-
}
1264-
if (k == 0) {
1265-
Level level = (Level) values[index];
1266-
if (level != null) {
1267-
return level.putIfAbsent(key, item, size);
1268-
}
1269-
keys[index] = key;
1270-
values[index] = storeDirectly(size) ? item : new SoftReference<>(item);
1271-
return item;
1272-
}
1273-
// Collision: Add a child level, move the old item there,
1274-
// and then insert the current item.
1275-
Level level = new Level(levelBitsList >> 4, shift + (levelBitsList & 0xf));
1276-
int i = (k >> level.shift) & level.mask;
1277-
level.keys[i] = k;
1278-
level.values[i] = values[index];
1279-
keys[index] = 0;
1280-
values[index] = level;
1281-
return level.putIfAbsent(key, item, size);
1282-
}
1283-
}
1284-
1285-
ResourceCache(int maxOffset) {
1286-
assert maxOffset != 0;
1287-
maxOffsetBits = 28;
1288-
while (maxOffset <= 0x7ffffff) {
1289-
maxOffset <<= 1;
1290-
--maxOffsetBits;
1291-
}
1292-
int keyBits = maxOffsetBits + 2; // +2 for mini type: at most 30 bits used in a key
1293-
// Precompute for each level the number of bits it handles.
1294-
if (keyBits <= ROOT_BITS) {
1295-
levelBitsList = keyBits;
1296-
} else if (keyBits < (ROOT_BITS + 3)) {
1297-
levelBitsList = 0x30 | (keyBits - 3);
1298-
} else {
1299-
levelBitsList = ROOT_BITS;
1300-
keyBits -= ROOT_BITS;
1301-
int shift = 4;
1302-
for (; ; ) {
1303-
if (keyBits <= NEXT_BITS) {
1304-
levelBitsList |= keyBits << shift;
1305-
break;
1306-
} else if (keyBits < (NEXT_BITS + 3)) {
1307-
levelBitsList |= (0x30 | (keyBits - 3)) << shift;
1308-
break;
1309-
} else {
1310-
levelBitsList |= NEXT_BITS << shift;
1311-
keyBits -= NEXT_BITS;
1312-
shift += 4;
1313-
}
1314-
}
1315-
}
1316-
}
1317-
1318-
/**
1319-
* Turns a resource integer (with unused bits in the middle) into a key with fewer bits (at
1320-
* most keyBits).
1321-
*/
1322-
private int makeKey(int res) {
1323-
// It is possible for resources of different types in the 16-bit array
1324-
// to share a start offset; distinguish between those with a 2-bit value,
1325-
// as a tie-breaker in the bits just above the highest possible offset.
1326-
// It is not possible for "regular" resources of different types
1327-
// to share a start offset with each other,
1328-
// but offsets for 16-bit and "regular" resources overlap;
1329-
// use 2-bit value 0 for "regular" resources.
1330-
int type = RES_GET_TYPE(res);
1331-
int miniType =
1332-
(type == ICUResourceBundle.STRING_V2)
1333-
? 1
1334-
: (type == ICUResourceBundle.TABLE16)
1335-
? 3
1336-
: (type == ICUResourceBundle.ARRAY16) ? 2 : 0;
1337-
return RES_GET_OFFSET(res) | (miniType << maxOffsetBits);
1338-
}
1339-
1340-
private int findSimple(int key) {
1341-
return Arrays.binarySearch(keys, 0, length, key);
1185+
ResourceCache() {
1186+
map = new ConcurrentHashMap<>();
13421187
}
13431188

13441189
@SuppressWarnings("unchecked")
1345-
synchronized Object get(int res) {
1190+
Object get(int res) {
13461191
// Integers and empty resources need not be cached.
1347-
// The cache itself uses res=0 for "no match".
13481192
assert RES_GET_OFFSET(res) != 0;
1349-
Object value;
1350-
if (length >= 0) {
1351-
int index = findSimple(res);
1352-
if (index >= 0) {
1353-
value = values[index];
1354-
} else {
1355-
return null;
1356-
}
1357-
} else {
1358-
value = rootLevel.get(makeKey(res));
1359-
if (value == null) {
1360-
return null;
1361-
}
1193+
Integer resKey = res;
1194+
Object value = map.get(resKey);
1195+
if (value == null) {
1196+
return null;
13621197
}
13631198
if (value instanceof SoftReference) {
1364-
value = ((SoftReference<Object>) value).get();
1365-
}
1366-
return value; // null if the reference was cleared
1367-
}
1368-
1369-
synchronized Object putIfAbsent(int res, Object item, int size) {
1370-
if (length >= 0) {
1371-
int index = findSimple(res);
1372-
if (index >= 0) {
1373-
return putIfCleared(values, index, item, size);
1374-
} else if (length < SIMPLE_LENGTH) {
1375-
index = ~index;
1376-
if (index < length) {
1377-
System.arraycopy(keys, index, keys, index + 1, length - index);
1378-
System.arraycopy(values, index, values, index + 1, length - index);
1379-
}
1380-
++length;
1381-
keys[index] = res;
1382-
values[index] = storeDirectly(size) ? item : new SoftReference<>(item);
1383-
return item;
1384-
} else /* not found && length == SIMPLE_LENGTH */ {
1385-
// Grow to become trie-like.
1386-
rootLevel = new Level(levelBitsList, 0);
1387-
for (int i = 0; i < SIMPLE_LENGTH; ++i) {
1388-
rootLevel.putIfAbsent(makeKey(keys[i]), values[i], 0);
1389-
}
1390-
keys = null;
1391-
values = null;
1392-
length = -1;
1199+
Object referent = ((SoftReference<Object>) value).get();
1200+
if (referent == null) {
1201+
// SoftReference was cleared by GC. Remove the dead entry to prevent
1202+
// unbounded accumulation. Two-arg remove avoids ABA race.
1203+
map.remove(resKey, value);
13931204
}
1205+
return referent;
13941206
}
1395-
return rootLevel.putIfAbsent(makeKey(res), item, size);
1207+
return value;
1208+
}
1209+
1210+
@SuppressWarnings("unchecked")
1211+
Object putIfAbsent(int res, Object item, int size) {
1212+
// Use compute() for both paths to atomically handle cleared SoftReferences.
1213+
// putIfAbsent() cannot replace a cleared SoftReference (non-null but dead),
1214+
// which would return null to the caller.
1215+
Integer resKey = res;
1216+
Object[] result = new Object[] {item};
1217+
map.compute(
1218+
resKey,
1219+
(key, existing) -> {
1220+
if (existing != null) {
1221+
Object val =
1222+
existing instanceof SoftReference
1223+
? ((SoftReference<Object>) existing).get()
1224+
: existing;
1225+
if (val != null) {
1226+
result[0] = val;
1227+
return existing;
1228+
}
1229+
}
1230+
result[0] = item;
1231+
return storeDirectly(size) ? item : new SoftReference<>(item);
1232+
});
1233+
return result[0];
13961234
}
13971235
}
13981236

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
// © 2026 and later: Unicode, Inc. and others.
2+
// License & terms of use: http://www.unicode.org/copyright.html
3+
package com.ibm.icu.dev.test.util;
4+
5+
import com.ibm.icu.dev.test.CoreTestFmwk;
6+
import java.util.ArrayList;
7+
import java.util.Collections;
8+
import java.util.List;
9+
import java.util.concurrent.CyclicBarrier;
10+
import org.junit.Assume;
11+
12+
/**
13+
* Base class for concurrent regression tests. Spins up {@link #THREAD_COUNT} threads, synchronizes
14+
* them at a barrier, runs the task, and collects errors. Threads that hang past {@link #TIMEOUT_MS}
15+
* trigger a failure with diagnostic info.
16+
*
17+
* <p>Tests are skipped unless exhaustiveness >= 5 (pass {@code -DICU.exhaustive=5} to Maven).
18+
*/
19+
public abstract class ConcurrencyTest extends CoreTestFmwk {
20+
21+
protected static final int THREAD_COUNT = 16;
22+
protected static final int ITERATIONS = 2_000;
23+
private static final long TIMEOUT_MS = 30_000;
24+
private static final int MIN_EXHAUSTIVENESS = 5;
25+
26+
@FunctionalInterface
27+
protected interface ConcurrentTask {
28+
void run(int threadId) throws Exception;
29+
}
30+
31+
protected void runConcurrent(String testName, ConcurrentTask task) throws Exception {
32+
Assume.assumeTrue(
33+
"Concurrency tests require exhaustiveness >= "
34+
+ MIN_EXHAUSTIVENESS
35+
+ " (use -e "
36+
+ MIN_EXHAUSTIVENESS
37+
+ ")",
38+
getExhaustiveness() >= MIN_EXHAUSTIVENESS);
39+
40+
final CyclicBarrier barrier = new CyclicBarrier(THREAD_COUNT);
41+
final List<Throwable> errors = Collections.synchronizedList(new ArrayList<>());
42+
43+
Thread[] threads = new Thread[THREAD_COUNT];
44+
for (int t = 0; t < THREAD_COUNT; t++) {
45+
final int tid = t;
46+
threads[t] =
47+
new Thread(
48+
() -> {
49+
try {
50+
barrier.await();
51+
task.run(tid);
52+
} catch (Throwable e) {
53+
errors.add(e);
54+
}
55+
},
56+
testName + "-thread-" + t);
57+
threads[t].start();
58+
}
59+
60+
for (Thread thread : threads) {
61+
thread.join(TIMEOUT_MS);
62+
}
63+
List<String> hung = new ArrayList<>();
64+
for (Thread thread : threads) {
65+
if (thread.isAlive()) {
66+
hung.add(thread.getName());
67+
thread.interrupt();
68+
}
69+
}
70+
if (!hung.isEmpty()) {
71+
fail(
72+
testName
73+
+ ": threads still alive after "
74+
+ TIMEOUT_MS
75+
+ "ms (possible deadlock): "
76+
+ String.join(", ", hung));
77+
}
78+
if (!errors.isEmpty()) {
79+
Throwable first = errors.get(0);
80+
fail(
81+
testName
82+
+ ": "
83+
+ errors.size()
84+
+ " thread(s) threw exceptions. First: "
85+
+ first.getClass().getName()
86+
+ ": "
87+
+ first.getMessage());
88+
}
89+
}
90+
}

0 commit comments

Comments
 (0)