perf: speed up cache restores (#243)

* fix: find install scripts for package names with regex metacharacters findInstallScript built a regex from the raw package name, so names like g++ or libstdc++6 threw "Nothing to repeat" on cache save and on restore with install scripts, and a '.' in a name could match another package's script. Compare file names as plain strings instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * perf: speed up cache restores - Upload the diagnostic artifact only when debug is on or the action fails. The upload took ~1.4s of a ~3.7s restore in CI. - Extract all package archives in one sudo shell when install scripts are off, instead of a sudo and tar process per package. Per-package extraction stays when scripts run, to keep install ordering (#196). - Skip the apt-fast availability probe on a cache hit, where nothing is installed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * docs: document Manifest and ManifestEntry public members Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
2026-11-13 21:36:56 +00:00 · 2026-11-08 16:44:39 -08:00 · 2026-11-08 16:44:39 -08:00 · 5905c29b85
parent 9ff16f8ab9
commit 5905c29b85
9 changed files with 189 additions and 30 deletions
--- a/action.yml
+++ b/action.yml
@ -29,7 +29,7 @@ inputs:
    required: false
    default: "error"
  debug:
-    description: "Enable debugging when there are issues with action. Minor performance penalty."
+    description: "Enable debugging when there are issues with action. Also uploads the action logs and manifests as a workflow artifact, which is otherwise done only when the action fails. Minor performance penalty."
    required: false
    default: "false"

@ -37,9 +37,15 @@ outputs:
  cache-hit:
    description: "A boolean value to indicate a cache was found for the packages requested."
  package-version-list:
-    description: "The main requested packages and versions that are installed. Represented as a sorted comma delimited list with equals delimit on the package version (i.e. <package1>=<version1>,<package2>=<version2>,...). Packages installed for a non-native architecture include it (i.e. <package>:<arch>=<version>)."
+    description: >
+      The main requested packages and versions that are installed.
+      Represented as a sorted comma delimited list with equals delimit on the package version (i.e. <package1>=<version1>,<package2>=<version2>,...).
+      Packages installed for a non-native architecture include it (i.e. <package>:<arch>=<version>).
  all-package-version-list:
-    description: "All the pulled in packages and versions, including dependencies, that are installed. Represented as a sorted comma delimited list with equals delimit on the package version (i.e. <package1>=<version1>,<package2>=<version2>,...). Packages installed for a non-native architecture include it (i.e. <package>:<arch>=<version>)."
+    description: >
+      All the pulled in packages and versions, including dependencies, that are installed.
+      Represented as a sorted comma delimited list with equals delimit on the package version (i.e. <package1>=<version1>,<package2>=<version2>,...).
+      Packages installed for a non-native architecture include it (i.e. <package>:<arch>=<version>).

 runs:
  using: "node24"
--- a/src/action.ts
+++ b/src/action.ts
@ -95,8 +95,9 @@ export class ActionRunner {
    const cacheHit = manifest !== undefined;

    // Needed on a cache hit too, to resolve the native arch for outputs.
+    // apt-fast only speeds up installs, so skip probing for it on a hit.
    const installManager = await createPackageManager(
-      true,
+      !cacheHit,
      this.logger,
      this.logger,
    );
--- a/src/cache.ts
+++ b/src/cache.ts
@ -31,6 +31,15 @@ export function resolveCacheDir(
 */
 const NO_TIMEOUT_MS = -1;

+/**
+ * Shell script that extracts each archive given as an argument onto `/`.
+ *
+ * Paths are passed as positional arguments, not interpolated, so they are
+ * never parsed by the shell.
+ */
+const EXTRACT_ARCHIVES_SCRIPT =
+  'for archive do tar --extract --file="$archive" --directory=/ --keep-directory-symlink || exit 1; done';
+
 /** Maintainer scripts run on restore when install scripts are enabled. */
 type InstallScriptType = "preinst" | "postinst";

@ -258,12 +267,15 @@ export class Cache {
      }
    }

-    for (const { packageName, archivePath } of archivePaths) {
-      await this.extractArchive(archivePath);
-
-      if (executeInstallScripts) {
+    if (executeInstallScripts) {
+      // Each package's scripts must run before the next package is extracted.
+      for (const { packageName, archivePath } of archivePaths) {
+        await this.extractArchives([archivePath]);
        await this.runInstallScripts(packageName);
      }
+    } else {
+      // One process for all archives avoids a sudo and tar spawn per package.
+      await this.extractArchives(archivePaths.map((item) => item.archivePath));
    }

    // Return every installed package so outputs can report dependencies too.
@ -271,23 +283,23 @@ export class Cache {
  }

  /**
-   * Extracts a package archive onto the root filesystem.
+   * Extracts package archives, in order, onto the root filesystem.
   *
   * Runs system tar under sudo because package files live in root-owned
-   * directories. Fails on any entry error instead of skipping it.
+   * directories. All archives share one sudo shell so the process cost is paid
+   * once. Stops at the first archive with any entry error.
   *
-   * @param archivePath Absolute path to the package archive.
+   * @param archivePaths Absolute paths to the package archives.
   */
-  private async extractArchive(archivePath: string): Promise<void> {
+  private async extractArchives(
+    archivePaths: readonly string[],
+  ): Promise<void> {
+    if (archivePaths.length === 0) {
+      return;
+    }
    await this.commandRunner.run(
      "sudo",
-      [
-        "tar",
-        "--extract",
-        `--file=${archivePath}`,
-        "--directory=/",
-        "--keep-directory-symlink",
-      ],
+      ["sh", "-c", EXTRACT_ARCHIVES_SCRIPT, "sh", ...archivePaths],
      { timeoutMs: NO_TIMEOUT_MS },
    );
  }
--- a/src/index.ts
+++ b/src/index.ts
@ -75,7 +75,9 @@ function getInputs(): ActionInputs {
 async function main(): Promise<void> {
  const cacheDir = CACHE_DEFAULT_DIRNAME;
  const inputs = getInputs();
-  const instruments = new Instruments(cacheDir, inputs.debug || core.isDebug());
+  const debug = inputs.debug || core.isDebug();
+  const instruments = new Instruments(cacheDir, debug);
+  let failed = false;
  try {
    ensureBundledScriptsExecutable();
    const commandRunner = new DefaultCommandRunner(
@ -96,12 +98,16 @@ async function main(): Promise<void> {
  } catch (error) {
    const message = error instanceof Error ? error.message : String(error);
    core.setFailed(message);
+    failed = true;
  } finally {
-    // Logs are diagnostics, so a failed upload shouldn't fail the action.
-    await instruments.artifacts.upload().catch((error: unknown) => {
-      const message = error instanceof Error ? error.message : String(error);
-      core.warning(`Failed to upload diagnostic artifacts: ${message}`);
-    });
+    // The upload takes over a second, so only pay for it when the logs are
+    // wanted. Logs are diagnostics, so a failed upload shouldn't fail the action.
+    if (debug || failed) {
+      await instruments.artifacts.upload().catch((error: unknown) => {
+        const message = error instanceof Error ? error.message : String(error);
+        core.warning(`Failed to upload diagnostic artifacts: ${message}`);
+      });
+    }
  }
 }

--- a/src/manifest.ts
+++ b/src/manifest.ts
@ -2,15 +2,32 @@ import fs from "node:fs";
 import { CacheKey } from "./cache.js";
 import { packageNameFromJSON, PackageName } from "ts-apt";

+/** One installed package and the files archived for it in the cache. */
 export class ManifestEntry {
+  /** Installed package, with the version and arch apt resolved. */
  readonly packageName: PackageName;
+  /**
+   * Files installed by the package, relative to `/` (no leading slash) so they
+   * can be passed straight to tar. Sorted for stable output. Empty for
+   * packages without files (e.g. metapackages), which get no archive.
+   */
  readonly filepaths: string[];

+  /**
+   * @param packageName Installed package.
+   * @param filepaths Root-relative file paths; copied and sorted.
+   */
  constructor(packageName: PackageName, filepaths: string[] = []) {
    this.packageName = packageName;
    this.filepaths = [...filepaths].sort((a, b) => a.localeCompare(b));
  }

+  /**
+   * Creates an entry from its parsed JSON form.
+   *
+   * @param json Parsed JSON object as written by {@link Manifest.writeToFile}.
+   * @returns The deserialized entry.
+   */
  static fromJSON(json: any): ManifestEntry {
    return new ManifestEntry(
      packageNameFromJSON(json.packageName),
@ -19,14 +36,35 @@ export class ManifestEntry {
  }
 }

+/**
+ * Record of the packages stored in a cache, written alongside the archives.
+ *
+ * Restore reads it to find each package's archive and to report outputs.
+ */
 export class Manifest {
+  /**
+   * @param created When the manifest was created.
+   * @param entries Packages in the order apt installed them. Restore relies on
+   *   this order because install scripts can depend on earlier packages
+   *   (see issue #196).
+   * @param cacheKey Key the cache was saved under.
+   */
  constructor(
+    /** When the manifest was created. */
    public readonly created: Date,
+    /** Packages in the order apt installed them. */
    public readonly entries: ManifestEntry[],
+    /** Key the cache was saved under. */
    public readonly cacheKey: CacheKey,
  ) {}

-  // Static factory method
+  /**
+   * Creates a manifest from its parsed JSON form.
+   *
+   * @param json Parsed JSON object as written by {@link Manifest.writeToFile}.
+   *   Its `cacheKey` is the serialized string from {@link CacheKey.toJSON}.
+   * @returns The deserialized manifest.
+   */
  static fromJSON(json: any): Manifest {
    return new Manifest(
      new Date(json.created),
@ -41,11 +79,23 @@ export class Manifest {
    );
  }

+  /**
+   * Reads and parses a manifest file.
+   *
+   * @param filePath Path to a manifest JSON file.
+   * @returns The parsed manifest.
+   * @throws Error when the file can't be read or isn't a valid manifest.
+   */
  static async readFromFile(filePath: string): Promise<Manifest> {
    const content = await fs.promises.readFile(filePath, "utf-8");
    return Manifest.fromJSON(JSON.parse(content));
  }

+  /**
+   * Writes the manifest as pretty-printed JSON, replacing any existing file.
+   *
+   * @param filePath Destination path.
+   */
  async writeToFile(filePath: string): Promise<void> {
    await fs.promises.writeFile(
      filePath,
--- a/src/packages.ts
+++ b/src/packages.ts
@ -66,10 +66,15 @@ export async function findInstallScript(
    return undefined;
  }

-  const pattern = new RegExp(`^${packageName.name}(:.*)?\\.${extension}$`);
+  // Match on plain strings since package names can hold regex metacharacters
+  // (e.g. g++, libstdc++6). Scripts are named <name>[:<arch>].<extension>.
+  const suffix = `.${extension}`;
+  const isMatch = (entry: string) =>
+    entry === `${packageName.name}${suffix}` ||
+    (entry.startsWith(`${packageName.name}:`) && entry.endsWith(suffix));
  const matches = fs
    .readdirSync(scriptsDir)
-    .filter((entry) => pattern.test(entry))
+    .filter(isMatch)
    .sort((a, b) => a.localeCompare(b));
  const candidate = matches[0];
  if (!candidate) {
--- a/test/action-runner.test.ts
+++ b/test/action-runner.test.ts
@ -63,6 +63,28 @@ describe("ActionRunner", () => {
    expect(cache.archiveAndSave).not.toHaveBeenCalled();
  });

+  it("runAction cache hit creates package manager without apt-fast", async () => {
+    const { runner, cache } = createRunner();
+    const packageNames = ActionPackageNames.fromInput("curl");
+    const cacheKey = new CacheKey("v1", "0", process.arch, packageNames);
+    cache.loadAndRestore.mockResolvedValue(
+      new Manifest(new Date(), [], cacheKey),
+    );
+    vi.mocked(createPackageManager).mockResolvedValue({
+      getNativeArchitecture: vi.fn().mockResolvedValue(NATIVE_ARCH),
+    } as never);
+
+    await runner.runAction({
+      packages: "curl",
+      version: "v1",
+      executeInstallScripts: false,
+      emptyPackagesBehavior: "error",
+      debug: false,
+    });
+
+    expect(vi.mocked(createPackageManager).mock.calls[0]?.[0]).toBe(false);
+  });
+
  it("runAction cache miss archives installed packages with their files", async () => {
    // Unlikely to have dpkg install scripts on the test host.
    const installedPackage = createPackageName("test-pkg-not-real", "1.0.0");
--- a/test/cache-restore.test.ts
+++ b/test/cache-restore.test.ts
@ -87,6 +87,8 @@ async function writeCache(archivedPackages = INSTALL_ORDER): Promise<void> {
 interface TarCalls {
  created: [archive: string, files: string[]][];
  extracted: string[];
+  /** Number of commands run that extracted archives. */
+  extractCommands?: number;
 }

 const argValue = (args: string[], flag: string) =>
@ -100,8 +102,10 @@ function recordTar(calls: TarCalls) {
      if (args.includes("--create")) {
        calls.created.push([archive, String(options.input ?? "").split("\0")]);
      }
-      if (args.includes("--extract")) {
-        calls.extracted.push(archive);
+      // Extraction runs as `sh -c <script> sh <archive>...`.
+      if (args[0] === "sh" && args[1] === "-c") {
+        calls.extracted.push(...args.slice(4));
+        calls.extractCommands = (calls.extractCommands ?? 0) + 1;
      }
      return { cmdLine: "", stdout: "", stderr: "", exitCode: 0 };
    },
@ -136,6 +140,27 @@ describe("Cache", () => {
    expect(calls.extracted).toEqual(INSTALL_ORDER.map(archivePath));
  });

+  it("loadAndRestore without install scripts extracts all archives in one command", async () => {
+    await writeCache();
+
+    const calls: TarCalls = { created: [], extracted: [] };
+
+    await createCache(calls).loadAndRestore(KEY, false);
+
+    expect(calls.extractCommands).toBe(1);
+  });
+
+  it("loadAndRestore with install scripts extracts archives one at a time in install order", async () => {
+    await writeCache();
+
+    const calls: TarCalls = { created: [], extracted: [] };
+
+    await createCache(calls).loadAndRestore(KEY, true);
+
+    expect(calls.extracted).toEqual(INSTALL_ORDER.map(archivePath));
+    expect(calls.extractCommands).toBe(INSTALL_ORDER.length);
+  });
+
  it("loadAndRestore archive missing returns undefined without extracting", async () => {
    await writeCache(INSTALL_ORDER.slice(1));

--- a/test/packages.test.ts
+++ b/test/packages.test.ts
@ -98,6 +98,38 @@ describe("packages", () => {
    expect(result).toBe(path.join(scriptsDir, "test-package.preinst"));
  });

+  it.each([
+    ["g++", "g++.postinst"],
+    ["libstdc++6", "libstdc++6:amd64.postinst"],
+  ])(
+    "findInstallScript name %s with regex metacharacters returns its script",
+    async (name, scriptFilename) => {
+      const root = makeTempRoot("pkg-metachar-");
+      const scriptsDir = ensureScriptsDir(root);
+      fs.writeFileSync(path.join(scriptsDir, scriptFilename), "", "utf8");
+
+      const result = await findInstallScript(
+        createPackageName(name),
+        "postinst",
+        root,
+      );
+      expect(result).toBe(path.join(scriptsDir, scriptFilename));
+    },
+  );
+
+  it("findInstallScript name with dot does not match a similarly named package", async () => {
+    const root = makeTempRoot("pkg-dot-");
+    const scriptsDir = ensureScriptsDir(root);
+    fs.writeFileSync(path.join(scriptsDir, "libaXb.postinst"), "", "utf8");
+
+    const result = await findInstallScript(
+      createPackageName("liba.b"),
+      "postinst",
+      root,
+    );
+    expect(result).toBeUndefined();
+  });
+
  it("updateAptLists calls the package manager's update method", async () => {
    const aptListsPath = makeTempRoot("apt-lists-");