Skip to content

Commit 2e64af1

Browse files
teshullteshull
committed
[GR-54712] Add layered string interning support.
PullRequest: graal/18075
2 parents b6e01ad + 45aba98 commit 2e64af1

File tree

4 files changed

+132
-26
lines changed

4 files changed

+132
-26
lines changed

‎substratevm/src/com.oracle.graal.pointsto/src/com/oracle/graal/pointsto/heap/ImageLayerWriter.java‎

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@
6969
import java.nio.file.Path;
7070
import java.util.ArrayList;
7171
import java.util.Arrays;
72+
import java.util.IdentityHashMap;
7273
import java.util.List;
7374
import java.util.Map;
7475
import java.util.Set;
@@ -86,7 +87,6 @@
8687
import com.oracle.graal.pointsto.util.AnalysisFuture;
8788
import com.oracle.svm.util.FileDumpingUtil;
8889

89-
import jdk.graal.compiler.core.common.SuppressFBWarnings;
9090
import jdk.graal.compiler.debug.GraalError;
9191
import jdk.graal.compiler.nodes.spi.IdentityHashCodeProvider;
9292
import jdk.graal.compiler.util.json.JsonWriter;
@@ -99,10 +99,7 @@ public class ImageLayerWriter {
9999
public static final String TYPE_SWITCH_SUBSTRING = "$$TypeSwitch";
100100
private final ImageLayerSnapshotUtil imageLayerSnapshotUtil;
101101
private ImageHeap imageHeap;
102-
/**
103-
* Contains the same array as StringInternSupport#imageInternedStrings, which is sorted.
104-
*/
105-
private String[] imageInternedStrings;
102+
private IdentityHashMap<String, String> internedStringsIdentityMap;
106103

107104
protected EconomicMap<String, Object> jsonMap = EconomicMap.create();
108105
protected List<Integer> constantsToRelink;
@@ -120,8 +117,8 @@ public ImageLayerWriter(ImageLayerSnapshotUtil imageLayerSnapshotUtil) {
120117
this.constantsToRelink = new ArrayList<>();
121118
}
122119

123-
public void setImageInternedStrings(String[] imageInternedStrings) {
124-
this.imageInternedStrings = imageInternedStrings;
120+
public void setInternedStringsIdentityMap(IdentityHashMap<String, String> map) {
121+
this.internedStringsIdentityMap = map;
125122
}
126123

127124
public void setImageHeap(ImageHeap heap) {
@@ -338,16 +335,13 @@ public void persistConstantRelinkingInfo(EconomicMap<String, Object> constantMap
338335
}
339336
}
340337

341-
@SuppressFBWarnings(value = "ES", justification = "Reference equality check needed to detect intern status")
342338
public void persistConstantRelinkingInfo(EconomicMap<String, Object> constantMap, BigBang bb, Class<?> clazz, JavaConstant hostedObject, int id) {
343339
if (clazz.equals(String.class)) {
344340
String value = bb.getSnippetReflectionProvider().asObject(String.class, hostedObject);
345-
int stringIndex = Arrays.binarySearch(imageInternedStrings, value);
346-
/*
347-
* Arrays.binarySearch compares the strings by value. A comparison by reference is
348-
* needed here as only interned strings are relinked.
349-
*/
350-
if (stringIndex >= 0 && imageInternedStrings[stringIndex] == value) {
341+
if (internedStringsIdentityMap.containsKey(value)) {
342+
/*
343+
* Interned strings must be relinked.
344+
*/
351345
constantMap.put(VALUE_TAG, value);
352346
constantsToRelink.add(id);
353347
}

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/StringInternSupport.java‎

Lines changed: 112 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,12 @@
2525
package com.oracle.svm.core.jdk;
2626

2727
import java.lang.reflect.Field;
28+
import java.util.ArrayList;
2829
import java.util.Arrays;
30+
import java.util.EnumSet;
31+
import java.util.IdentityHashMap;
32+
import java.util.List;
33+
import java.util.Set;
2934
import java.util.concurrent.ConcurrentHashMap;
3035

3136
import org.graalvm.nativeimage.Platform;
@@ -34,10 +39,19 @@
3439
import com.oracle.svm.core.BuildPhaseProvider.AfterHeapLayout;
3540
import com.oracle.svm.core.feature.AutomaticallyRegisteredImageSingleton;
3641
import com.oracle.svm.core.heap.UnknownObjectField;
42+
import com.oracle.svm.core.imagelayer.ImageLayerBuildingSupport;
43+
import com.oracle.svm.core.layeredimagesingleton.ImageSingletonLoader;
44+
import com.oracle.svm.core.layeredimagesingleton.ImageSingletonWriter;
45+
import com.oracle.svm.core.layeredimagesingleton.LayeredImageSingletonBuilderFlags;
46+
import com.oracle.svm.core.layeredimagesingleton.MultiLayeredImageSingleton;
3747
import com.oracle.svm.util.ReflectionUtil;
3848

3949
@AutomaticallyRegisteredImageSingleton
40-
public final class StringInternSupport {
50+
public final class StringInternSupport implements MultiLayeredImageSingleton {
51+
52+
interface SetGenerator {
53+
Set<String> generateSet();
54+
}
4155

4256
@Platforms(Platform.HOSTED_ONLY.class)
4357
public static Field getInternedStringsField() {
@@ -59,17 +73,75 @@ public static Field getInternedStringsField() {
5973
*/
6074
@UnknownObjectField(availability = AfterHeapLayout.class) private String[] imageInternedStrings;
6175

76+
@Platforms(Platform.HOSTED_ONLY.class) private Object priorLayersInternedStrings;
77+
78+
@Platforms(Platform.HOSTED_ONLY.class) private IdentityHashMap<String, String> internedStringsIdentityMap;
79+
6280
@Platforms(Platform.HOSTED_ONLY.class)
6381
public StringInternSupport() {
82+
this(Set.of());
83+
}
84+
85+
private StringInternSupport(Object obj) {
6486
this.internedStrings = new ConcurrentHashMap<>(16, 0.75f, 1);
87+
this.priorLayersInternedStrings = obj;
6588
}
6689

6790
@Platforms(Platform.HOSTED_ONLY.class)
6891
public void setImageInternedStrings(String[] newImageInternedStrings) {
92+
assert !ImageLayerBuildingSupport.buildingImageLayer();
6993
this.imageInternedStrings = newImageInternedStrings;
7094
}
7195

72-
protected String intern(String str) {
96+
@SuppressWarnings("unchecked")
97+
@Platforms(Platform.HOSTED_ONLY.class)
98+
public String[] layeredSetImageInternedStrings(Set<String> layerInternedStrings) {
99+
assert ImageLayerBuildingSupport.buildingImageLayer();
100+
/*
101+
* When building a layered image, it is possible that this string has been interned in a
102+
* prior layer. Thus, we must filter the interned string away from this array.
103+
*
104+
* In addition, the hashcode for the string should match across layers.
105+
*/
106+
String[] currentLayerInternedStrings;
107+
Set<String> priorInternedStrings;
108+
if (priorLayersInternedStrings instanceof SetGenerator generator) {
109+
priorInternedStrings = generator.generateSet();
110+
} else {
111+
priorInternedStrings = (Set<String>) priorLayersInternedStrings;
112+
}
113+
// don't need this anymore
114+
priorLayersInternedStrings = null;
115+
116+
if (priorInternedStrings.isEmpty()) {
117+
currentLayerInternedStrings = layerInternedStrings.toArray(String[]::new);
118+
} else {
119+
currentLayerInternedStrings = layerInternedStrings.stream().filter(value -> !priorInternedStrings.contains(value)).toArray(String[]::new);
120+
}
121+
122+
Arrays.sort(currentLayerInternedStrings);
123+
124+
this.imageInternedStrings = currentLayerInternedStrings;
125+
126+
if (ImageLayerBuildingSupport.buildingSharedLayer()) {
127+
internedStringsIdentityMap = new IdentityHashMap<>(priorInternedStrings.size() + currentLayerInternedStrings.length);
128+
for (var value : priorInternedStrings) {
129+
String internedVersion = value.intern();
130+
internedStringsIdentityMap.put(internedVersion, internedVersion);
131+
}
132+
Arrays.stream(currentLayerInternedStrings).forEach(internedString -> internedStringsIdentityMap.put(internedString, internedString));
133+
}
134+
135+
return imageInternedStrings;
136+
}
137+
138+
@Platforms(Platform.HOSTED_ONLY.class)
139+
public IdentityHashMap<String, String> getInternedStringsIdentityMap() {
140+
assert internedStringsIdentityMap != null;
141+
return internedStringsIdentityMap;
142+
}
143+
144+
String intern(String str) {
73145
String result = internedStrings.get(str);
74146
if (result != null) {
75147
return result;
@@ -80,14 +152,49 @@ protected String intern(String str) {
80152

81153
private String doIntern(String str) {
82154
String result = str;
83-
int imageIdx = Arrays.binarySearch(imageInternedStrings, str);
84-
if (imageIdx >= 0) {
85-
result = imageInternedStrings[imageIdx];
155+
if (ImageLayerBuildingSupport.buildingImageLayer()) {
156+
StringInternSupport[] layers = MultiLayeredImageSingleton.getAllLayers(StringInternSupport.class);
157+
for (StringInternSupport layer : layers) {
158+
String[] layerImageInternedStrings = layer.imageInternedStrings;
159+
int imageIdx = Arrays.binarySearch(layerImageInternedStrings, str);
160+
if (imageIdx >= 0) {
161+
result = layerImageInternedStrings[imageIdx];
162+
break;
163+
}
164+
}
165+
} else {
166+
int imageIdx = Arrays.binarySearch(imageInternedStrings, str);
167+
if (imageIdx >= 0) {
168+
result = imageInternedStrings[imageIdx];
169+
}
86170
}
87171
String oldValue = internedStrings.putIfAbsent(result, result);
88172
if (oldValue != null) {
89173
result = oldValue;
90174
}
91175
return result;
92176
}
177+
178+
@Override
179+
public EnumSet<LayeredImageSingletonBuilderFlags> getImageBuilderFlags() {
180+
return LayeredImageSingletonBuilderFlags.ALL_ACCESS;
181+
}
182+
183+
@Override
184+
public PersistFlags preparePersist(ImageSingletonWriter writer) {
185+
// This can be switched to use constant ids in the future
186+
List<String> newPriorInternedStrings = new ArrayList<>(internedStringsIdentityMap.size());
187+
188+
newPriorInternedStrings.addAll(internedStringsIdentityMap.keySet());
189+
190+
writer.writeStringList("internedStrings", newPriorInternedStrings);
191+
return PersistFlags.CREATE;
192+
}
193+
194+
@SuppressWarnings("unused")
195+
public static Object createFromLoader(ImageSingletonLoader loader) {
196+
SetGenerator gen = (() -> Set.of(loader.readStringList("internedStrings").toArray(new String[0])));
197+
198+
return new StringInternSupport(gen);
199+
}
93200
}

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/image/NativeImageHeap.java‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -245,11 +245,17 @@ public void addTrailingObjects() {
245245
* By now, all interned Strings have been added to our internal interning table.
246246
* Populate the VM configuration with this table, and ensure it is part of the heap.
247247
*/
248-
String[] imageInternedStrings = internedStrings.keySet().toArray(new String[0]);
249-
Arrays.sort(imageInternedStrings);
250-
ImageSingletons.lookup(StringInternSupport.class).setImageInternedStrings(imageInternedStrings);
251-
if (ImageLayerBuildingSupport.buildingSharedLayer()) {
252-
HostedImageLayerBuildingSupport.singleton().getWriter().setImageInternedStrings(imageInternedStrings);
248+
String[] imageInternedStrings;
249+
if (ImageLayerBuildingSupport.buildingImageLayer()) {
250+
var internSupport = ImageSingletons.lookup(StringInternSupport.class);
251+
imageInternedStrings = internSupport.layeredSetImageInternedStrings(internedStrings.keySet());
252+
if (ImageLayerBuildingSupport.buildingSharedLayer()) {
253+
HostedImageLayerBuildingSupport.singleton().getWriter().setInternedStringsIdentityMap(internSupport.getInternedStringsIdentityMap());
254+
}
255+
} else {
256+
imageInternedStrings = internedStrings.keySet().toArray(new String[0]);
257+
Arrays.sort(imageInternedStrings);
258+
ImageSingletons.lookup(StringInternSupport.class).setImageInternedStrings(imageInternedStrings);
253259
}
254260
/* Manually snapshot the interned strings array. */
255261
aUniverse.getHeapScanner().rescanObject(imageInternedStrings, OtherReason.LATE_SCAN);

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/imagelayer/LoadImageSingletonFeature.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -302,8 +302,7 @@ public void addInitialObjects(NativeImageHeap heap, HostedUniverse hUniverse) {
302302
/*
303303
* Need to install the array which points to all installed singletons.
304304
*/
305-
heap.hMetaAccess.lookupJavaType(slotInfo.keyClass());
306-
ImageHeapObjectArray imageHeapArray = createMultiLayerArray(slotInfo.keyClass(), heap.hMetaAccess.lookupJavaType(slotInfo.keyClass()).getWrapped(),
305+
ImageHeapObjectArray imageHeapArray = createMultiLayerArray(slotInfo.keyClass(), heap.hMetaAccess.lookupJavaType(slotInfo.keyClass().arrayType()).getWrapped(),
307306
hUniverse.getSnippetReflection());
308307

309308
heap.addConstant(imageHeapArray, true, addReason);

0 commit comments

Comments
 (0)