Skip to content

Commit 8c15627

Browse files
eamodioeamodio
committed
Improves commit creation perf
 - Lazy loads "file" lookup - Reduces memory churn in log parsers Ensures file is staged if verifying the staged "SHA"
1 parent 365b62c commit 8c15627

File tree

5 files changed

+131
-69
lines changed

5 files changed

+131
-69
lines changed

‎src/env/node/git/sub-providers/commits.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1229,6 +1229,8 @@ function createCommit(
12291229
currentUser: GitUser | undefined,
12301230
) {
12311231
const message = c.message.trim();
1232+
const index = message.indexOf('\n');
1233+
12321234
return new GitCommit(
12331235
container,
12341236
repoPath,
@@ -1243,13 +1245,13 @@ function createCommit(
12431245
c.committerEmail,
12441246
new Date((c.committerDate as unknown as number) * 1000),
12451247
),
1246-
message.split('\n', 1)[0],
1247-
c.parents ? c.parents.split(' ') : [],
1248+
index !== -1 ? message.substring(0, index) : message,
1249+
c.parents?.split(' ') ?? [],
12481250
message,
12491251
createCommitFileset(container, c, repoPath, pathspec),
12501252
c.stats,
12511253
undefined,
1252-
c.tips ? c.tips.split(' ') : undefined,
1254+
c.tips?.split(' '),
12531255
);
12541256
}
12551257

‎src/env/node/git/sub-providers/revision.ts‎

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,20 @@ export class RevisionGitSubProvider implements GitRevisionSubProvider {
134134
if (isUncommittedWithParentSuffix(ref)) {
135135
ref = 'HEAD';
136136
}
137-
if (isUncommitted(ref)) return { sha: ref, revision: ref };
138-
139137
const relativePath = this.provider.getRelativePath(pathOrUri, repoPath);
140138

139+
if (isUncommitted(ref)) {
140+
if (!isUncommittedStaged(ref)) {
141+
return { sha: ref, revision: ref };
142+
}
143+
144+
// Ensure that the file is actually staged
145+
const status = await this.provider.status.getStatusForFile(repoPath, relativePath, { renames: false });
146+
if (status?.indexStatus) return { sha: ref, revision: ref };
147+
148+
ref = 'HEAD';
149+
}
150+
141151
const parser = getShaAndFileSummaryLogParser();
142152
let result = await this.git.exec(
143153
{ cwd: repoPath, errors: GitErrorHandling.Ignore },

‎src/git/models/commit.ts‎

Lines changed: 51 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,21 @@ import { ensureArray } from '../../system/array';
99
import { formatDate, fromNow } from '../../system/date';
1010
import { gate } from '../../system/decorators/-webview/gate';
1111
import { memoize } from '../../system/decorators/-webview/memoize';
12+
import { Lazy } from '../../system/lazy';
1213
import { getLoggableName } from '../../system/logger';
1314
import { getSettledValue } from '../../system/promise';
1415
import { pluralize } from '../../system/string';
1516
import type { DiffRange, PreviousRangeComparisonUrisResult } from '../gitProvider';
1617
import { GitUri } from '../gitUri';
1718
import type { RemoteProvider } from '../remotes/remoteProvider';
1819
import { getChangedFilesCount } from '../utils/commit.utils';
19-
import { isSha, isUncommitted, isUncommittedStaged, isUncommittedWithParentSuffix } from '../utils/revision.utils';
20+
import {
21+
isSha,
22+
isUncommitted,
23+
isUncommittedStaged,
24+
isUncommittedStagedWithParentSuffix,
25+
isUncommittedWithParentSuffix,
26+
} from '../utils/revision.utils';
2027
import type { GitDiffFileStats } from './diff';
2128
import type { GitFile } from './file';
2229
import { GitFileChange } from './fileChange';
@@ -122,9 +129,9 @@ export class GitCommit implements GitRevisionReference {
122129
return this.container.CommitDateFormatting.dateSource === 'committed' ? this.committer.date : this.author.date;
123130
}
124131

125-
private _file: GitFileChange | undefined;
132+
private _file: Lazy<GitFileChange | undefined> | undefined;
126133
get file(): GitFileChange | undefined {
127-
return this._file;
134+
return this._file?.value;
128135
}
129136

130137
private _fileset: GitCommitFileset | undefined;
@@ -133,7 +140,7 @@ export class GitCommit implements GitRevisionReference {
133140
}
134141
private set fileset(value: GitCommitFileset | undefined) {
135142
if (value == null) {
136-
this._fileset = value;
143+
this._fileset = undefined;
137144
this._file = undefined;
138145
return;
139146
}
@@ -144,41 +151,42 @@ export class GitCommit implements GitRevisionReference {
144151
}
145152
this._fileset = value;
146153

147-
let file;
148-
if (value.pathspec) {
149-
if (value.files.length === 1) {
150-
[file] = value.files;
151-
} else {
152-
let files = value.files.filter(f => f.path === value.pathspec!);
153-
// If we found multiple files with the same path and is uncommitted, then use the existing file if we have one, otherwise use the first
154-
if (files.length > 1) {
155-
if (this.isUncommitted) {
156-
file = this._file ?? files[0];
157-
}
158-
} else if (files.length === 1) {
159-
[file] = files;
154+
const current = this._file?.value;
155+
this._file = new Lazy(() => {
156+
let file;
157+
if (value.pathspec) {
158+
if (value.files.length === 1) {
159+
[file] = value.files;
160160
} else {
161-
files = value.files.filter(f => f.path.startsWith(value.pathspec!));
162-
file = files.length === 1 ? files[0] : undefined;
161+
let files = value.files.filter(f => f.path === value.pathspec!);
162+
// If we found multiple files with the same path and is uncommitted, then use the existing file if we have one, otherwise use the first
163+
if (files.length > 1) {
164+
if (this.isUncommitted) {
165+
file = current ?? files[0];
166+
}
167+
} else if (files.length === 1) {
168+
[file] = files;
169+
} else {
170+
files = value.files.filter(f => f.path.startsWith(value.pathspec!));
171+
file = files.length === 1 ? files[0] : undefined;
172+
}
163173
}
164174
}
165-
}
166175

167-
if (file != null) {
168-
this._file = new GitFileChange(
176+
if (file == null) return undefined;
177+
178+
return new GitFileChange(
169179
this.container,
170180
file.repoPath,
171181
file.path,
172182
file.status,
173-
file.originalPath ?? this._file?.originalPath,
174-
file.previousSha ?? this._file?.previousSha,
175-
file.stats ?? this._file?.stats,
176-
file.staged ?? this._file?.staged,
177-
file.range ?? this._file?.range,
183+
file.originalPath ?? current?.originalPath,
184+
file.previousSha ?? current?.previousSha,
185+
file.stats ?? current?.stats,
186+
file.staged ?? current?.staged,
187+
file.range ?? current?.range,
178188
);
179-
} else {
180-
this._file = undefined;
181-
}
189+
});
182190
}
183191

184192
get formattedDate(): string {
@@ -230,14 +238,18 @@ export class GitCommit implements GitRevisionReference {
230238
this._resolvedPreviousSha ??
231239
(this.file != null ? this.file.previousSha : this.parents[0]) ??
232240
`${this.sha}^`;
233-
return isUncommittedWithParentSuffix(previousSha) ? 'HEAD' : previousSha;
241+
return isUncommittedWithParentSuffix(previousSha)
242+
? isUncommittedStagedWithParentSuffix(previousSha)
243+
? 'HEAD'
244+
: uncommittedStaged
245+
: previousSha;
234246
}
235247

236248
private _etagFileSystem: number | undefined;
237249

238250
hasFullDetails(options?: {
239251
allowFilteredFiles?: boolean;
240-
include?: { stats?: boolean };
252+
include?: { stats?: boolean; uncommittedFiles?: boolean };
241253
}): this is GitCommitWithFullDetails {
242254
if (this.message == null || this.fileset == null) return false;
243255
if (this.fileset.filtered && !options?.allowFilteredFiles) return false;
@@ -259,7 +271,10 @@ export class GitCommit implements GitRevisionReference {
259271
}
260272

261273
@gate()
262-
async ensureFullDetails(options?: { allowFilteredFiles?: boolean; include?: { stats?: boolean } }): Promise<void> {
274+
async ensureFullDetails(options?: {
275+
allowFilteredFiles?: boolean;
276+
include?: { stats?: boolean; uncommittedFiles?: boolean };
277+
}): Promise<void> {
263278
if (this.hasFullDetails(options)) return;
264279

265280
const repo = this.container.git.getRepository(this.repoPath);
@@ -268,7 +283,7 @@ export class GitCommit implements GitRevisionReference {
268283
if (this.isUncommitted) {
269284
this._etagFileSystem = repo?.etagFileSystem;
270285

271-
if (this._etagFileSystem != null) {
286+
if (this._etagFileSystem != null || options?.include?.uncommittedFiles) {
272287
const status = await repo?.git.status().getStatus();
273288
if (status != null) {
274289
let files = status.files.flatMap(f => f.getPseudoFileChanges());
@@ -576,10 +591,10 @@ export class GitCommit implements GitRevisionReference {
576591

577592
@memoize()
578593
getGitUri(previous: boolean = false): GitUri {
579-
const uri = this._file?.uri ?? this.container.git.getAbsoluteUri(this.repoPath, this.repoPath);
594+
const uri = this.file?.uri ?? this.container.git.getAbsoluteUri(this.repoPath, this.repoPath);
580595
if (!previous) return new GitUri(uri, this);
581596

582-
return new GitUri(this._file?.originalUri ?? uri, {
597+
return new GitUri(this.file?.originalUri ?? uri, {
583598
repoPath: this.repoPath,
584599
sha: this.unresolvedPreviousSha,
585600
});

‎src/git/parsers/logParser.ts‎

Lines changed: 57 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -344,36 +344,42 @@ function createLogParserWithFilesAndStats<T extends Record<string, string> | voi
344344

345345
const fileMap = new Map<string, number>(); // Maps path to index in files array
346346

347-
const lines = iterateByDelimiter(content, '\n');
348-
for (const line of lines) {
347+
let fileIndex;
348+
let startIndex;
349+
let endIndex;
350+
351+
let file: LogParsedFileWithStats;
352+
let status;
353+
let additions;
354+
let deletions;
355+
let path;
356+
let originalPath;
357+
358+
for (const line of iterateByDelimiter(content, '\n')) {
349359
if (!line) continue;
350360

351361
if (line.startsWith(' ')) {
352362
if (line.startsWith(' rename ')) {
353-
const { path, originalPath } = parseCopyOrRename(
354-
line.substring(8 /* move past ' rename ' */),
355-
true,
356-
);
357-
const fileIndex = fileMap.get(path);
363+
({ path, originalPath } = parseCopyOrRename(line.substring(8 /* move past ' rename ' */), true));
364+
fileIndex = fileMap.get(path);
358365
if (fileIndex != null) {
359-
const file = files[fileIndex];
366+
file = files[fileIndex];
360367
file.status = 'R';
361368
file.originalPath = originalPath;
362369
} else {
363370
debugger;
364371
}
365372
} else if (line.startsWith(' copy ')) {
366-
const { path, originalPath } = parseCopyOrRename(line.substring(6 /* move past ' copy ' */), true);
367-
const fileIndex = fileMap.get(path);
373+
({ path, originalPath } = parseCopyOrRename(line.substring(6 /* move past ' copy ' */), true));
374+
fileIndex = fileMap.get(path);
368375
if (fileIndex != null) {
369-
const file = files[fileIndex];
376+
file = files[fileIndex];
370377
file.status = 'C';
371378
file.originalPath = originalPath;
372379
} else {
373380
debugger;
374381
}
375382
} else {
376-
let status;
377383
if (line.startsWith(' create mode ')) {
378384
status = 'A';
379385
} else if (line.startsWith(' delete mode ')) {
@@ -386,11 +392,11 @@ function createLogParserWithFilesAndStats<T extends Record<string, string> | voi
386392
continue;
387393
}
388394

389-
const pathIndex = line.indexOf(' ', 13 /* move past 'create mode <num>' or 'delete mode <num>' */);
390-
if (pathIndex > -1) {
391-
const path = line.substring(pathIndex + 1);
395+
startIndex = line.indexOf(' ', 13 /* move past 'create mode <num>' or 'delete mode <num>' */);
396+
if (startIndex > -1) {
397+
const path = line.substring(startIndex + 1);
392398

393-
const fileIndex = fileMap.get(path);
399+
fileIndex = fileMap.get(path);
394400
if (fileIndex != null) {
395401
files[fileIndex].status = status;
396402
} else {
@@ -401,13 +407,29 @@ function createLogParserWithFilesAndStats<T extends Record<string, string> | voi
401407
}
402408
}
403409
} else {
404-
let [additions, deletions, path] = line.split('\t');
410+
startIndex = 0;
411+
endIndex = line.indexOf('\t');
412+
if (endIndex === -1) {
413+
debugger;
414+
}
415+
416+
additions = line.substring(startIndex, endIndex);
417+
418+
startIndex = endIndex + 1;
419+
endIndex = line.indexOf('\t', startIndex);
420+
if (endIndex === -1) {
421+
debugger;
422+
}
423+
424+
deletions = line.substring(startIndex, endIndex);
425+
426+
startIndex = endIndex + 1;
427+
path = line.substring(startIndex);
405428

406-
let originalPath;
407429
// Check for renamed files
408430
({ path, originalPath } = parseCopyOrRename(path, false));
409431

410-
const file: LogParsedFileWithStats = {
432+
file = {
411433
status: originalPath == null ? 'M' : 'R',
412434
path: path.trim(),
413435
originalPath: originalPath?.trim(),
@@ -585,30 +607,37 @@ function createLogParserWithFileSummary<T extends Record<string, string> | void>
585607
const files: LogParsedFile[] = [];
586608
if (!content?.length) return files;
587609

588-
const lines = iterateByDelimiter(content, '\n');
589-
for (const line of lines) {
610+
let startIndex;
611+
612+
let path;
613+
let originalPath;
614+
let status;
615+
616+
for (const line of iterateByDelimiter(content, '\n')) {
590617
if (!line) continue;
591618

592619
if (line.startsWith(' rename ')) {
593-
const { path, originalPath } = parseCopyOrRename(line.substring(8 /* move past ' rename ' */), true);
620+
({ path, originalPath } = parseCopyOrRename(line.substring(8 /* move past ' rename ' */), true));
594621
files.push({ path: path, originalPath: originalPath, status: 'R' });
595622
} else if (line.startsWith(' copy ')) {
596-
const { path, originalPath } = parseCopyOrRename(line.substring(6 /* move past ' copy ' */), true);
623+
({ path, originalPath } = parseCopyOrRename(line.substring(6 /* move past ' copy ' */), true));
597624
files.push({ path: path, originalPath: originalPath, status: 'C' });
598625
} else {
599-
let status;
600626
if (line.startsWith(' create mode ')) {
601627
status = 'A';
602628
} else if (line.startsWith(' delete mode ')) {
603629
status = 'D';
604630
} else {
605-
debugger;
631+
// Ignore " mode change " lines
632+
if (!line.startsWith(' mode change ')) {
633+
debugger;
634+
}
606635
continue;
607636
}
608637

609-
const pathIndex = line.indexOf(' ', 13 /* move past 'create mode <num>' or 'delete mode <num>' */);
610-
if (pathIndex > -1) {
611-
const path = line.substring(pathIndex + 1);
638+
startIndex = line.indexOf(' ', 13 /* move past 'create mode <num>' or 'delete mode <num>' */);
639+
if (startIndex > -1) {
640+
path = line.substring(startIndex + 1);
612641
files.push({ path: path, status: status });
613642
} else {
614643
debugger;

‎src/git/utils/revision.utils.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,12 @@ export function isUncommittedWithParentSuffix(rev: string | undefined): rev is `
5252
return rev === `${uncommitted}^` || rev === `${uncommittedStaged}^`;
5353
}
5454

55+
export function isUncommittedStagedWithParentSuffix(
56+
rev: string | undefined,
57+
): rev is `${typeof uncommitted}${'^' | ':^'}` {
58+
return rev === `${uncommittedStaged}^`;
59+
}
60+
5561
let abbreviatedShaLength = 7;
5662
export function getAbbreviatedShaLength(): number {
5763
return abbreviatedShaLength;

0 commit comments

Comments
 (0)