Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: optimize git log explore by paginating to minimize traversal #714

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 17 additions & 15 deletions packages/reg-keygen-git-hash-plugin/src/commit-explorer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { CommitExplorer } from "./commit-explorer";
import process from "process";
import path from "path";

const GIT_LOG_COUNT_PER_PAGE_FOR_TEST = 2;

const rimraf = require("rimraf");

process.chdir("./test");
Expand All @@ -21,20 +23,20 @@ const copyGitFiles = (name: string) => {
// assert.equal issue will be fixed in version using git-tiny-walker
// test.serial("no commit", () => {
// copyGitFiles("no-commit");
// t.throws(() => new CommitExplorer().getCurrentCommitHash());
// t.throws(() => new CommitExplorer(GIT_LOG_COUNT_PER_PAGE_FOR_TEST).getCurrentCommitHash());
// });

test("detached head", () => {
copyGitFiles("detached-head");
expect(() => new CommitExplorer().getCurrentBranchName()).toThrowError();
expect(() => new CommitExplorer(GIT_LOG_COUNT_PER_PAGE_FOR_TEST).getCurrentBranchName()).toThrowError();
});

/*
* first commit
*/
test("initial commit", () => {
copyGitFiles("initial-commit");
const baseHash = new CommitExplorer().getBaseCommitHash();
const baseHash = new CommitExplorer(GIT_LOG_COUNT_PER_PAGE_FOR_TEST).getBaseCommitHash();
assert.equal(null, baseHash);
});

Expand All @@ -44,7 +46,7 @@ test("initial commit", () => {
*/
test("master two commits", () => {
copyGitFiles("master-two-commits");
const baseHash = new CommitExplorer().getBaseCommitHash();
const baseHash = new CommitExplorer(GIT_LOG_COUNT_PER_PAGE_FOR_TEST).getBaseCommitHash();
assert.equal(null, baseHash);
});

Expand All @@ -54,7 +56,7 @@ test("master two commits", () => {
*/
test("after create new branch", () => {
copyGitFiles("after-create-new-branch");
const baseHash = new CommitExplorer().getBaseCommitHash();
const baseHash = new CommitExplorer(GIT_LOG_COUNT_PER_PAGE_FOR_TEST).getBaseCommitHash();
assert.equal(null, baseHash);
});

Expand All @@ -65,7 +67,7 @@ test("after create new branch", () => {
*/
test("commit after create new branch", () => {
copyGitFiles("commit-new-branch");
const baseHash = new CommitExplorer().getBaseCommitHash();
const baseHash = new CommitExplorer(GIT_LOG_COUNT_PER_PAGE_FOR_TEST).getBaseCommitHash();
const expected = execSync("git rev-parse expected", { encoding: "utf8" }).trim();
assert.equal(expected, baseHash);
});
Expand All @@ -78,7 +80,7 @@ test("commit after create new branch", () => {
*/
test("two commits after create new branch", () => {
copyGitFiles("two-commit-new-branch");
const baseHash = new CommitExplorer().getBaseCommitHash();
const baseHash = new CommitExplorer(GIT_LOG_COUNT_PER_PAGE_FOR_TEST).getBaseCommitHash();
const expected = execSync("git rev-parse expected", { encoding: "utf8" }).trim();
assert.equal(expected, baseHash);
});
Expand All @@ -95,7 +97,7 @@ test("two commits after create new branch", () => {
*/
test("after catch up master merge", () => {
copyGitFiles("after-catch-up-master");
const baseHash = new CommitExplorer().getBaseCommitHash();
const baseHash = new CommitExplorer(GIT_LOG_COUNT_PER_PAGE_FOR_TEST).getBaseCommitHash();
const expected = execSync("git rev-parse expected", { encoding: "utf8" }).trim();
assert.equal(expected, baseHash);
});
Expand All @@ -113,7 +115,7 @@ test("after catch up master merge", () => {
*/
test("commit after merge", () => {
copyGitFiles("commit-after-merge");
const baseHash = new CommitExplorer().getBaseCommitHash();
const baseHash = new CommitExplorer(GIT_LOG_COUNT_PER_PAGE_FOR_TEST).getBaseCommitHash();
const expected = execSync("git rev-parse expected", { encoding: "utf8" }).trim();
assert.equal(expected, baseHash);
});
Expand All @@ -131,7 +133,7 @@ test("commit after merge", () => {

test("master to catch up branch", () => {
copyGitFiles("master-to-catch-up-branch");
const baseHash = new CommitExplorer().getBaseCommitHash();
const baseHash = new CommitExplorer(GIT_LOG_COUNT_PER_PAGE_FOR_TEST).getBaseCommitHash();
const expected = execSync("git rev-parse expected", { encoding: "utf8" }).trim();
assert.equal(expected, baseHash);
});
Expand All @@ -157,7 +159,7 @@ test("master to catch up branch", () => {

test("commit after catch up and merge", () => {
copyGitFiles("commit-after-catch-up-and-merge");
const baseHash = new CommitExplorer().getBaseCommitHash();
const baseHash = new CommitExplorer(GIT_LOG_COUNT_PER_PAGE_FOR_TEST).getBaseCommitHash();
const expected = execSync("git rev-parse expected", { encoding: "utf8" }).trim();
assert.equal(expected, baseHash);
});
Expand All @@ -175,7 +177,7 @@ test("commit after catch up and merge", () => {
// * first commit
test("after merge catch up", () => {
copyGitFiles("after-merge-catch-up");
const baseHash = new CommitExplorer().getBaseCommitHash();
const baseHash = new CommitExplorer(GIT_LOG_COUNT_PER_PAGE_FOR_TEST).getBaseCommitHash();
const expected = execSync("git rev-parse expected", { encoding: "utf8" }).trim();
assert.equal(expected, baseHash);
});
Expand All @@ -195,7 +197,7 @@ test("after merge catch up", () => {
// * first commit
test("merge catch up and commit", () => {
copyGitFiles("merge-catch-up-then-commit");
const baseHash = new CommitExplorer().getBaseCommitHash();
const baseHash = new CommitExplorer(GIT_LOG_COUNT_PER_PAGE_FOR_TEST).getBaseCommitHash();
const expected = execSync("git rev-parse expected", { encoding: "utf8" }).trim();
assert.equal(expected, baseHash);
});
Expand All @@ -213,14 +215,14 @@ test("merge catch up and commit", () => {
// * (tag: expected) init import
test("merge multipe commit three", () => {
copyGitFiles("merge-multipe-commit-three");
const baseHash = new CommitExplorer().getBaseCommitHash();
const baseHash = new CommitExplorer(GIT_LOG_COUNT_PER_PAGE_FOR_TEST).getBaseCommitHash();
const expected = execSync("git rev-parse expected", { encoding: "utf8" }).trim();
assert.equal(expected, baseHash);
});

test("error patter found in reg-suit repository", () => {
copyGitFiles("reg-suit-error-pattern");
const baseHash = new CommitExplorer().getBaseCommitHash();
const baseHash = new CommitExplorer(GIT_LOG_COUNT_PER_PAGE_FOR_TEST).getBaseCommitHash();
const expected = "49d38a929ae3675a1c79216709c35884f0b78900";
assert.equal(expected, baseHash);
});
48 changes: 35 additions & 13 deletions packages/reg-keygen-git-hash-plugin/src/commit-explorer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,17 +4,23 @@ export type CommitNode = string[];

export class CommitExplorer {
private _gitCmdClient = new GitCmdClient();
private _commitNodes!: CommitNode[];
private _branchName!: string;
private _commitHash!: string;
private _branchHash!: string;
private _branchNameCache: { [hash: string]: string[] } = {};

constructor(private _gitLogCountPerPage = 10) {}

/*
* e.g. return `[["a38df15", "8e1ac3a"], ["8e1ac3a", "7ba8507"]]`.
* The first element of node means commit hash, rest elements means parent commit hashes.
*/
getCommitNodes(): CommitNode[] {
getCommitNodes(skip: number): CommitNode[] {
return this._gitCmdClient
.logGraph()
.logGraph({
number: this._gitLogCountPerPage,
skip,
})
.split("\n")
.map((hashes: string) =>
hashes
Expand Down Expand Up @@ -95,11 +101,11 @@ export class CommitExplorer {
.map(b => b.hash)[0];
}

getCandidateHashes(): string[] {
const mergedBranches = this.getBranchNames(this._commitNodes[0][0]).filter(
getCandidateHashes(commitNotes: ReadonlyArray<CommitNode>): string[] {
const mergedBranches = this.getBranchNames(this._commitHash).filter(
b => !b.endsWith("/" + this._branchName) && b !== this._branchName,
);
return this._commitNodes
return commitNotes
.map(c => c[0])
.filter(c => {
const branches = this.getBranchNames(c);
Expand All @@ -125,15 +131,31 @@ export class CommitExplorer {
return target;
}

getBaseCommitHashRec(cursor: number = 0): string | null {
const commitNotes = this.getCommitNodes(cursor);
if (commitNotes.length === 0) return null;

const candidateHashes = this.getCandidateHashes(commitNotes);
const baseHash = this.findBaseCommitHash(candidateHashes, this._branchHash);
if (!baseHash) {
const lastCommitNode = commitNotes.at(-1)!;
if (lastCommitNode.length === 1) return null;
const nextCursor = cursor + this._gitLogCountPerPage;
if (nextCursor === 300) return null;
return this.getBaseCommitHashRec(nextCursor);
}

const result = this._gitCmdClient.revParse(baseHash).replace("\n", "");
return result ? result : null;
}

getBaseCommitHash(): string | null {
this._branchName = this.getCurrentBranchName();
this._commitNodes = this.getCommitNodes();
const candidateHashes = this.getCandidateHashes();
this._commitHash = this.getCurrentCommitHash();
const branchHash = this.getBranchHash();
if (!branchHash) return null;
const baseHash = this.findBaseCommitHash(candidateHashes, branchHash);
if (!baseHash) return null;
const result = this._gitCmdClient.revParse(baseHash).replace("\n", "");
return result ? result : null;
if (branchHash === undefined) return null;
this._branchHash = branchHash;

return this.getBaseCommitHashRec();
}
}
22 changes: 20 additions & 2 deletions packages/reg-keygen-git-hash-plugin/src/git-cmd-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,26 @@ export class GitCmdClient {
return execSync(shellEscape(["git", "log", "--oneline", `${a}..${b}`]), { encoding: "utf8" });
}

logGraph() {
return execSync('git log -n 300 --graph --pretty=format:"%h %p"', { encoding: "utf8" });
logGraph(
options?: Partial<{
number: number;
skip: number;
}>,
) {
const { number = 300, skip = 0 } = options ?? {};
return execSync(
shellEscape([
"git",
"log",
"-n",
number.toString(),
"--skip",
skip.toString(),
"--graph",
"--pretty=format:%h %p",
]),
{ encoding: "utf8" },
);
}

mergeBase(a: string, b: string) {
Expand Down