Skip to content

Commit

Permalink
Always create new builtin instances
Browse files Browse the repository at this point in the history
Turns out one cannot re-use builtins that are initialized in the static
context because some state is not reset between tests. Trying to enforce
that will also open a can of worms so we decided against that.

However, while trying to instantiate builtins in the constructor
we would no longer inling constructor calls, resulting in
`NoSuchMethodException` once the native image was generated. This turned
out to be a limitation related to oracle/graal#2500
so that we cannot make calls like `clazz.getContructor().newInstance()`.
`clazz.getConstructor()` in the static initializer and creating new
instances in Builtins constructor was however OK for the inliner and it
was able to procede with constant folding.
  • Loading branch information
hubertp committed Oct 25, 2022
1 parent 65bbdd4 commit a464a8f
Showing 1 changed file with 33 additions and 23 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import com.oracle.truffle.api.CompilerDirectives;

import java.lang.reflect.Constructor;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
Expand Down Expand Up @@ -48,14 +49,17 @@

/** Container class for static predefined atoms, methods, and their containing scope. */
public class Builtins {
private static Map<String, Method> builtinMethods;
private static Map<Class<? extends Builtin>, Builtin> builtins;

private static final List<Constructor<? extends Builtin>> loadedBuiltinConstructors;
private static final Map<String, Method> loadedBbuiltinMethods;

static {
builtinMethods = readBuiltinMethodsMethods();
builtins = readBuiltinTypes();
loadedBuiltinConstructors = readBuiltinTypes();
loadedBbuiltinMethods = readBuiltinMethodsMethods();
}

Map<Class<? extends Builtin>, Builtin> builtins;

public static final String PACKAGE_NAME = "Builtins";
public static final String NAMESPACE = "Standard";
public static final String MODULE_NAME = NAMESPACE + "." + PACKAGE_NAME + ".Main";
Expand Down Expand Up @@ -110,13 +114,13 @@ public Builtins(Context context) {
module = Module.empty(QualifiedName.fromString(MODULE_NAME), null, null);
scope = module.compileScope(context);

initializeBuiltinTypes(builtins, language, scope);
builtins = initializeBuiltinTypes(loadedBuiltinConstructors, language, scope);
builtinsByName =
builtins.values().stream()
.collect(
Collectors.toMap(
v -> v.getType().getName(), java.util.function.Function.identity()));
builtinMethodNodes = readBuiltinMethodsMetadata(builtinMethods, scope);
builtinMethodNodes = readBuiltinMethodsMetadata(loadedBbuiltinMethods, scope);
registerBuiltinMethods(scope, language);

error = new Error(this, context);
Expand Down Expand Up @@ -215,7 +219,7 @@ public void initializeBuiltinsIr(FreshNameSupply freshNameSupply, Passes passes)
*
* @return map of builtin types' classes and their instances
*/
private static Map<Class<? extends Builtin>, Builtin> readBuiltinTypes() {
private static List<Constructor<? extends Builtin>> readBuiltinTypes() {
ClassLoader classLoader = Builtins.class.getClassLoader();
List<String> lines;
try (InputStream resource = classLoader.getResourceAsStream(TypeProcessor.META_PATH)) {
Expand All @@ -240,27 +244,35 @@ private static Map<Class<? extends Builtin>, Builtin> readBuiltinTypes() {
@SuppressWarnings("unchecked")
Class<? extends Builtin> clazz =
(Class<? extends Builtin>) Class.forName(builtinMeta[1]);
return clazz.getConstructor().newInstance();
} catch (ClassNotFoundException e) {

// Note: Don't create a new instance of the builtin at this point
// because that will be too much for the inliner and won't get
// constant folded.
return clazz.getConstructor();
} catch (ClassNotFoundException | NoSuchMethodException e) {
e.printStackTrace();
throw new CompilerError("Invalid builtin type entry: " + builtinMeta[1]);
} catch (NoSuchMethodException
| InstantiationException
| IllegalAccessException
| InvocationTargetException e) {
e.printStackTrace();
throw new CompilerError("Invalid builtin type entry: " + line);
}
})
.collect(Collectors.toMap(Builtin::getClass, b -> b));
.collect(Collectors.toList());
}

/** Initialize builting types in the context of the given language and module scope */
private void initializeBuiltinTypes(
Map<Class<? extends Builtin>, Builtin> builtinInstances,
Language language,
ModuleScope scope) {
builtinInstances.values().forEach(b -> b.initialize(language, scope, builtinInstances));
private Map<Class<? extends Builtin>, Builtin> initializeBuiltinTypes(
List<Constructor<? extends Builtin>> constrs, Language language, ModuleScope scope) {
Map<Class<? extends Builtin>, Builtin> builtins = new HashMap<>();
constrs.forEach(
constr -> {
try {
Builtin builtin = constr.newInstance();
builtins.put(builtin.getClass(), builtin);
} catch (InstantiationException | IllegalAccessException | InvocationTargetException e) {
e.printStackTrace();
throw new CompilerError("Invalid builtin type entry: " + constr);
}
});
builtins.values().forEach(b -> b.initialize(language, scope, builtins));
return builtins;
}

/**
Expand Down Expand Up @@ -313,8 +325,6 @@ private Map<String, Map<String, Method>> readBuiltinMethodsMetadata(
* single builtin method per row. The format of the row is as follows: <Fully qualified name of
* the builtin method>:<Class name of the builtin method representing it>
*
* @param scope Builtins scope
* @param classes a map of (already loaded) builtin methods
* @return A map of builtin method nodes per builtin type name
*/
private static Map<String, Method> readBuiltinMethodsMethods() {
Expand Down

0 comments on commit a464a8f

Please sign in to comment.