NETSCRIPT: Compiled modules will be even more shared (#468)

src/Netscript/NetscriptHelpers.ts

@@ -251,9 +251,7 @@ function makeRuntimeErrorMsg(ctx: NetscriptContext, msg: string, type = "RUNTIME
   const userstack = [];
   for (const stackline of stack) {
     const filename = (() => {
-      // Filename is current file if url found
-      if (ws.scriptRef.url && stackline.includes(ws.scriptRef.url)) return ws.scriptRef.filename;
-      // Also check urls for dependencies
+      // Check urls for dependencies
       for (const [url, script] of ws.scriptRef.dependencies) if (stackline.includes(url)) return script.filename;
       // Check for filenames directly if no URL found
       if (stackline.includes(ws.scriptRef.filename)) return ws.scriptRef.filename;

src/NetscriptFunctions.ts

@@ -900,8 +900,6 @@ export const ns: InternalAPI<NSFull> = {
 
         // Create new script if it does not already exist
         const newScript = new Script(file, sourceScript.code, destServer.hostname);
-        // If the script being copied has no dependencies, reuse the module / URL
-        // The new script will not show up in the correct location in the sources tab because it is just reusing the module from a different server
         destServer.scripts.push(newScript);
         helpers.log(ctx, () => `File '${file}' copied over to '${destServer?.hostname}'.`);
       }

src/NetscriptJSEvaluator.ts

@@ -5,9 +5,9 @@
 import * as walk from "acorn-walk";
 import { parse } from "acorn";
 
-import { Script, ScriptURL } from "./Script/Script";
-import { areImportsEquals } from "./Terminal/DirectoryHelpers";
-import { ScriptModule } from "./Script/ScriptModule";
+import { LoadedModule, ScriptURL, ScriptModule } from "./Script/LoadedModule";
+import { Script } from "./Script/Script";
+import { areImportsEquals, removeLeadingSlash } from "./Terminal/DirectoryHelpers";
 
 // Acorn type def is straight up incomplete so we have to fill with our own.
 export type Node = any;
@@ -17,24 +17,6 @@ function makeScriptBlob(code: string): Blob {
   return new Blob([code], { type: "text/javascript" });
 }
 
-const urlsToRevoke: ScriptURL[] = [];
-let activeCompilations = 0;
-/** Function to queue up revoking of script URLs. If there's no active compilation, just revoke it now. */
-export const queueUrlRevoke = (url: ScriptURL) => {
-  if (!activeCompilations) return URL.revokeObjectURL(url);
-  urlsToRevoke.push(url);
-};
-
-/** Function to revoke any expired urls */
-function triggerURLRevokes() {
-  if (activeCompilations === 0) {
-    // Revoke all pending revoke URLS
-    urlsToRevoke.forEach((url) => URL.revokeObjectURL(url));
-    // Remove all url strings from array
-    urlsToRevoke.length = 0;
-  }
-}
-
 // Webpack likes to turn the import into a require, which sort of
 // but not really behaves like import. So we use a "magic comment"
 // to disable that and leave it as a dynamic import.
@@ -51,50 +33,53 @@ export const config = {
   },
 };
 
+// Maps code to LoadedModules, so we can reuse compiled code across servers,
+// or possibly across files (if someone makes two copies of the same script,
+// or changes a script and then changes it back).
+// Modules can never be garbage collected by Javascript, so it's good to try
+// to keep from making more than we need.
+const moduleCache: Map<string, WeakRef<LoadedModule>> = new Map();
+const cleanup = new FinalizationRegistry((mapKey: string) => {
+  // A new entry can be created with the same key, before this callback is called.
+  if (moduleCache.get(mapKey)?.deref() === undefined) {
+    moduleCache.delete(mapKey);
+  }
+});
+
 export function compile(script: Script, scripts: Script[]): Promise<ScriptModule> {
   // Return the module if it already exists
-  if (script.module) return script.module;
-  // While importing, use an existing url or generate a new one.
-  if (!script.url) script.url = generateScriptUrl(script, scripts, []);
-  activeCompilations++;
-  script.module = config
-    .doImport(script.url)
-    .catch((e) => {
-      script.invalidateModule();
-      console.error(`Error occurred while attempting to compile ${script.filename} on ${script.server}:`);
-      console.error(e);
-      throw e;
-    })
-    .finally(() => {
-      activeCompilations--;
-      triggerURLRevokes();
-    });
-  return script.module;
+  if (script.mod) return script.mod.module;
+
+  script.mod = generateLoadedModule(script, scripts, []);
+  return script.mod.module;
 }
 
 /** Add the necessary dependency relationships for a script.
  * Dependents are used only for passing invalidation up an import tree, so only direct dependents need to be stored.
  * Direct and indirect dependents need to have the current url/script added to their dependency map for error text.
  *
- * This should only be called once the script has an assigned URL. */
-function addDependencyInfo(script: Script, dependents: Script[]) {
-  if (!script.url) throw new Error(`addDependencyInfo called without an assigned script URL (${script.filename})`);
-  if (dependents.length) {
-    script.dependents.add(dependents[dependents.length - 1]);
-    for (const dependent of dependents) dependent.dependencies.set(script.url, script);
+ * This should only be called once the script has a LoadedModule. */
+function addDependencyInfo(script: Script, seenStack: Script[]) {
+  if (!script.mod) throw new Error(`addDependencyInfo called without a LoadedModule (${script.filename})`);
+  if (seenStack.length) {
+    script.dependents.add(seenStack[seenStack.length - 1]);
+    for (const dependent of seenStack) dependent.dependencies.set(script.mod.url, script);
   }
+  // Add self to dependencies (it's not part of the stack, since we don't want
+  // it in dependents.)
+  script.dependencies.set(script.mod.url, script);
 }
 
 /**
  * @param script the script that needs a URL assigned
  * @param scripts array of other scripts on the server
- * @param dependents All scripts that were higher up in the import tree in a recursive call.
+ * @param seenStack A stack of scripts that were higher up in the import tree in a recursive call.
  */
-function generateScriptUrl(script: Script, scripts: Script[], dependents: Script[]): ScriptURL {
+function generateLoadedModule(script: Script, scripts: Script[], seenStack: Script[]): LoadedModule {
   // Early return for recursive calls where the script already has a URL
-  if (script.url) {
-    addDependencyInfo(script, dependents);
-    return script.url;
+  if (script.mod) {
+    addDependencyInfo(script, seenStack);
+    return script.mod;
   }
 
   // Inspired by: https://stackoverflow.com/a/43834063/91401
@@ -145,14 +130,40 @@ function generateScriptUrl(script: Script, scripts: Script[], dependents: Script
     const importedScript = scripts.find((s) => areImportsEquals(s.filename, filename));
     if (!importedScript) continue;
 
-    importedScript.url = generateScriptUrl(importedScript, scripts, [...dependents, script]);
-    newCode = newCode.substring(0, node.start) + importedScript.url + newCode.substring(node.end);
+    seenStack.push(script);
+    importedScript.mod = generateLoadedModule(importedScript, scripts, seenStack);
+    seenStack.pop();
+    newCode = newCode.substring(0, node.start) + importedScript.mod.url + newCode.substring(node.end);
   }
 
-  newCode += `\n//# sourceURL=${script.server}/${script.filename}`;
+  const cachedMod = moduleCache.get(newCode)?.deref();
+  if (cachedMod) {
+    script.mod = cachedMod;
+  } else {
+    // Add an inline source-map to make debugging nicer. This won't be right
+    // in all cases, since we can share the same script across multiple
+    // servers; it will be listed under the first server it was compiled for.
+    // We don't include this in the cache key, so that other instances of the
+    // script dedupe properly.
+    const adjustedCode = newCode + `\n//# sourceURL=${script.server}/${removeLeadingSlash(script.filename)}`;
+    // At this point we have the full code and can construct a new blob / assign the URL.
+    const url = URL.createObjectURL(makeScriptBlob(adjustedCode)) as ScriptURL;
+    const module = config.doImport(url).catch((e) => {
+      script.invalidateModule();
+      console.error(`Error occurred while attempting to compile ${script.filename} on ${script.server}:`);
+      console.error(e);
+      throw e;
+    }) as Promise<ScriptModule>;
+    // We can *immediately* invalidate the Blob, because we've already started the fetch
+    // by starting the import. From now on, any imports using the blob's URL *must*
+    // directly return the module, without even attempting to fetch, due to the way
+    // modules work.
+    URL.revokeObjectURL(url);
+    script.mod = new LoadedModule(url, module);
+    moduleCache.set(newCode, new WeakRef(script.mod));
+    cleanup.register(script.mod, newCode);
+  }
 
-  // At this point we have the full code and can construct a new blob / assign the URL.
-  script.url = URL.createObjectURL(makeScriptBlob(newCode)) as ScriptURL;
-  addDependencyInfo(script, dependents);
-  return script.url;
+  addDependencyInfo(script, seenStack);
+  return script.mod;
 }

src/Prestige.ts

@@ -192,7 +192,7 @@ export function prestigeSourceFile(flume: boolean): void {
   AddToAllServers(homeComp);
   prestigeHomeComputer(homeComp);
   // Ram usage needs to be cleared for bitnode-level resets, due to possible change in singularity cost.
-  for (const script of homeComp.scripts) script.ramUsage = undefined;
+  for (const script of homeComp.scripts) script.ramUsage = null;
 
   // Re-create foreign servers
   initForeignServers(Player.getHomeComputer());

src/Script/LoadedModule.ts

@@ -0,0 +1,21 @@
+import { NSFull } from "../NetscriptFunctions";
+import { AutocompleteData } from "@nsdefs";
+
+// The object portion of this type is not runtime information, it's only to ensure type validation
+// And make it harder to overwrite a url with a random non-url string.
+export type ScriptURL = string & { __type: "ScriptURL" };
+
+export interface ScriptModule {
+  main?: (ns: NSFull) => unknown;
+  autocomplete?: (data: AutocompleteData, flags: string[]) => unknown;
+}
+
+export class LoadedModule {
+  url: ScriptURL;
+  module: Promise<ScriptModule>;
+
+  constructor(url: ScriptURL, module: Promise<ScriptModule>) {
+    this.url = url;
+    this.module = module;
+  }
+}

src/Script/RunningScript.ts

@@ -3,7 +3,8 @@
  * A Script can have multiple active instances
  */
 import type React from "react";
-import { Script, ScriptURL } from "./Script";
+import { Script } from "./Script";
+import { ScriptURL } from "./LoadedModule";
 import { Settings } from "../Settings/Settings";
 import { Terminal } from "../Terminal";
 
@@ -67,7 +68,6 @@ export class RunningScript {
 
   // Script urls for the current running script for translating urls back to file names in errors
   dependencies: Map<ScriptURL, Script> = new Map();
-  url?: ScriptURL;
 
   constructor(script?: Script, ramUsage?: number, args: ScriptArg[] = []) {
     if (!script) return;
@@ -76,7 +76,7 @@ export class RunningScript {
     this.args = args;
     this.server = script.server;
     this.ramUsage = ramUsage;
-    this.dependencies = new Map(script.dependencies);
+    this.dependencies = script.dependencies;
   }
 
   log(txt: React.ReactNode): void {

src/Script/Script.ts

@@ -5,32 +5,30 @@
  * being evaluated. See RunningScript for that
  */
 import { calculateRamUsage, RamUsageEntry } from "./RamCalculations";
+import { LoadedModule, ScriptURL } from "./LoadedModule";
 
 import { Generic_fromJSON, Generic_toJSON, IReviverValue, constructorsForReviver } from "../utils/JSONReviver";
 import { roundToTwo } from "../utils/helpers/roundToTwo";
-import { ScriptModule } from "./ScriptModule";
 import { RamCostConstants } from "../Netscript/RamCostGenerator";
-import { queueUrlRevoke } from "../NetscriptJSEvaluator";
-
-// The object portion of this type is not runtime information, it's only to ensure type validation
-// And make it harder to overwrite a url with a random non-url string.
-export type ScriptURL = string & { __type: "ScriptURL" };
 
 export class Script {
-  code = "";
-  filename = "default.js";
-  server = "home";
+  code: string;
+  filename: string;
+  server: string;
 
   // Ram calculation, only exists after first poll of ram cost after updating
-  ramUsage?: number;
-  ramUsageEntries?: RamUsageEntry[];
+  ramUsage: number | null = null;
+  ramUsageEntries: RamUsageEntry[] = [];
 
   // Runtime data that only exists when the script has been initiated. Cleared when script or a dependency script is updated.
-  module?: Promise<ScriptModule>;
-  url?: ScriptURL;
+  mod: LoadedModule | null = null;
   /** Scripts that directly import this one. Stored so we can invalidate these dependent scripts when this one is invalidated. */
   dependents: Set<Script> = new Set();
-  /** Scripts that are imported by this one, either directly or through an import chain */
+  /**
+   * Scripts that we directly or indirectly import, including ourselves.
+   * Stored only so RunningScript can use it, to translate urls in error messages.
+   * Because RunningScript uses the reference directly (to reduce object copies), it must be immutable.
+   */
   dependencies: Map<ScriptURL, Script> = new Map();
 
   constructor(fn = "", code = "", server = "") {
@@ -58,16 +56,16 @@ export class Script {
   /** Invalidates the current script module and related data, e.g. when modifying the file. */
   invalidateModule(): void {
     // Always clear ram usage
-    this.ramUsage = undefined;
-    this.ramUsageEntries = undefined;
+    this.ramUsage = null;
+    this.ramUsageEntries.length = 0;
     // Early return if there's already no URL
-    if (!this.url) return;
-    this.module = undefined;
-    queueUrlRevoke(this.url);
-    this.url = undefined;
-    for (const dependency of this.dependencies.values()) dependency.dependents.delete(this);
-    this.dependencies.clear();
+    if (!this.mod) return;
+    this.mod = null;
     for (const dependent of this.dependents) dependent.invalidateModule();
+    this.dependents.clear();
+    // This will be mutated in compile(), but is immutable after that.
+    // (No RunningScripts can access this copy before that point).
+    this.dependencies = new Map();
   }
 
   /**
@@ -86,7 +84,7 @@ export class Script {
   getRamUsage(otherScripts: Script[]): number | null {
     if (this.ramUsage) return this.ramUsage;
     this.updateRamUsage(otherScripts);
-    return this.ramUsage ?? null;
+    return this.ramUsage;
   }
 
   /**
@@ -97,12 +95,10 @@ export class Script {
     const ramCalc = calculateRamUsage(this.code, otherScripts);
     if (ramCalc.cost >= RamCostConstants.Base) {
       this.ramUsage = roundToTwo(ramCalc.cost);
-      this.ramUsageEntries = ramCalc.entries;
-    } else delete this.ramUsage;
-  }
-
-  imports(): string[] {
-    return [];
+      this.ramUsageEntries = ramCalc.entries as RamUsageEntry[];
+    } else {
+      this.ramUsage = null;
+    }
   }
 
   /** The keys that are relevant in a save file */

src/Script/ScriptModule.ts

@@ -1,7 +0,0 @@
-import { NSFull } from "../NetscriptFunctions";
-import { AutocompleteData } from "@nsdefs";
-
-export interface ScriptModule {
-  main?: (ns: NSFull) => unknown;
-  autocomplete?: (data: AutocompleteData, flags: string[]) => unknown;
-}

test/jest/Netscript/RunScript.test.ts

@@ -99,7 +99,11 @@ test.each([
     const result = await Promise.race([
       alerted,
       new Promise((resolve) => {
-        eventDelete = WorkerScriptStartStopEventEmitter.subscribe(() => resolve(null));
+        eventDelete = WorkerScriptStartStopEventEmitter.subscribe(() => {
+          if (!server.runningScripts.includes(runningScript)) {
+            resolve(null);
+          }
+        });
       }),
     ]);
     // If an error alert was thrown, we catch it here.

tsconfig.json

@@ -14,7 +14,7 @@
     "skipLibCheck": true,
     "sourceMap": true,
     "strict": true,
-    "target": "es2019"
+    "target": "es2021"
   },
   "include": ["src/**/*", "electron/**/*", ".eslintrc.js", "node_modules/monaco-editor/monaco.d.ts"]
 }