Skip to content

Commit cccb647

Browse files
author
Test
committed
fix(refDetailsCache): normalize ref hashes and batch Memento writes
getAllRefListExtended emitted %(objectname:short) (7 chars) while VscodeGitProvider.mapRef stored the full 40-char SHA, so the cache's hash-equality check compared different-length hashes for the same commit and every fast-path lookup missed. Normalize the format string to %(objectname), add a defensive prefix-aware hash comparison to tolerate stale short-SHA entries already persisted, and truncate to 7 chars only at the two QuickPick-description render sites that wanted a compact hash. Also rework upsertFromRefs to read Memento state once, apply all upserts in memory, and write once instead of once per ref — avoiding O(N) full-state read-modify-write cycles on activation for large repos. Closes #211
1 parent fd3d513 commit cccb647

6 files changed

Lines changed: 114 additions & 9 deletions

File tree

src/commands/cleanupBranchesCommand/candidates.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ function describeCandidate(candidate: ICleanupCandidate): string {
4747
const relativeDate = ref.committerDate
4848
? formatDistanceToNow(Number(ref.committerDate) * 1000, { addSuffix: true })
4949
: undefined;
50-
const sha = ref.hash;
50+
const sha = ref.hash ? ref.hash.slice(0, 7) : ref.hash;
5151
const parts = [relativeDate, sha].filter((part): part is string => !!part && part.length > 0);
5252

5353
if (group === 'gone') {

src/commands/utils/refFormatting.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,14 @@ export const getRefDescription = (ref: IGitRef) => {
5252
.join(' • ');
5353
};
5454

55+
const SHORT_HASH_LENGTH = 7;
56+
57+
/** Truncates a full-length SHA to a short, compact form for display. */
58+
const shortenHash = (hash: string | undefined): string | undefined =>
59+
hash ? hash.slice(0, SHORT_HASH_LENGTH) : hash;
60+
5561
export const getRefDetails = (ref: IGitRef) => {
56-
return [ref.authorName, ref.hash, ref.comment]
62+
return [ref.authorName, shortenHash(ref.hash), ref.comment]
5763
.filter((part): part is string => !!part && part.trim().length > 0)
5864
.join(' • ');
5965
};

src/common/git/gitExecutor.ts

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,12 @@ export class GitExecutor {
540540
// Separator cannot occur in ref names, hashes, dates, or (practically)
541541
// commit subjects, and Git passes it through the format string verbatim.
542542
const SEPARATOR = '\x1f';
543-
const format = `%(refname)${SEPARATOR}%(objectname:short)${SEPARATOR}%(*objectname:short)${SEPARATOR}%(committerdate:unix)${SEPARATOR}%(*committerdate:unix)${SEPARATOR}%(subject)${SEPARATOR}%(*subject)${SEPARATOR}%(upstream:track)${SEPARATOR}%(authorname)${SEPARATOR}%(*authorname)`;
543+
// Use the full %(objectname) (not :short) so hashes produced here match the
544+
// full 40-char SHAs VscodeGitProvider stores — RefDetailsCache validates
545+
// cache hits by comparing hashes from both producers, and a length
546+
// mismatch there defeats the cache. Short hashes are truncated at render
547+
// time where a compact display is wanted (see refFormatting.ts).
548+
const format = `%(refname)${SEPARATOR}%(objectname)${SEPARATOR}%(*objectname)${SEPARATOR}%(committerdate:unix)${SEPARATOR}%(*committerdate:unix)${SEPARATOR}%(subject)${SEPARATOR}%(*subject)${SEPARATOR}%(upstream:track)${SEPARATOR}%(authorname)${SEPARATOR}%(*authorname)`;
544549
const { stdout: branchesOutput } = await this.#execGitCommand([
545550
'for-each-ref',
546551
'--sort', '-committerdate',

src/services/refDetailsCache.ts

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,7 @@ export class RefDetailsCache {
4747
}
4848

4949
const refHash = this.getRefHash(ref);
50-
if (entry.refHash !== refHash && entry.details.hash !== refHash) {
50+
if (!this.hashesMatch(entry.refHash, refHash) && !this.hashesMatch(entry.details.hash, refHash)) {
5151
return undefined;
5252
}
5353

@@ -84,10 +84,35 @@ export class RefDetailsCache {
8484
});
8585
}
8686

87+
/**
88+
* Upserts many refs in a single Memento read-modify-write cycle instead of
89+
* one per ref — `upsert` alone would read and rewrite the entire cache
90+
* state for every ref, which is O(N) full-state writes for large repos.
91+
*/
8792
async upsertFromRefs(repoKey: string, refs: IGitRef[], now = Date.now()): Promise<void> {
88-
for (const ref of refs) {
89-
await this.upsert(repoKey, ref, ref, now);
93+
if (!this.storage) {
94+
return;
95+
}
96+
97+
const pending = refs
98+
.map((ref) => ({ ref, sanitized: this.sanitize(ref) }))
99+
.filter(({ sanitized }) => Object.keys(sanitized).length > 0);
100+
101+
if (pending.length === 0) {
102+
return;
90103
}
104+
105+
await this.enqueueUpdate(async () => {
106+
const state = this.getState();
107+
for (const { ref, sanitized } of pending) {
108+
state.entries[this.createKey(repoKey, ref)] = {
109+
refHash: this.getRefHash(ref),
110+
details: sanitized,
111+
updatedAt: now,
112+
};
113+
}
114+
await this.updateState(state);
115+
});
91116
}
92117

93118
isMissing(repoKey: string, ref: IGitRef, now = Date.now()): boolean {
@@ -148,6 +173,20 @@ export class RefDetailsCache {
148173
private getRefHash(ref: IGitRef): string {
149174
return ref.hash ?? '';
150175
}
176+
177+
/**
178+
* Compares two commit hashes for cache-validity purposes. Different
179+
* producers may emit different SHA lengths (e.g. legacy short-SHA entries
180+
* persisted before hashes were normalized to full length), so hashes are
181+
* considered equal when one is a non-empty prefix of the other rather than
182+
* requiring an exact string match.
183+
*/
184+
private hashesMatch(a: string | undefined, b: string | undefined): boolean {
185+
if (!a || !b) {
186+
return false;
187+
}
188+
return a.startsWith(b) || b.startsWith(a);
189+
}
151190
}
152191

153192
export function mergeRefDetails(ref: IGitRef, details: Partial<IGitRef>): void {

src/test/unit/cleanupBranches.test.ts

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,10 @@ describe('buildCleanupQuickPickItems', () => {
121121
const gone = items.find((item) => item.candidate?.ref.name === 'orphan');
122122

123123
assert.ok(gone?.description?.includes('not merged — force delete'));
124-
assert.ok(gone?.description?.includes('orphan-sha'));
124+
// The QuickPick description shows a truncated (7-char) SHA for compact
125+
// display; the full SHA is still used for the recovery document (see
126+
// buildRecoveryDocument tests below).
127+
assert.ok(gone?.description?.includes('orphan-'.slice(0, 7)));
125128
});
126129

127130
it('omits a group separator entirely when that group has no candidates', () => {

src/test/unit/refDetailsCache.test.ts

Lines changed: 54 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,15 +4,21 @@ import * as vscode from 'vscode';
44
import { IGitRef } from '../../common/git/types';
55
import { RefDetailsCache, REF_DETAILS_CACHE_TTL_MS } from '../../services/refDetailsCache';
66

7-
function makeMemoryMemento(): Pick<vscode.Memento, 'get' | 'update'> & { values: Map<string, unknown> } {
7+
function makeMemoryMemento(): Pick<vscode.Memento, 'get' | 'update'> & {
8+
values: Map<string, unknown>;
9+
updateCallCount: number;
10+
} {
811
const values = new Map<string, unknown>();
9-
return {
12+
const memento = {
1013
values,
14+
updateCallCount: 0,
1115
get: <T>(key: string) => values.get(key) as T | undefined,
1216
update: async (key: string, value: unknown) => {
17+
memento.updateCallCount += 1;
1318
values.set(key, value);
1419
},
1520
};
21+
return memento;
1622
}
1723

1824
function makeRef(overrides: Partial<IGitRef> = {}): IGitRef {
@@ -165,4 +171,50 @@ describe('RefDetailsCache', () => {
165171
'three'
166172
);
167173
});
174+
175+
it('hits the cache when a short-SHA producer entry is looked up with a full-SHA ref', async () => {
176+
const cache = new RefDetailsCache(makeMemoryMemento());
177+
const fullHash = 'abc1234def5678901234567890123456789012';
178+
const shortHash = fullHash.slice(0, 7);
179+
180+
// Seed via the short-SHA producer path (e.g. getAllRefListExtended before
181+
// the objectname:short -> objectname normalization).
182+
await cache.upsert(
183+
'repo',
184+
makeRef({ hash: shortHash }),
185+
{ comment: 'Seeded via short SHA', authorName: 'A' },
186+
1000
187+
);
188+
189+
// Look up via the full-SHA producer path (e.g. VscodeGitProvider.mapRef).
190+
const result = cache.get('repo', makeRef({ hash: fullHash }), 1000);
191+
192+
assert.deepStrictEqual(result, { comment: 'Seeded via short SHA', authorName: 'A' });
193+
});
194+
195+
it('performs exactly one Memento update when upserting many refs via upsertFromRefs', async () => {
196+
const memento = makeMemoryMemento();
197+
const cache = new RefDetailsCache(memento);
198+
199+
const refs = Array.from({ length: 100 }, (_, i) =>
200+
makeRef({
201+
name: `branch-${i}`,
202+
fullName: `branch-${i}`,
203+
hash: `hash-${i}`,
204+
comment: `Subject ${i}`,
205+
})
206+
);
207+
208+
await cache.upsertFromRefs('repo', refs, 1000);
209+
210+
assert.strictEqual(memento.updateCallCount, 1);
211+
212+
for (let i = 0; i < 100; i++) {
213+
assert.strictEqual(
214+
cache.get('repo', makeRef({ name: `branch-${i}`, fullName: `branch-${i}`, hash: `hash-${i}` }), 1000)
215+
?.comment,
216+
`Subject ${i}`
217+
);
218+
}
219+
});
168220
});

0 commit comments

Comments
 (0)