Skip to content

Commit

Permalink
[GR-54712] Add layered string interning support.
Browse files Browse the repository at this point in the history
PullRequest: graal/18075
  • Loading branch information
teshull committed Jun 22, 2024
2 parents b6e01ad + 45aba98 commit 2e64af1
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.IdentityHashMap;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand All @@ -86,7 +87,6 @@
import com.oracle.graal.pointsto.util.AnalysisFuture;
import com.oracle.svm.util.FileDumpingUtil;

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

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

public void setImageInternedStrings(String[] imageInternedStrings) {
this.imageInternedStrings = imageInternedStrings;
public void setInternedStringsIdentityMap(IdentityHashMap<String, String> map) {
this.internedStringsIdentityMap = map;
}

public void setImageHeap(ImageHeap heap) {
Expand Down Expand Up @@ -338,16 +335,13 @@ public void persistConstantRelinkingInfo(EconomicMap<String, Object> constantMap
}
}

@SuppressFBWarnings(value = "ES", justification = "Reference equality check needed to detect intern status")
public void persistConstantRelinkingInfo(EconomicMap<String, Object> constantMap, BigBang bb, Class<?> clazz, JavaConstant hostedObject, int id) {
if (clazz.equals(String.class)) {
String value = bb.getSnippetReflectionProvider().asObject(String.class, hostedObject);
int stringIndex = Arrays.binarySearch(imageInternedStrings, value);
/*
* Arrays.binarySearch compares the strings by value. A comparison by reference is
* needed here as only interned strings are relinked.
*/
if (stringIndex >= 0 && imageInternedStrings[stringIndex] == value) {
if (internedStringsIdentityMap.containsKey(value)) {
/*
* Interned strings must be relinked.
*/
constantMap.put(VALUE_TAG, value);
constantsToRelink.add(id);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,12 @@
package com.oracle.svm.core.jdk;

import java.lang.reflect.Field;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.EnumSet;
import java.util.IdentityHashMap;
import java.util.List;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;

import org.graalvm.nativeimage.Platform;
Expand All @@ -34,10 +39,19 @@
import com.oracle.svm.core.BuildPhaseProvider.AfterHeapLayout;
import com.oracle.svm.core.feature.AutomaticallyRegisteredImageSingleton;
import com.oracle.svm.core.heap.UnknownObjectField;
import com.oracle.svm.core.imagelayer.ImageLayerBuildingSupport;
import com.oracle.svm.core.layeredimagesingleton.ImageSingletonLoader;
import com.oracle.svm.core.layeredimagesingleton.ImageSingletonWriter;
import com.oracle.svm.core.layeredimagesingleton.LayeredImageSingletonBuilderFlags;
import com.oracle.svm.core.layeredimagesingleton.MultiLayeredImageSingleton;
import com.oracle.svm.util.ReflectionUtil;

@AutomaticallyRegisteredImageSingleton
public final class StringInternSupport {
public final class StringInternSupport implements MultiLayeredImageSingleton {

interface SetGenerator {
Set<String> generateSet();
}

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

@Platforms(Platform.HOSTED_ONLY.class) private Object priorLayersInternedStrings;

@Platforms(Platform.HOSTED_ONLY.class) private IdentityHashMap<String, String> internedStringsIdentityMap;

@Platforms(Platform.HOSTED_ONLY.class)
public StringInternSupport() {
this(Set.of());
}

private StringInternSupport(Object obj) {
this.internedStrings = new ConcurrentHashMap<>(16, 0.75f, 1);
this.priorLayersInternedStrings = obj;
}

@Platforms(Platform.HOSTED_ONLY.class)
public void setImageInternedStrings(String[] newImageInternedStrings) {
assert !ImageLayerBuildingSupport.buildingImageLayer();
this.imageInternedStrings = newImageInternedStrings;
}

protected String intern(String str) {
@SuppressWarnings("unchecked")
@Platforms(Platform.HOSTED_ONLY.class)
public String[] layeredSetImageInternedStrings(Set<String> layerInternedStrings) {
assert ImageLayerBuildingSupport.buildingImageLayer();
/*
* When building a layered image, it is possible that this string has been interned in a
* prior layer. Thus, we must filter the interned string away from this array.
*
* In addition, the hashcode for the string should match across layers.
*/
String[] currentLayerInternedStrings;
Set<String> priorInternedStrings;
if (priorLayersInternedStrings instanceof SetGenerator generator) {
priorInternedStrings = generator.generateSet();
} else {
priorInternedStrings = (Set<String>) priorLayersInternedStrings;
}
// don't need this anymore
priorLayersInternedStrings = null;

if (priorInternedStrings.isEmpty()) {
currentLayerInternedStrings = layerInternedStrings.toArray(String[]::new);
} else {
currentLayerInternedStrings = layerInternedStrings.stream().filter(value -> !priorInternedStrings.contains(value)).toArray(String[]::new);
}

Arrays.sort(currentLayerInternedStrings);

this.imageInternedStrings = currentLayerInternedStrings;

if (ImageLayerBuildingSupport.buildingSharedLayer()) {
internedStringsIdentityMap = new IdentityHashMap<>(priorInternedStrings.size() + currentLayerInternedStrings.length);
for (var value : priorInternedStrings) {
String internedVersion = value.intern();
internedStringsIdentityMap.put(internedVersion, internedVersion);
}
Arrays.stream(currentLayerInternedStrings).forEach(internedString -> internedStringsIdentityMap.put(internedString, internedString));
}

return imageInternedStrings;
}

@Platforms(Platform.HOSTED_ONLY.class)
public IdentityHashMap<String, String> getInternedStringsIdentityMap() {
assert internedStringsIdentityMap != null;
return internedStringsIdentityMap;
}

String intern(String str) {
String result = internedStrings.get(str);
if (result != null) {
return result;
Expand All @@ -80,14 +152,49 @@ protected String intern(String str) {

private String doIntern(String str) {
String result = str;
int imageIdx = Arrays.binarySearch(imageInternedStrings, str);
if (imageIdx >= 0) {
result = imageInternedStrings[imageIdx];
if (ImageLayerBuildingSupport.buildingImageLayer()) {
StringInternSupport[] layers = MultiLayeredImageSingleton.getAllLayers(StringInternSupport.class);
for (StringInternSupport layer : layers) {
String[] layerImageInternedStrings = layer.imageInternedStrings;
int imageIdx = Arrays.binarySearch(layerImageInternedStrings, str);
if (imageIdx >= 0) {
result = layerImageInternedStrings[imageIdx];
break;
}
}
} else {
int imageIdx = Arrays.binarySearch(imageInternedStrings, str);
if (imageIdx >= 0) {
result = imageInternedStrings[imageIdx];
}
}
String oldValue = internedStrings.putIfAbsent(result, result);
if (oldValue != null) {
result = oldValue;
}
return result;
}

@Override
public EnumSet<LayeredImageSingletonBuilderFlags> getImageBuilderFlags() {
return LayeredImageSingletonBuilderFlags.ALL_ACCESS;
}

@Override
public PersistFlags preparePersist(ImageSingletonWriter writer) {
// This can be switched to use constant ids in the future
List<String> newPriorInternedStrings = new ArrayList<>(internedStringsIdentityMap.size());

newPriorInternedStrings.addAll(internedStringsIdentityMap.keySet());

writer.writeStringList("internedStrings", newPriorInternedStrings);
return PersistFlags.CREATE;
}

@SuppressWarnings("unused")
public static Object createFromLoader(ImageSingletonLoader loader) {
SetGenerator gen = (() -> Set.of(loader.readStringList("internedStrings").toArray(new String[0])));

return new StringInternSupport(gen);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -245,11 +245,17 @@ public void addTrailingObjects() {
* By now, all interned Strings have been added to our internal interning table.
* Populate the VM configuration with this table, and ensure it is part of the heap.
*/
String[] imageInternedStrings = internedStrings.keySet().toArray(new String[0]);
Arrays.sort(imageInternedStrings);
ImageSingletons.lookup(StringInternSupport.class).setImageInternedStrings(imageInternedStrings);
if (ImageLayerBuildingSupport.buildingSharedLayer()) {
HostedImageLayerBuildingSupport.singleton().getWriter().setImageInternedStrings(imageInternedStrings);
String[] imageInternedStrings;
if (ImageLayerBuildingSupport.buildingImageLayer()) {
var internSupport = ImageSingletons.lookup(StringInternSupport.class);
imageInternedStrings = internSupport.layeredSetImageInternedStrings(internedStrings.keySet());
if (ImageLayerBuildingSupport.buildingSharedLayer()) {
HostedImageLayerBuildingSupport.singleton().getWriter().setInternedStringsIdentityMap(internSupport.getInternedStringsIdentityMap());
}
} else {
imageInternedStrings = internedStrings.keySet().toArray(new String[0]);
Arrays.sort(imageInternedStrings);
ImageSingletons.lookup(StringInternSupport.class).setImageInternedStrings(imageInternedStrings);
}
/* Manually snapshot the interned strings array. */
aUniverse.getHeapScanner().rescanObject(imageInternedStrings, OtherReason.LATE_SCAN);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -302,8 +302,7 @@ public void addInitialObjects(NativeImageHeap heap, HostedUniverse hUniverse) {
/*
* Need to install the array which points to all installed singletons.
*/
heap.hMetaAccess.lookupJavaType(slotInfo.keyClass());
ImageHeapObjectArray imageHeapArray = createMultiLayerArray(slotInfo.keyClass(), heap.hMetaAccess.lookupJavaType(slotInfo.keyClass()).getWrapped(),
ImageHeapObjectArray imageHeapArray = createMultiLayerArray(slotInfo.keyClass(), heap.hMetaAccess.lookupJavaType(slotInfo.keyClass().arrayType()).getWrapped(),
hUniverse.getSnippetReflection());

heap.addConstant(imageHeapArray, true, addReason);
Expand Down

0 comments on commit 2e64af1

Please sign in to comment.