diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java index f15fc9d12..843a3028a 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java @@ -956,7 +956,7 @@ public LuaCompilationUnit transformProgToLua() { timeTaker.endPhase(); } beginPhase(13, "lua remove garbage"); - RemoveGarbage.removeGarbage(imProg); + RemoveGarbage.removeGarbage(imProg, imTranslator); imProg.flatten(imTranslator); timeTaker.endPhase(); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImOptimizer.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImOptimizer.java index f9c707d6e..04c8c2526 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImOptimizer.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imoptimizer/ImOptimizer.java @@ -19,6 +19,8 @@ import de.peeeq.wurstscript.utils.Pair; import de.peeeq.wurstscript.validation.TRVEHelper; +import java.util.stream.Collectors; + import java.util.*; public class ImOptimizer { @@ -150,8 +152,15 @@ public boolean removeGarbage() { totalFunctionsRemoved += classFunctionsBefore - classFunctionsAfter; allFunctions.addAll(c.getFunctions()); + // A field of a specialised class is a copy which nothing refers to, an access made + // before specialisation still naming the original's variable. It is live exactly + // when the field it was copied from is; dropping it leaves an instance allocated + // with no fields while the emitted code goes on reading them. int classFieldsBefore = c.getFields().size(); - changes |= c.getFields().retainAll(readVars); + changes |= c.getFields().retainAll(c.getFields().stream() + .filter(field -> readVars.contains(field) + || readVars.contains(trans.originalOfSpecializedField(field))) + .collect(Collectors.toSet())); int classFieldsAfter = c.getFields().size(); totalGlobalsRemoved += classFieldsBefore - classFieldsAfter; } diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java index 9d423291b..696f6be40 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/EliminateGenerics.java @@ -1427,6 +1427,11 @@ private ImClass specializeClass(ImClass c, GenericTypes generics) { } ImClass newC = c.copyWithRefs(); newC.setSuperClasses(new ArrayList<>(newC.getSuperClasses())); + // The copy is structural, so field i of the copy is field i of the original. Nothing will + // refer to the copies, so this is the only record that they are the same fields. + for (int i = 0; i < c.getFields().size() && i < newC.getFields().size(); i++) { + translator.recordSpecializedField(newC.getFields().get(i), c.getFields().get(i)); + } specializedClasses.put(c, generics, newC); prog.getClasses().add(newC); newC.getTypeVariables().removeAll(); diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java index 18cf87807..93f480826 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/imtranslation/ImTranslator.java @@ -42,6 +42,26 @@ public class ImTranslator { public static final String $DEBUG_PRINT = "$debugPrint"; + /** + * The field each field of a specialised class was copied from. + *

+ * Nothing refers to a copy: an access made before specialisation still names the original's + * variable. A pass which drops fields nothing reads would drop every copy, leaving an instance of + * the specialised class allocated with no fields while the emitted code goes on reading them. A + * copy is live exactly when the field it was made from is. + */ + private final java.util.Map specializedFieldOrigins = new java.util.IdentityHashMap<>(); + + public void recordSpecializedField(ImVar copy, ImVar original) { + specializedFieldOrigins.put(copy, original); + } + + /** The field {@code copy} was specialised from, or {@code copy} itself if it is not a copy. */ + public ImVar originalOfSpecializedField(ImVar copy) { + ImVar origin = specializedFieldOrigins.get(copy); + return origin == null ? copy : origin; + } + private static final de.peeeq.wurstscript.ast.Element emptyTrace = Ast.NoExpr(); // existing fields (keep callRelations as Guava Multimap to avoid ripple effects) diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java index f00242efa..ea86cbfc1 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/LuaTranslator.java @@ -391,20 +391,52 @@ private void setNameFromTrace(JassImElementWithName named) { } private void normalizeFieldNames() { + Map> namesToAvoid = collectNamesEachFieldMustAvoid(); + Map chosenNames = new IdentityHashMap<>(); Set processed = new HashSet<>(); for (ImClass c : prog.getClasses()) { - normalizeFieldNames(c, processed); + normalizeFieldNames(c, processed, namesToAvoid, chosenNames); } } - private void normalizeFieldNames(ImClass c, Set processed) { + /** + * The method names a field has to keep clear of, gathered per original field rather than per + * class. + *

+ * A specialised class holds a copy of each field, and the accesses reaching either still name the + * original's variable — so the two must end up as one table key. They cannot be normalised + * independently: the classes need not hold the same methods once unused ones are dropped, and a + * specialisation has slots of its own that the original never had. Naming each side around only + * its own methods leaves them different; restoring the original's name afterwards puts back + * whatever collision the specialisation had escaped, and an instance field which shadows a method + * slot is found first by a virtual call, which then tries to call a field. + *

+ * One name chosen against the methods of the original and of every specialisation is safe on all + * of them. + */ + private Map> collectNamesEachFieldMustAvoid() { + Map> namesToAvoid = new IdentityHashMap<>(); + for (ImClass c : prog.getClasses()) { + Set reserved = new HashSet<>(LuaReservedNames.LUA_KEYWORDS); + collectMethodNames(c, reserved, new HashSet<>()); + for (ImVar field : c.getFields()) { + namesToAvoid + .computeIfAbsent(imTr.originalOfSpecializedField(field), origin -> new HashSet<>()) + .addAll(reserved); + } + } + return namesToAvoid; + } + + private void normalizeFieldNames(ImClass c, Set processed, + Map> namesToAvoid, Map chosenNames) { if (!processed.add(c)) { return; } // Superclasses first: all fields of a hierarchy share one instance table, // so a subclass field must be renamed around already-final ancestor names. for (ImClassType sc : c.getSuperClasses()) { - normalizeFieldNames(sc.getClassDef(), processed); + normalizeFieldNames(sc.getClassDef(), processed, namesToAvoid, chosenNames); } // Field names become raw Lua table keys / field accesses, so they must not // collide with Lua keywords, method dispatch slots, or inherited fields. @@ -412,15 +444,26 @@ private void normalizeFieldNames(ImClass c, Set processed) { collectMethodNames(c, reserved, new HashSet<>()); collectSuperFieldNames(c, reserved, new HashSet<>()); for (ImVar field : c.getFields()) { - if (reserved.contains(field.getName())) { + ImVar origin = imTr.originalOfSpecializedField(field); + String settled = chosenNames.get(origin); + if (settled != null) { + // The original and its copies are one key, decided the first time any of them is met. + field.setName(settled); + reserved.add(settled); + continue; + } + Set avoid = new HashSet<>(reserved); + avoid.addAll(namesToAvoid.getOrDefault(origin, Collections.emptySet())); + if (avoid.contains(field.getName())) { String base = field.getName() + "_field"; String candidate = base; int i = 1; - while (reserved.contains(candidate)) { + while (avoid.contains(candidate)) { candidate = base + i++; } field.setName(candidate); } + chosenNames.put(origin, field.getName()); reserved.add(field.getName()); } } diff --git a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/RemoveGarbage.java b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/RemoveGarbage.java index 7c2fb7582..a5f01337e 100644 --- a/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/RemoveGarbage.java +++ b/de.peeeq.wurstscript/src/main/java/de/peeeq/wurstscript/translation/lua/translation/RemoveGarbage.java @@ -4,6 +4,7 @@ import com.google.common.collect.Multimap; import de.peeeq.wurstscript.jassIm.*; import de.peeeq.wurstscript.translation.imtranslation.ImHelper; +import de.peeeq.wurstscript.translation.imtranslation.ImTranslator; import de.peeeq.wurstscript.validation.TRVEHelper; import java.util.Collection; @@ -74,7 +75,7 @@ public void addClass(ImClass c) { } } - public static void removeGarbage(ImProg prog) { + public static void removeGarbage(ImProg prog, ImTranslator translator) { Used used = new Used(); for (ImFunction f : ImHelper.calculateFunctionsOfProg(prog)) { if (f.getName().equals("main") @@ -86,8 +87,13 @@ public static void removeGarbage(ImProg prog) { prog.getClasses().removeIf(c -> !used.getClasses().contains(c)); prog.getGlobals().removeIf(g -> !used.getVars().contains(g) && !TRVEHelper.protectedVariables.contains(g.getName())); prog.getFunctions().removeIf(f -> !used.getFunctions().contains(f)); + // A field of a specialised class is a copy which nothing refers to, an access made before + // specialisation still naming the original's variable. It is live exactly when the field it + // was copied from is; dropping it leaves an instance of the specialised class allocated with + // no fields at all while the emitted code goes on reading them. for (ImClass c : prog.getClasses()) { - c.getFields().removeIf(g -> !used.getVars().contains(g)); + c.getFields().removeIf(g -> !used.getVars().contains(g) + && !used.getVars().contains(translator.originalOfSpecializedField(g))); c.getFunctions().removeIf(f -> !used.getFunctions().contains(f)); c.getMethods().removeIf(m -> !used.getMethods().contains(m)); for (ImMethod m : c.getMethods()) { diff --git a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/FastHashMapTests.java b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/FastHashMapTests.java index 23817b5a2..f98c1a09b 100644 --- a/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/FastHashMapTests.java +++ b/de.peeeq.wurstscript/src/test/java/tests/wurstscript/tests/FastHashMapTests.java @@ -386,6 +386,94 @@ public void emittedCodeCostsNothingExtra() throws IOException { } } + /** + * Everything above compiles a package on its own. The container is meant to live in the standard + * library, and that is a different question: the bound has to keep dispatching with everything the + * library defines in scope, {@code int} has to keep taking the instance declared beside the map + * rather than anything the library brings, and the specialised copies have to survive a program of + * that size being optimised around them. + */ + private static String[] withStandardLibrary(String[] lines) { + return java.util.Arrays.stream(lines) + .filter(line -> !line.equals("native testSuccess()")) + .toArray(String[]::new); + } + + @Test + public void fastHashMapAgainstTheStandardLibrary() { + test().withStdLib().executeProg() + .lines(withStandardLibrary(program(fastHashMap(), INT_INSTANCE, USE_WITH_COLLISION))); + } + + /** + * Compiled rather than run, as every other test which puts the standard library on Lua is: the + * runtime shim cannot initialise the library's own packages, so no standard library program has + * ever executed on that target here. What this covers is that the container survives translation + * with the library in scope, which is where the specialised copies and the erased ones meet. + */ + @Test + public void fastHashMapAgainstTheStandardLibraryLua() throws IOException { + test().withStdLib().testLua(true) + .lines(withStandardLibrary(program(fastHashMap(), INT_INSTANCE, USE_WITH_COLLISION))); + assertSpecialisedClassesAllocateTheirFields( + Files.toString(new File("test-output/lua/FastHashMapTests_fastHashMapAgainstTheStandardLibraryLua.lua"), + Charsets.UTF_8)); + } + + /** + * A specialised class allocates the same fields as the class it was specialised from. Nothing + * refers to the copies it holds - an access made before specialisation still names the original's + * variable - so a pass which drops unread fields drops all of them, and an instance allocated + * from the specialised class comes out with no fields at all while the emitted code goes on + * reading them by name. + */ + private static void assertSpecialisedClassesAllocateTheirFields(String compiled) { + String erasedFields = allocatedFields(compiled, "FastHashMap"); + String specialisedFields = allocatedFields(compiled, "FastHashMap_specialized\\w*"); + if (!erasedFields.equals(specialisedFields)) { + throw new AssertionError("the specialised class should allocate the same fields as the erased one." + + "\n erased: " + erasedFields + + "\n specialised: " + specialisedFields); + } + } + + private static String allocatedFields(String compiled, String classPattern) { + Matcher m = Pattern.compile("function " + classPattern + ":create\\d*\\(\\)\\s*\\R" + + "\\s*local new_inst = \\(\\{([^}]*)\\}\\)").matcher(compiled); + if (!m.find()) { + throw new AssertionError("expected an allocation for " + classPattern + " in:\n" + compiled); + } + return m.group(1).trim(); + } + + /** + * A field may share its name with a method, and Lua puts both in one table, so the field is + * renamed around the method. That renaming is decided per class from that class's own methods, + * and pruning can leave a specialised class holding a different set than the class it was copied + * from - so the two can be renamed differently. The accesses still name the original's field, so + * the copy has to end up with the same key or the allocation writes one nothing reads. + */ + private static String[] fieldNamedLikeAMethod() { + String[] lines = fastHashMap(); + for (int i = 0; i < lines.length; i++) { + lines[i] = lines[i] + .replace("private int count = 0", "private int size = 0") + .replace("count++", "size++") + .replace("count--", "size--") + .replace("function size() returns int", "function size() returns int") + .replace(" return count", " return size"); + } + return lines; + } + + @Test + public void aFieldNamedLikeAMethodKeepsOneKeyAcrossSpecialisation() throws IOException { + test().testLua(true).executeProg() + .lines(program(fieldNamedLikeAMethod(), INT_INSTANCE, USE_WITH_COLLISION)); + assertSpecialisedClassesAllocateTheirFields( + compiledLua("aFieldNamedLikeAMethodKeepsOneKeyAcrossSpecialisation")); + } + private String compiledJass(String testName) throws IOException { return Files.toString(new File(TEST_OUTPUT_PATH, "FastHashMapTests_" + testName + ".j"), Charsets.UTF_8); }