Skip to content

Commit edde2b3

Browse files
josephv123josephv123
committed
perf(server): a checkpoint capture starts 8 git processes instead of 10
Every capture looked up the common git dir, whether HEAD resolves and the absolute index path with three separate `git rev-parse` processes. One `rev-parse --git-common-dir --path-format=absolute --git-path index --verify --quiet HEAD` prints all three, so a capture now runs 8 git processes (6 with an unborn HEAD) instead of 10 (7). Each process start blocks the server's event loop for longer as its memory grows: at about 1 GB RSS a capture finished 46-66 ms sooner. When the output is not the expected two paths and HEAD's oid (Git before 2.31 echoes --path-format, and a path with a newline spans lines), the paths are read with the previous commands; a failed lookup still reports the same error.
1 parent e2cc34f commit edde2b3

2 files changed

Lines changed: 174 additions & 11 deletions

File tree

‎apps/server/src/vcs/GitVcsDriver.test.ts‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { ChildProcessSpawner } from "effect/process";
1515
import { assert, it } from "@effect/vitest";
1616

1717
import { CheckpointRef, GitCommandError, VcsProcessExitError } from "@t3tools/contracts";
18+
import * as HostProcess from "@t3tools/shared/HostProcess";
1819
import * as ServerConfig from "../config.ts";
1920
import * as CheckpointStore from "../checkpointing/CheckpointStore.ts";
2021
import * as ProcessRunner from "../processRunner.ts";
@@ -1036,6 +1037,124 @@ it.effect.each(["missing", "invalid"] as const)(
10361037
}).pipe(Effect.scoped, Effect.provide(layerGitContract)),
10371038
);
10381039

1040+
const captureRecordingGitCommands = Effect.fn("test.captureRecordingGitCommands")(function* (
1041+
cwd: string,
1042+
checkpointRef: CheckpointRef,
1043+
) {
1044+
const liveProcess = yield* VcsProcess.VcsProcess;
1045+
const commands: Array<string | undefined> = [];
1046+
const driver = yield* GitVcsDriver.makeVcsDriverShape().pipe(
1047+
Effect.provideService(VcsProcess.VcsProcess, {
1048+
run: (input) => {
1049+
let index = 0;
1050+
while (input.args[index] === "-C" || input.args[index] === "-c") index += 2;
1051+
commands.push(input.args[index]);
1052+
return liveProcess.run(input);
1053+
},
1054+
}),
1055+
);
1056+
yield* driver.checkpoints.captureCheckpoint({ cwd, checkpointRef });
1057+
return commands;
1058+
});
1059+
1060+
it.effect.each(["committed", "unborn"] as const)(
1061+
"checkpoint capture reads the repository with one rev-parse (HEAD=%s)",
1062+
(head) =>
1063+
Effect.gen(function* () {
1064+
const fileSystem = yield* FileSystem.FileSystem;
1065+
const path = yield* Path.Path;
1066+
const driver = yield* GitVcsDriver.makeVcsDriverShape();
1067+
const cwd = yield* fileSystem.makeTempDirectoryScoped({ prefix: "t3-checkpoint-processes-" });
1068+
const git = (args: ReadonlyArray<string>) =>
1069+
driver.execute({ operation: "checkpoint-test", cwd, args });
1070+
const checkpointRef = CheckpointRef.make("refs/t3/checkpoints/test");
1071+
if (head === "committed") {
1072+
yield* makeCheckpointFixture(driver, cwd);
1073+
} else {
1074+
yield* git(["init"]);
1075+
yield* fileSystem.writeFileString(path.join(cwd, "file.txt"), "unstaged\n");
1076+
}
1077+
1078+
const commands = yield* captureRecordingGitCommands(cwd, checkpointRef);
1079+
1080+
assert.deepStrictEqual(commands, [
1081+
"rev-parse",
1082+
"config",
1083+
...(head === "committed" ? ["read-tree", "ls-files"] : []),
1084+
"add",
1085+
"write-tree",
1086+
"commit-tree",
1087+
"update-ref",
1088+
]);
1089+
assert.strictEqual((yield* git(["show", `${checkpointRef}:file.txt`])).stdout, "unstaged\n");
1090+
const [, ...parents] = (yield* git([
1091+
"rev-list",
1092+
"--parents",
1093+
"-n",
1094+
"1",
1095+
checkpointRef,
1096+
])).stdout
1097+
.trim()
1098+
.split(" ");
1099+
assert.deepStrictEqual(
1100+
parents,
1101+
head === "committed" ? [(yield* git(["rev-parse", "HEAD"])).stdout.trim()] : [],
1102+
);
1103+
}).pipe(Effect.scoped, Effect.provide(layerGitContract)),
1104+
);
1105+
1106+
it.effect("checkpoint capture outside a repository fails with the common dir lookup error", () =>
1107+
Effect.gen(function* () {
1108+
const fileSystem = yield* FileSystem.FileSystem;
1109+
const cwd = yield* fileSystem.makeTempDirectoryScoped({ prefix: "t3-checkpoint-no-repo-" });
1110+
1111+
const error = yield* captureRecordingGitCommands(
1112+
cwd,
1113+
CheckpointRef.make("refs/t3/checkpoints/test"),
1114+
).pipe(Effect.flip);
1115+
1116+
assert.instanceOf(error, VcsProcessExitError);
1117+
assert.strictEqual(error.operation, "GitVcsDriver.checkpoints.resolveGitCommonDir");
1118+
assert.strictEqual(error.exitCode, 128);
1119+
}).pipe(Effect.scoped, Effect.provide(layerGitContract)),
1120+
);
1121+
1122+
it.effect.skipIf(HostProcess.Platform.defaultValue() === "win32")(
1123+
"checkpoint capture handles a repository path containing a newline",
1124+
() =>
1125+
Effect.gen(function* () {
1126+
const fileSystem = yield* FileSystem.FileSystem;
1127+
const path = yield* Path.Path;
1128+
const driver = yield* GitVcsDriver.makeVcsDriverShape();
1129+
const cwd = yield* fileSystem.makeTempDirectoryScoped({ prefix: "t3-checkpoint-new\nline-" });
1130+
const { git, checkpointRef } = yield* makeCheckpointFixture(driver, cwd);
1131+
const originalIndex = yield* fileSystem.readFile(path.join(cwd, ".git", "index"));
1132+
1133+
const commands = yield* captureRecordingGitCommands(cwd, checkpointRef);
1134+
1135+
assert.deepStrictEqual(commands, [
1136+
"rev-parse",
1137+
"rev-parse",
1138+
"config",
1139+
"rev-parse",
1140+
"read-tree",
1141+
"ls-files",
1142+
"add",
1143+
"write-tree",
1144+
"commit-tree",
1145+
"update-ref",
1146+
]);
1147+
assert.strictEqual((yield* git(["show", `${checkpointRef}:file.txt`])).stdout, "unstaged\n");
1148+
assert.deepEqual(yield* fileSystem.readFile(path.join(cwd, ".git", "index")), originalIndex);
1149+
assert.deepStrictEqual(
1150+
(yield* fileSystem.readDirectory(path.join(cwd, ".git"))).filter((name) =>
1151+
name.startsWith("t3-checkpoint-index-"),
1152+
),
1153+
[],
1154+
);
1155+
}).pipe(Effect.scoped, Effect.provide(layerGitContract)),
1156+
);
1157+
10391158
it.effect("restores empty checkpoints without changing paths outside the workspace", () =>
10401159
Effect.gen(function* () {
10411160
const fileSystem = yield* FileSystem.FileSystem;

‎apps/server/src/vcs/GitVcsDriver.ts‎

Lines changed: 55 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -769,15 +769,58 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* (
769769
}),
770770
);
771771

772+
const resolveFromCwd = (cwd: string, output: string) => {
773+
const resolved = output.trim();
774+
return path.isAbsolute(resolved) ? resolved : path.resolve(cwd, resolved);
775+
};
776+
772777
const resolveGitCommonDir = (cwd: string) =>
773778
Effect.gen(function* () {
774779
const result = yield* execute({
775780
operation: "GitVcsDriver.checkpoints.resolveGitCommonDir",
776781
cwd,
777782
args: ["rev-parse", "--git-common-dir"],
778783
});
779-
const gitCommonDir = result.stdout.trim();
780-
return path.isAbsolute(gitCommonDir) ? gitCommonDir : path.resolve(cwd, gitCommonDir);
784+
return resolveFromCwd(cwd, result.stdout);
785+
});
786+
787+
// --git-common-dir precedes --path-format, so it prints in its default form. --verify prints
788+
// HEAD's oid after both paths, and with --quiet an unborn HEAD exits 1 with only the paths.
789+
const resolveCaptureRepository = (cwd: string) =>
790+
Effect.gen(function* () {
791+
const result = yield* execute({
792+
operation: "GitVcsDriver.checkpoints.resolveCaptureRepository",
793+
cwd,
794+
args: [
795+
"rev-parse",
796+
"--git-common-dir",
797+
"--path-format=absolute",
798+
"--git-path",
799+
"index",
800+
"--verify",
801+
"--quiet",
802+
"HEAD",
803+
],
804+
allowNonZeroExit: true,
805+
});
806+
const headExists = result.exitCode === 0;
807+
const headChecked = headExists || result.exitCode === 1;
808+
const lines = result.stdout.split("\n");
809+
const [gitCommonDir = "", indexPath = ""] = lines;
810+
if (headChecked && lines.length === (headExists ? 4 : 3) && lines.at(-1) === "") {
811+
return {
812+
gitCommonDir: resolveFromCwd(cwd, gitCommonDir),
813+
indexPath: indexPath.trim(),
814+
headExists,
815+
};
816+
}
817+
// Git before 2.31 echoes --path-format, and a path containing a newline spans lines.
818+
// Separate commands read the paths as before and report a failure the same way.
819+
return {
820+
gitCommonDir: yield* resolveGitCommonDir(cwd),
821+
indexPath: undefined,
822+
headExists: headChecked ? headExists : yield* hasHeadCommit(cwd),
823+
};
781824
});
782825

783826
// Git renames loose objects and refs into place without fsync by default, so
@@ -806,7 +849,7 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* (
806849
"-c",
807850
"splitIndex.sharedIndexExpire=never",
808851
];
809-
const gitCommonDir = yield* resolveGitCommonDir(input.cwd);
852+
const { gitCommonDir, indexPath, headExists } = yield* resolveCaptureRepository(input.cwd);
810853
const indexId = yield* crypto.randomUUIDv4().pipe(Effect.orDie);
811854
const tempIndexPath = path.join(gitCommonDir, `t3-checkpoint-index-${indexId}`);
812855
const commitEnv: NodeJS.ProcessEnv = {
@@ -825,7 +868,6 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* (
825868
);
826869

827870
yield* Effect.gen(function* () {
828-
const headExists = yield* hasHeadCommit(input.cwd);
829871
const sparseConfig = yield* execute({
830872
operation,
831873
cwd: input.cwd,
@@ -844,17 +886,19 @@ export const makeVcsDriverShape = Effect.fn("makeGitVcsDriverShape")(function* (
844886
}
845887
if (headExists) {
846888
const reusedIndex = yield* Effect.gen(function* () {
847-
const indexPath = yield* execute({
848-
operation,
849-
cwd: input.cwd,
850-
args: ["rev-parse", "--path-format=absolute", "--git-path", "index"],
851-
});
852-
const { mtime } = yield* fileSystem.stat(indexPath.stdout.trim());
889+
const userIndexPath =
890+
indexPath ??
891+
(yield* execute({
892+
operation,
893+
cwd: input.cwd,
894+
args: ["rev-parse", "--path-format=absolute", "--git-path", "index"],
895+
})).stdout.trim();
896+
const { mtime } = yield* fileSystem.stat(userIndexPath);
853897
if (Option.isNone(mtime)) return false;
854898
// Stay below the source timestamp even if Date rounded up, preserving Git's racy check.
855899
const indexTime = Math.floor((mtime.value.getTime() - 1) / 1000);
856900
if (indexTime <= 0) return false;
857-
yield* fileSystem.copyFile(indexPath.stdout.trim(), tempIndexPath);
901+
yield* fileSystem.copyFile(userIndexPath, tempIndexPath);
858902
yield* execute({
859903
operation,
860904
cwd: input.cwd,

0 commit comments

Comments
 (0)