Reuse checked models for cancellable map runs and builds

BACKLOG.md

@@ -6,11 +6,10 @@ Notes; finished-work narrative does not.
 
 ## Open
 
-- **One model for run and build.** Run and build keep two ways to compile (`SafetyLevel`: the managed
-  model as it is, or a clean, rebuild and copy) because the incremental model was not trusted to be
-  complete. `IncrementalModelOracleTests` compares the model after random edits with a build from
-  scratch (`INCREMENTAL_ORACLE_SEEDS=100` runs 100 seeds) and found two propagation gaps, both fixed.
-  What is left: a `.j` change makes `calculateCUsToUpdate` return the whole model, and a compilation
+- **Incremental map-script declarations and build source policy.** Run, editor build and CLI build use
+  the managed model; cancellation is honored at safe boundaries around shared AST preparation and
+  throughout the private IM/backend work. See `docs/RUNMAP_PERFORMANCE.md`. What is left:
+  a `.j` change makes `calculateCUsToUpdate` return the whole model, and a compilation
   swaps the map script with the project config applied into the model (`replaceBaseScriptWithConfig`).
   The swap changes only function bodies (`config`, `InitCustomPlayerSlots`, ...) yet invalidates every
   attribute, so the first compilation of a model checks all of it again; later ones skip the check. The
@@ -19,15 +18,21 @@ Notes; finished-work narrative does not.
   run still checks again. The swap's own cost in the language server was not isolated: on castle fight
   without optimisations the first build of a model takes 21 s and later ones 11 to 12 s in one JVM, and
   the stage samples show most of the difference in passes the swap does not touch, so JIT warm-up. Next,
-  in this order: keep the unchanged top-level
-  declarations of a changed Jass file and re-check only the units which mention a changed name; then
-  drop `SafetyLevel` (purging unimported units from the managed model in `compileMap` changes it for the
-  next run, and a build could compare file hashes instead of cleaning). Run and build both read the
-  open editor buffers today (`readCompilationUnitContents` prefers an open buffer, also in the rebuild after
-  `clean()`), so a build can contain text which is not on disk, while grill reads the disk. wurst4vscode
+  keep the unchanged top-level declarations of a changed Jass file and re-check only the units which
+  mention a changed name. Compilation uses an import-root view and retains the editor's units. Run and build consume the current
+  editor model (`readCompilationUnitContents` prefers an open buffer), so a build can contain text
+  which is not on disk, while grill reads the disk. wurst4vscode
   sends `wurst.buildmap` and `wurst.startmap` without saving the open files first. Whether a build should
   save first, or refuse unsaved files, is open.
 
+- **Reuse translation work on unchanged map runs.** After checked-model reuse, Survival Chaos Reborn
+  takes 4.5–5.2 s for an unoptimised warm RunMap: AST→IM 1.3–1.4 s, required Jass lowering 1.3–1.7 s,
+  compile-time work 0.9–1.0 s, map injection 0.4–0.6 s. Whole-project typechecking is now skipped.
+  Investigate retaining a pristine IM snapshot keyed by model/config/target state, then copying it
+  before mutable compile-time/backend passes; compare snapshot cost with translation and test edits,
+  configuration changes, cancellation and both targets against fresh builds. This needs an explicit
+  AST/IM ownership and invalidation contract before implementation. See `docs/RUNMAP_PERFORMANCE.md`.
+
 - **A dead dispatch slot survives for overloads inside a specialised class.** Two overloads of one
   source method share a declared name, so a specialised class holding only overloads still composes
   one shared slot and binds it to whichever is reached first. Nothing calls it. A fix needs an

de.peeeq.wurstscript/parserspec/jass_im.parseq

@@ -11,7 +11,8 @@ ImProg(
 	ImMethods methods,
 	ImClasses classes,
 	ImTypeClassFuncs typeClassFunctions,
-	java.util.Map<ImVar, java.util.List<ImSet>> globalInits)
+	java.util.Map<ImVar, java.util.List<ImSet>> globalInits,
+    @ignoreForEquality java.util.Set<de.peeeq.wurstscript.ast.NameDef> preservedVariables)
 
 ImVars * ImVar
 ImFunctions * ImFunction

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/WurstCompilerJassImpl.java

@@ -414,6 +414,11 @@ public boolean checkProg(WurstModel model) {
 
     /** @return whether the units were validated, see {@link WurstChecker#checkProg} */
     public boolean checkProg(WurstModel model, Collection<CompilationUnit> toCheck) {
+        return checkProg(model, toCheck, model);
+    }
+
+    public boolean checkProg(WurstModel model, Collection<CompilationUnit> toCheck,
+                             Collection<CompilationUnit> compilationUnits) {
         for (CompilationUnit cu : toCheck) {
             Preconditions.checkNotNull(cu);
             if (!model.contains(cu)) {
@@ -422,14 +427,18 @@ public boolean checkProg(WurstModel model, Collection<CompilationUnit> toCheck)
             }
         }
 
-        return checker.checkProg(model, toCheck);
+        return checker.checkProg(model, toCheck, compilationUnits);
     }
 
     /** Takes over a model which was checked completely and has not changed since, instead of checking it again. */
     public void adoptCheckedModel(WurstModel model) {
         checker.adoptCheckedModel(model);
     }
 
+    public void adoptCheckedModel(WurstModel model, Collection<CompilationUnit> compilationUnits) {
+        checker.adoptCheckedModel(model, compilationUnits);
+    }
+
     public JassProg transformProgToJass() {
         ImTranslator imTranslator2 = getImTranslator();
         ImProg imProg2 = getImProg();
@@ -703,12 +712,17 @@ public ImTranslator getImTranslator() {
     }
 
     public @Nullable ImProg translateProgToIm(WurstModel root) {
+        return translateProgToIm(root, root);
+    }
+
+    /** Translate a compilation view without removing units from the checked editor model. */
+    public @Nullable ImProg translateProgToIm(WurstModel root, Collection<CompilationUnit> compilationUnits) {
         beginPhase(1, "to intermediate lang");
         // Names of generated temporaries are counted from here, so that the same source compiles
         // to the same script whatever was compiled before it in this process.
         Flatten.resetTempVarCounters();
         // translate wurst to intermediate lang:
-        imTranslator = new ImTranslator(root, errorHandler.isUnitTestMode(), runArgs);
+        imTranslator = new ImTranslator(root, errorHandler.isUnitTestMode(), runArgs, compilationUnits);
         imProg = getImTranslator().translateProg();
         // Before any compile-time function runs: the library asks whether this compiler lowers CodeList.
         CodeListSupport.markSupported(imProg);

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/ModelManager.java

@@ -12,6 +12,7 @@
 
 import java.io.File;
 import java.util.List;
+import java.util.Collection;
 import java.util.Set;
 import java.util.function.Consumer;
 import java.util.function.Predicate;
@@ -117,6 +118,18 @@ default boolean isFullyChecked(WurstModel model) {
     default void markFullyChecked(WurstModel model) {
     }
 
+    /** Whether every selected unit has been validated since it last changed. */
+    default boolean isChecked(WurstModel model, Collection<CompilationUnit> units) {
+        return isFullyChecked(model);
+    }
+
+    /** Record a successful check without claiming excluded units were checked. */
+    default void markChecked(WurstModel model, Collection<CompilationUnit> units) {
+        if (units.size() == model.size() && units.containsAll(model)) {
+            markFullyChecked(model);
+        }
+    }
+
     static WurstModel copy(WurstModel model) {
         WurstModel m = model.copy();
         // clear all module instantiations, since they might include old stuff

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/ModelManagerImpl.java

@@ -1193,6 +1193,23 @@ public boolean isFullyChecked(WurstModel checkedModel) {
         }
     }
 
+    @Override
+    public boolean isChecked(WurstModel checkedModel, Collection<CompilationUnit> units) {
+        synchronized (modelLock) {
+            return checkedModel != null && checkedModel == model
+                && units.stream().noneMatch(uncheckedUnits::contains);
+        }
+    }
+
+    @Override
+    public void markChecked(WurstModel checkedModel, Collection<CompilationUnit> units) {
+        synchronized (modelLock) {
+            if (checkedModel != null && checkedModel == model) {
+                uncheckedUnits.removeAll(units);
+            }
+        }
+    }
+
     @Override
     public boolean hasErrors() {
         return errorStream().findAny().isPresent();

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/requests/CliBuildMap.java

@@ -29,8 +29,6 @@ public class CliBuildMap extends MapRequest {
     public CliBuildMap(WFile workspaceRoot, Optional<File> map, List<String> compileArgs, Optional<String> wc3Path, WurstGui gui) {
         super(null, map, compileArgs, workspaceRoot, wc3Path, Optional.empty());
         this.gui = gui;
-        this.rebuildModelBeforeCompile = false;
-        this.safeCompilation = SafetyLevel.QuickAndDirty;
     }
 
     @Override

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/requests/MapRequest.java

@@ -115,26 +115,11 @@ protected WurstGui createGui(ModelManager modelManager, String title) {
     /** RunArgs flag (see {@link RunArgs}) that relaxes Jass type checks and skips PJass; injected for pre-1.24 targets. */
     private static final String LEGACY_JASS_CHECKS_ARG = "-legacyJassChecks";
 
-    /**
-     * makes the compilation slower, but more safe by discarding results from the editor and working on a copy of the model
-     */
-    protected SafetyLevel safeCompilation = SafetyLevel.KindOfSafe;
-
-    /**
-     * Whether compiling first cleans and rebuilds the project model. That discards state from the editor,
-     * which a CLI build does not have: it has built the model itself immediately before.
-     */
-    protected boolean rebuildModelBeforeCompile = true;
-
     public TimeTaker getTimeTaker() {
         return timeTaker;
     }
 
 
-    enum SafetyLevel {
-        QuickAndDirty, KindOfSafe
-    }
-
     public static class CompilationResult {
         public File script;
         public File w3i;
@@ -202,14 +187,19 @@ public void handleException(LanguageClient languageClient, Throwable err, Comple
      */
     public static void checkModel(ModelManager modelManager, WurstCompilerJassImpl compiler, WurstModel model,
                                   RunArgs runArgs) {
+        checkModel(modelManager, compiler, model, runArgs, model);
+    }
+
+    private static void checkModel(ModelManager modelManager, WurstCompilerJassImpl compiler, WurstModel model,
+                                   RunArgs runArgs, Collection<CompilationUnit> units) {
         boolean sameChecks = checksLikeTheModelManager(runArgs);
-        if (sameChecks && modelManager.isFullyChecked(model)) {
-            compiler.adoptCheckedModel(model);
+        if (sameChecks && modelManager.isChecked(model, units)) {
+            compiler.adoptCheckedModel(model, units);
             return;
         }
-        boolean validated = compiler.checkProg(model);
+        boolean validated = compiler.checkProg(model, units, units);
         if (sameChecks && validated && compiler.getErrorHandler().getErrorCount() == 0) {
-            modelManager.markFullyChecked(model);
+            modelManager.markChecked(model, units);
         }
     }
 
@@ -229,20 +219,16 @@ protected File compileMap(ModelManager modelManager, File projectFolder, WurstGu
                 // The cached map holds the object data written back by the previous run, so start from the source map's.
                 compiler.setObjectDataSource(map.get());
             }
-            purgeUnimportedFiles(modelManager, model);
-
-            gui.sendProgress("Check program");
-            timeTaker.measure("Typecheck program", () -> checkModel(modelManager, compiler, model, runArgs));
-
-            if (gui.getErrorCount() > 0) {
-                throw new RequestFailedException(MessageType.Warning, "Could not compile project: ", gui.getErrorList().get(0));
-            }
-
-            print("translating program ... ");
-            compiler.translateProgToIm(model);
-
-            if (gui.getErrorCount() > 0) {
-                throw new RequestFailedException(MessageType.Error, "Could not compile project (error in translation): " + gui.getErrorList().get(0));
+            if (model == modelManager.getModel()) {
+                // The language worker owns this model. Finish its mutations and attribute calculations before
+                // honoring cancellation; throwing out of a generated attribute leaves its cache in progress.
+                // From here on compilation mutates private IM, not the shared AST.
+                CompilationCancellation.check();
+                CompilationCancellation.run(() -> false,
+                    () -> prepareImProgram(modelManager, compiler, model, gui, runArgs));
+                CompilationCancellation.check();
+            } else {
+                prepareImProgram(modelManager, compiler, model, gui, runArgs);
             }
 
             if (runArgs.isRunTests()) {
@@ -321,6 +307,26 @@ protected File compileMap(ModelManager modelManager, File projectFolder, WurstGu
         }
     }
 
+    private void prepareImProgram(ModelManager modelManager, WurstCompilerJassImpl compiler, WurstModel model,
+                                  WurstGui gui, RunArgs runArgs) {
+        Set<CompilationUnit> compilationUnits = compilationUnits(model);
+        List<CompilationUnit> units = model.stream().filter(compilationUnits::contains).toList();
+        try (de.peeeq.wurstscript.validation.NamePreservation.SavedMarkers markers = units.size() == model.size()
+            ? null : de.peeeq.wurstscript.validation.NamePreservation.saveSyntheticMarkers(model)) {
+            gui.sendProgress("Check program");
+            timeTaker.measure("Typecheck program", () -> checkModel(modelManager, compiler, model, runArgs, units));
+            if (gui.getErrorCount() > 0) {
+                throw new RequestFailedException(MessageType.Warning, "Could not compile project: ", gui.getErrorList().get(0));
+            }
+            print("translating program ... ");
+            compiler.translateProgToIm(model, units);
+            if (gui.getErrorCount() > 0) {
+                throw new RequestFailedException(MessageType.Error,
+                    "Could not compile project (error in translation): " + gui.getErrorList().get(0));
+            }
+        }
+    }
+
     protected File runJassHotCodeReload(File mapScript) throws IOException, InterruptedException {
         File buildDir = getBuildDir();
         File commonJ = new File(buildDir, "common.j");
@@ -341,12 +347,9 @@ protected File runJassHotCodeReload(File mapScript) throws IOException, Interrup
     }
 
     /**
-     * removes everything compilation unit which is neither
-     * - inside a wurst folder
-     * - a jass file
-     * - imported by a file in a wurst folder
+     * Selects project Wurst sources, the map/core Jass scripts and their imports, without changing the editor model.
      */
-    private void purgeUnimportedFiles(ModelManager modelManager, WurstModel model) {
+    private Set<CompilationUnit> compilationUnits(WurstModel model) {
 
         Set<CompilationUnit> imported = model.stream()
             .filter(cu -> isInProjectWurstFolder(cu.getCuInfo().getFile())
@@ -355,8 +358,7 @@ private void purgeUnimportedFiles(ModelManager modelManager, WurstModel model) {
             .collect(Collectors.toSet());
         addImports(imported, imported);
 
-        // the model may be the managed one (quick and dirty), which only the manager may change
-        modelManager.retainCompilationUnits(model, imported::contains);
+        return imported;
     }
 
     private boolean isProjectWar3MapScript(String file) {
@@ -449,24 +451,14 @@ protected File compileScript(WurstGui gui, ModelManager modelManager, List<Strin
             WLogger.debug("dep: " + dep.getPath());
         }
         print("Dependencies done.");
-        if (safeCompilation != RunMap.SafetyLevel.QuickAndDirty && rebuildModelBeforeCompile) {
-            // it is safer to rebuild the project, instead of taking the current editor state
-            gui.sendProgress("Cleaning project");
-            gui.sendProgress("Building project");
-            // A shared-model rebuild must finish as a unit; cancellation resumes on the
-            // private compilation copy, rather than leaving editor state half initialized.
-            CompilationCancellation.run(() -> false, () -> {
-                timeTaker.measure("Rebuild project", () -> {
-                    modelManager.clean();
-                    modelManager.buildProject();
-                });
-            });
-        }
-
-        timeTaker.beginPhase("Replace map script");
-        replaceBaseScriptWithConfig(modelManager, scriptFile);
-        timeTaker.endPhase();
-        timeTaker.measure("Typecheck project with map script", () -> checkModelWithMapScript(modelManager, gui, runArgs));
+        CompilationCancellation.check();
+        CompilationCancellation.run(() -> false, () -> {
+            timeTaker.beginPhase("Replace map script");
+            replaceBaseScriptWithConfig(modelManager, scriptFile);
+            timeTaker.endPhase();
+            timeTaker.measure("Typecheck project with map script", () -> checkModelWithMapScript(modelManager, gui, runArgs));
+        });
+        CompilationCancellation.check();
 
         if (modelManager.hasErrors()) {
             for (CompileError compileError : modelManager.getParseErrors()) {
@@ -476,14 +468,6 @@ protected File compileScript(WurstGui gui, ModelManager modelManager, List<Strin
         }
 
         WurstModel model = modelManager.getModel();
-        if (safeCompilation != RunMap.SafetyLevel.QuickAndDirty
-            || CompilationCancellation.isScoped()) {
-            // compilation will alter the model (e.g. remove unused imports),
-            // so it is safer to create a copy
-            WurstModel original = model;
-            model = timeTaker.measure("Copy model", () -> ModelManager.copy(original));
-        }
-
         return compileMap(modelManager, modelManager.getProjectPath(), gui, mapCopy, runArgs, model, projectConfigData, isProd);
     }
 

de.peeeq.wurstscript/src/main/java/de/peeeq/wurstio/languageserver/requests/RunMap.java

@@ -48,7 +48,6 @@ public class RunMap extends MapRequest {
     public RunMap(WurstLanguageServer langServer, WFile workspaceRoot, Optional<String> wc3Path, Optional<File> map,
                   List<String> compileArgs, Optional<String> gameExePath) {
         super(langServer, map, compileArgs, workspaceRoot, wc3Path, gameExePath);
-        safeCompilation = SafetyLevel.QuickAndDirty;
     }
 
     @Override