Skip to content

Commit 860f582

Browse files
d-kozakd-kozak
committed
address issues from the review
1 parent f87ef4a commit 860f582

File tree

8 files changed

+118
-77
lines changed

8 files changed

+118
-77
lines changed

‎substratevm/src/com.oracle.svm.agent/src/com/oracle/svm/agent/BreakpointInterceptor.java‎

Lines changed: 43 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
import static com.oracle.svm.jvmtiagentbase.Support.jvmtiEnv;
4242
import static com.oracle.svm.jvmtiagentbase.Support.jvmtiFunctions;
4343
import static com.oracle.svm.jvmtiagentbase.Support.newObjectL;
44-
import static com.oracle.svm.jvmtiagentbase.Support.readObjectField;
4544
import static com.oracle.svm.jvmtiagentbase.Support.testException;
4645
import static com.oracle.svm.jvmtiagentbase.Support.toCString;
4746
import static com.oracle.svm.jvmtiagentbase.jvmti.JvmtiEvent.JVMTI_EVENT_BREAKPOINT;
@@ -53,15 +52,13 @@
5352
import java.nio.ByteOrder;
5453
import java.util.ArrayList;
5554
import java.util.Arrays;
56-
import java.util.Collections;
5755
import java.util.HashMap;
5856
import java.util.List;
5957
import java.util.Map;
6058
import java.util.concurrent.ConcurrentHashMap;
6159
import java.util.concurrent.ConcurrentMap;
6260
import java.util.function.Supplier;
6361

64-
import org.graalvm.collections.Pair;
6562
import org.graalvm.compiler.core.common.NumUtil;
6663
import org.graalvm.nativeimage.StackValue;
6764
import org.graalvm.nativeimage.UnmanagedMemory;
@@ -650,14 +647,19 @@ private static boolean getBundleImplJDK8OrEarlier(JNIEnvironment jni, Breakpoint
650647
JNIObjectHandle loader = getObjectArgument(2);
651648
JNIObjectHandle control = getObjectArgument(3);
652649
JNIObjectHandle result = Support.callStaticObjectMethodLLLL(jni, bp.clazz, bp.method, baseName, locale, loader, control);
653-
List<Pair<String, String>> bundleInfo = Collections.emptyList();
650+
String[] classNames = new String[0];
651+
String[] locales = new String[0];
654652
if (clearException(jni)) {
655653
result = nullHandle();
656654
} else {
657-
bundleInfo = extractBundleInfo(jni, result);
655+
BundleInfo bundleInfo = extractBundleInfo(jni, result);
656+
if (bundleInfo != null) {
657+
classNames = bundleInfo.classNames;
658+
locales = bundleInfo.locales;
659+
}
658660
}
659661
traceBreakpoint(jni, nullHandle(), nullHandle(), callerClass, "getBundleImplJDK8OrEarlier", result.notEqual(nullHandle()),
660-
state.getFullStackTraceOrNull(), fromJniString(jni, baseName), Tracer.UNKNOWN_VALUE, Tracer.UNKNOWN_VALUE, Tracer.UNKNOWN_VALUE, bundleInfo);
662+
state.getFullStackTraceOrNull(), fromJniString(jni, baseName), Tracer.UNKNOWN_VALUE, Tracer.UNKNOWN_VALUE, Tracer.UNKNOWN_VALUE, classNames, locales);
661663
return true;
662664
}
663665

@@ -677,28 +679,50 @@ private static boolean getBundleImplJDK11OrLater(JNIEnvironment jni, Breakpoint
677679
JNIObjectHandle locale = getObjectArgument(3);
678680
JNIObjectHandle control = getObjectArgument(4);
679681
JNIObjectHandle result = Support.callStaticObjectMethodLLLLL(jni, bp.clazz, bp.method, callerModule, module, baseName, locale, control);
680-
List<Pair<String, String>> bundleInfo = Collections.emptyList();
682+
String[] classNames = new String[0];
683+
String[] locales = new String[0];
681684
if (clearException(jni)) {
682685
result = nullHandle();
683686
} else {
684-
bundleInfo = extractBundleInfo(jni, result);
687+
BundleInfo bundleInfo = extractBundleInfo(jni, result);
688+
if (bundleInfo != null) {
689+
classNames = bundleInfo.classNames;
690+
locales = bundleInfo.locales;
691+
}
685692
}
686693
traceBreakpoint(jni, nullHandle(), nullHandle(), callerClass, "getBundleImplJDK11OrLater", result.notEqual(nullHandle()),
687-
state.getFullStackTraceOrNull(), Tracer.UNKNOWN_VALUE, Tracer.UNKNOWN_VALUE, fromJniString(jni, baseName), Tracer.UNKNOWN_VALUE, Tracer.UNKNOWN_VALUE, bundleInfo);
694+
state.getFullStackTraceOrNull(), Tracer.UNKNOWN_VALUE, Tracer.UNKNOWN_VALUE, fromJniString(jni, baseName), Tracer.UNKNOWN_VALUE, Tracer.UNKNOWN_VALUE, classNames, locales);
688695
return true;
689696
}
690697

691698
private static String readLocaleTag(JNIEnvironment jni, JNIObjectHandle locale) {
692-
return fromJniString(jni, callObjectMethod(jni, locale, agent.handles().getJavaUtilLocaleToLanguageTag(jni)));
699+
JNIObjectHandle languageTag = callObjectMethod(jni, locale, agent.handles().getJavaUtilLocaleToLanguageTag(jni));
700+
if (clearException(jni)) {
701+
/*- return root locale */
702+
return "";
703+
}
704+
return fromJniString(jni, languageTag);
705+
}
706+
707+
private static final class BundleInfo {
708+
public final String[] classNames;
709+
public final String[] locales;
710+
711+
private BundleInfo(String[] classNames, String[] locales) {
712+
this.classNames = classNames;
713+
this.locales = locales;
714+
}
693715
}
694716

695717
/**
696718
* Traverses the bundle parent chain and collects classnames and locales of all encountered
697719
* bundles.
720+
*
698721
*/
699-
private static List<Pair<String, String>> extractBundleInfo(JNIEnvironment jni, JNIObjectHandle result) {
700-
List<Pair<String, String>> res = new ArrayList<>();
701-
JNIObjectHandle curr = result;
722+
private static BundleInfo extractBundleInfo(JNIEnvironment jni, JNIObjectHandle bundle) {
723+
List<String> locales = new ArrayList<>();
724+
List<String> classNames = new ArrayList<>();
725+
JNIObjectHandle curr = bundle;
702726
while (!nullHandle().equal(curr)) {
703727
JNIObjectHandle locale = callObjectMethod(jni, curr, agent.handles().getJavaUtilResourceBundleGetLocale(jni));
704728
if (clearException(jni)) {
@@ -713,16 +737,17 @@ private static List<Pair<String, String>> extractBundleInfo(JNIEnvironment jni,
713737
if (!clearException(jni)) {
714738
JNIObjectHandle classNameHandle = callObjectMethod(jni, clazz, agent.handles().javaLangClassGetName);
715739
if (!clearException(jni)) {
716-
res.add(Pair.create(fromJniString(jni, classNameHandle), localeTag));
740+
classNames.add(fromJniString(jni, classNameHandle));
741+
locales.add(localeTag);
717742
}
718743
}
719-
curr = getParent(jni, curr);
744+
curr = getResourceBundleParent(jni, curr);
720745
}
721-
return res;
746+
return new BundleInfo(classNames.toArray(new String[0]), locales.toArray(new String[0]));
722747
}
723748

724-
private static JNIObjectHandle getParent(JNIEnvironment jni, JNIObjectHandle curr) {
725-
JNIObjectHandle parent = readObjectField(jni, curr, agent.handles().getJavaUtilResourceBundleParentField(jni));
749+
private static JNIObjectHandle getResourceBundleParent(JNIEnvironment jni, JNIObjectHandle bundle) {
750+
JNIObjectHandle parent = Support.readObjectField(jni, bundle, agent.handles().getJavaUtilResourceBundleParentField(jni));
726751
if (!clearException(jni)) {
727752
return parent;
728753
}

‎substratevm/src/com.oracle.svm.agent/src/com/oracle/svm/agent/NativeImageAgentJNIHandleSet.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,9 @@ public class NativeImageAgentJNIHandleSet extends JNIHandleSet {
7272

7373
private JNIMethodId javaLangReflectConstructorDeclaringClassName;
7474

75-
private JNIMethodId javaUtilLocaleToLanguageTag = WordFactory.nullPointer();
76-
private JNIFieldId javaUtilResourceBundleParentField = WordFactory.nullPointer();
77-
private JNIMethodId javaUtilResourceBundleGetLocale = WordFactory.nullPointer();
75+
private JNIMethodId javaUtilLocaleToLanguageTag;
76+
private JNIFieldId javaUtilResourceBundleParentField;
77+
private JNIMethodId javaUtilResourceBundleGetLocale;
7878

7979
NativeImageAgentJNIHandleSet(JNIEnvironment env) {
8080
super(env);

‎substratevm/src/com.oracle.svm.configure/src/com/oracle/svm/configure/config/ResourceConfiguration.java‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@
4141
import com.oracle.svm.configure.json.JsonWriter;
4242
import com.oracle.svm.core.configure.ConditionalElement;
4343
import com.oracle.svm.core.configure.ResourcesRegistry;
44-
import org.graalvm.collections.Pair;
4544

4645
public class ResourceConfiguration implements ConfigurationBase {
4746

@@ -81,21 +80,21 @@ public void addClassBasedResourceBundle(ConfigurationCondition condition, String
8180
}
8281
}
8382

84-
public static class BundleConfiguration {
83+
private static final class BundleConfiguration {
8584
public final ConfigurationCondition condition;
8685
public final String baseName;
8786
public final Set<String> locales = ConcurrentHashMap.newKeySet();
8887
public final Set<String> classNames = ConcurrentHashMap.newKeySet();
8988

90-
public BundleConfiguration(ConfigurationCondition condition, String baseName) {
89+
private BundleConfiguration(ConfigurationCondition condition, String baseName) {
9190
this.condition = condition;
9291
this.baseName = baseName;
9392
}
9493
}
9594

9695
private final ConcurrentMap<ConditionalElement<String>, Pattern> addedResources = new ConcurrentHashMap<>();
9796
private final ConcurrentMap<ConditionalElement<String>, Pattern> ignoredResources = new ConcurrentHashMap<>();
98-
private final ConcurrentHashMap<ConditionalElement<String>, BundleConfiguration> bundles = new ConcurrentHashMap<>();
97+
private final ConcurrentMap<ConditionalElement<String>, BundleConfiguration> bundles = new ConcurrentHashMap<>();
9998

10099
public ResourceConfiguration() {
101100
}
@@ -135,11 +134,12 @@ private void addClassResourceBundle(ConfigurationCondition condition, String bas
135134
getOrCreateBundleConfig(condition, basename).classNames.add(className);
136135
}
137136

138-
public void addBundle(ConfigurationCondition condition, List<Pair<String, String>> bundleInfo, String baseName) {
137+
public void addBundle(ConfigurationCondition condition, List<String> classNames, List<String> locales, String baseName) {
138+
assert classNames.size() == locales.size() : "Each bundle should be represented by both classname and locale";
139139
BundleConfiguration config = getOrCreateBundleConfig(condition, baseName);
140-
for (Pair<String, String> pair : bundleInfo) {
141-
String className = pair.getLeft();
142-
String localeTag = pair.getRight();
140+
for (int i = 0; i < classNames.size(); i++) {
141+
String className = classNames.get(i);
142+
String localeTag = locales.get(i);
143143
if (!className.equals(PROPERTY_BUNDLE)) {
144144
config.classNames.add(className);
145145
} else {
@@ -193,7 +193,9 @@ public void printJson(JsonWriter writer) throws IOException {
193193
}
194194

195195
private static void printResourceBundle(BundleConfiguration config, JsonWriter writer) throws IOException {
196-
writer.append('{').quote("name").append(':').quote(config.baseName);
196+
writer.append('{');
197+
ConfigurationConditionPrintable.printConditionAttribute(config.condition, writer);
198+
writer.quote("name").append(':').quote(config.baseName);
197199
if (!config.locales.isEmpty()) {
198200
writer.append(',').quote("locales").append(":");
199201
JsonPrinter.printCollection(writer, config.locales, Comparator.naturalOrder(), (String p, JsonWriter w) -> w.quote(p));

‎substratevm/src/com.oracle.svm.configure/src/com/oracle/svm/configure/trace/ReflectionProcessor.java‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@
3232
import java.util.Map;
3333
import java.util.regex.Pattern;
3434

35-
import org.graalvm.collections.Pair;
3635
import org.graalvm.compiler.phases.common.LazyValue;
3736
import org.graalvm.nativeimage.impl.ConfigurationCondition;
3837

@@ -247,19 +246,23 @@ public void processEntry(Map<String, ?> entry) {
247246
}
248247

249248
case "getBundleImplJDK8OrEarlier": {
250-
expectSize(args, 5);
249+
expectSize(args, 6);
251250
String baseName = (String) args.get(0);
252251
@SuppressWarnings("unchecked")
253-
List<Pair<String, String>> bundleInfo = (List<Pair<String, String>>) args.get(4);
254-
resourceConfiguration.addBundle(condition, bundleInfo, baseName);
252+
List<String> classNames = (List<String>) args.get(4);
253+
@SuppressWarnings("unchecked")
254+
List<String> locales = (List<String>) args.get(5);
255+
resourceConfiguration.addBundle(condition, classNames, locales, baseName);
255256
break;
256257
}
257258
case "getBundleImplJDK11OrLater": {
258-
expectSize(args, 6);
259+
expectSize(args, 7);
259260
String baseName = (String) args.get(2);
260261
@SuppressWarnings("unchecked")
261-
List<Pair<String, String>> bundleInfo = (List<Pair<String, String>>) args.get(5);
262-
resourceConfiguration.addBundle(condition, bundleInfo, baseName);
262+
List<String> classNames = (List<String>) args.get(5);
263+
@SuppressWarnings("unchecked")
264+
List<String> locales = (List<String>) args.get(6);
265+
resourceConfiguration.addBundle(condition, classNames, locales, baseName);
263266
break;
264267
}
265268
default:

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/configure/ResourceConfigurationParser.java‎

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -102,38 +102,35 @@ private void parseTopLevelObject(Map<String, Object> obj) {
102102
private void parseBundle(Object bundle) {
103103
Map<String, Object> resource = asMap(bundle, "Elements of 'bundles' list must be a bundle descriptor object");
104104
checkAttributes(resource, "bundle descriptor object", Collections.singletonList("name"), Arrays.asList("locales", "classNames"));
105-
String basename = asString(resource.get("name"), "Missing attribute 'name' in bundle descriptor object");
105+
String basename = asString(resource.get("name"));
106106
ConfigurationCondition condition = parseCondition(resource);
107107
Object locales = resource.get("locales");
108108
if (locales != null) {
109109
List<Locale> asList = asList(locales, "Attribute 'locales' must be a list of locales")
110110
.stream()
111111
.map(ResourceConfigurationParser::parseLocale)
112112
.collect(Collectors.toList());
113-
if (asList.isEmpty()) {
114-
throw new JSONParserException("List of locales for " + basename + " is empty");
113+
if (!asList.isEmpty()) {
114+
registry.addResourceBundles(condition, basename, asList);
115115
}
116-
registry.addResourceBundles(condition, basename, asList);
116+
117117
}
118118
Object classNames = resource.get("classNames");
119119
if (classNames != null) {
120120
List<Object> asList = asList(classNames, "Attribute 'classNames' must be a list of classes");
121-
if (asList.isEmpty()) {
122-
throw new JSONParserException("List of classNames for " + basename + " is empty");
123-
}
124121
for (Object o : asList) {
125-
String className = asString(o, "Elements of 'classNames' must of strings.");
122+
String className = asString(o);
126123
registry.addClassBasedResourceBundle(condition, basename, className);
127124
}
128125
}
129126
if (locales == null && classNames == null) {
130-
/*- If nothing more precise is specified, register in every included locale */
127+
/* If nothing more precise is specified, register in every included locale */
131128
registry.addResourceBundles(condition, basename);
132129
}
133130
}
134131

135132
private static Locale parseLocale(Object input) {
136-
String localeTag = asString(input, "Elements of 'locales' must be strings.");
133+
String localeTag = asString(input);
137134
Locale locale = LocalizationSupport.parseLocaleFromTag(localeTag);
138135
if (locale == null) {
139136
throw new JSONParserException(localeTag + " is not a valid locale tag");

‎substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/localization/OptimizedLocalizationSupport.java‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,12 @@
2424
*/
2525
package com.oracle.svm.core.jdk.localization;
2626

27+
import com.oracle.svm.core.util.UserError;
28+
import com.oracle.svm.util.ReflectionUtil;
2729
import org.graalvm.collections.Pair;
2830

31+
//Checkstyle: allow reflection
32+
import java.lang.reflect.Field;
2933
import java.nio.charset.Charset;
3034
import java.util.HashMap;
3135
import java.util.Locale;
@@ -76,6 +80,32 @@ public ResourceBundle getCached(String baseName, Locale locale) throws MissingRe
7680

7781
}
7882

83+
private final Field bundleNameField = ReflectionUtil.lookupField(ResourceBundle.class, "name");
84+
private final Field bundleLocaleField = ReflectionUtil.lookupField(ResourceBundle.class, "locale");
85+
86+
@Override
87+
public void prepareClassResourceBundle(String basename, Class<?> bundleClass) {
88+
try {
89+
ResourceBundle bundle = ((ResourceBundle) ReflectionUtil.newInstance(bundleClass));
90+
Locale locale = extractLocale(bundleClass);
91+
/*- Set the basename and locale to be consistent with JVM lookup process */
92+
bundleNameField.set(bundle, basename);
93+
bundleLocaleField.set(bundle, locale);
94+
prepareBundle(basename, bundle, locale);
95+
} catch (ReflectionUtil.ReflectionUtilError | ReflectiveOperationException e) {
96+
throw UserError.abort(e, "Failed to instantiated bundle from class %s, reason %s", bundleClass, e.getCause().getMessage());
97+
}
98+
}
99+
100+
private static Locale extractLocale(Class<?> bundleClass) {
101+
String name = bundleClass.getName();
102+
int split = name.lastIndexOf('_');
103+
if (split == -1) {
104+
return Locale.ROOT;
105+
}
106+
return parseLocaleFromTag(name.substring(split + 1));
107+
}
108+
79109
@Platforms(Platform.HOSTED_ONLY.class)
80110
@Override
81111
public void prepareBundle(String bundleName, ResourceBundle bundle, Locale locale) {

‎substratevm/src/com.oracle.svm.hosted/src/com/oracle/svm/hosted/ResourcesFeature.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -168,15 +168,15 @@ public void addClassBasedResourceBundle(ConfigurationCondition condition, String
168168
if (configurationTypeResolver.resolveType(condition.getTypeName()) == null) {
169169
return;
170170
}
171-
ImageSingletons.lookup(LocalizationFeature.class).prepareClassResourceBundle(basename, className);
171+
registerConditionalConfiguration(condition, () -> ImageSingletons.lookup(LocalizationFeature.class).prepareClassResourceBundle(basename, className));
172172
}
173173

174174
@Override
175175
public void addResourceBundles(ConfigurationCondition condition, String basename, Collection<Locale> locales) {
176176
if (configurationTypeResolver.resolveType(condition.getTypeName()) == null) {
177177
return;
178178
}
179-
ImageSingletons.lookup(LocalizationFeature.class).prepareBundle(basename, locales);
179+
registerConditionalConfiguration(condition, () -> ImageSingletons.lookup(LocalizationFeature.class).prepareBundle(basename, locales));
180180
}
181181
}
182182

0 commit comments

Comments
 (0)