Skip to content

Commit

Permalink
Faster multiple file staging (git-up#629)
Browse files Browse the repository at this point in the history
* Improve performance when staging/unstaging multiple files at once

* Optimize multiple files discarding

* Run ClangFormat

* Changes following PR feedback

* Fix bug caused by bad merge and add comments

The selectedFiles array would always be empty because the `else` should've been outside the `if (submodule)`

* Correctly allocate and free multiple path strings
  • Loading branch information
lapfelix authored and simpzan committed Oct 22, 2020
1 parent 7cdf693 commit bd1775a
Show file tree
Hide file tree
Showing 9 changed files with 213 additions and 41 deletions.
14 changes: 12 additions & 2 deletions GitUpKit/Core/GCDiff-Tests.m
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,16 @@ - (void)testUnifiedDiff {
XCTAssertTrue([self.repository addFileToIndex:@"renamed1.txt" error:NULL]);
[self updateFileAtPath:@"type-changed.txt" withString:@""];
XCTAssertTrue([self.repository addFileToIndex:@"type-changed.txt" error:NULL]);

NSArray* files = @[ @".gitignore", @"modified.txt", @"deleted.txt", @"renamed1.txt", @"type-changed.txt" ];

// Test adding and removing multiple files
XCTAssertTrue([self.repository removeFilesFromIndex:files error:NULL]);
XCTAssertEqual([self.repository checkIndexStatus:NULL].deltas.count, 0);

XCTAssertTrue([self.repository addFilesToIndex:files error:NULL]);
XCTAssertEqual([self.repository checkIndexStatus:NULL].deltas.count, 5);

XCTAssertNotNil([self.repository createCommitFromHEADWithMessage:@"Update" error:NULL]);

// Touch files
Expand All @@ -179,8 +189,8 @@ - (void)testUnifiedDiff {

// Stage some files
XCTAssertTrue([self.repository addFileToIndex:@"modified.txt" error:NULL]);
XCTAssertTrue([self.repository removeFileFromIndex:@"deleted.txt" error:NULL]);
XCTAssertTrue([self.repository removeFileFromIndex:@"renamed1.txt" error:NULL]);
files = @[ @"deleted.txt", @"renamed1.txt" ];
XCTAssertTrue([self.repository removeFilesFromIndex:files error:NULL]);
XCTAssertTrue([self.repository addFileToIndex:@"renamed2.txt" error:NULL]);
XCTAssertTrue([self.repository addFileToIndex:@"added.txt" error:NULL]);

Expand Down
1 change: 1 addition & 0 deletions GitUpKit/Core/GCIndex.h
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ typedef BOOL (^GCIndexLineFilter)(GCLineDiffChange change, NSUInteger oldLineNum
- (BOOL)resetLinesInFile:(NSString*)path index:(GCIndex*)index toCommit:(GCCommit*)commit error:(NSError**)error usingFilter:(GCIndexLineFilter)filter;

- (BOOL)checkoutFileToWorkingDirectory:(NSString*)path fromIndex:(GCIndex*)index error:(NSError**)error;
- (BOOL)checkoutFilesToWorkingDirectory:(NSArray<NSString*>*)paths fromIndex:(GCIndex*)index error:(NSError**)error;
- (BOOL)checkoutLinesInFileToWorkingDirectory:(NSString*)path fromIndex:(GCIndex*)index error:(NSError**)error usingFilter:(GCIndexLineFilter)filter;

- (BOOL)clearConflictForFile:(NSString*)path inIndex:(GCIndex*)index error:(NSError**)error;
Expand Down
18 changes: 15 additions & 3 deletions GitUpKit/Core/GCIndex.m
Original file line number Diff line number Diff line change
Expand Up @@ -485,12 +485,24 @@ - (BOOL)resetLinesInFile:(NSString*)path index:(GCIndex*)index toCommit:(GCCommi
}

- (BOOL)checkoutFileToWorkingDirectory:(NSString*)path fromIndex:(GCIndex*)index error:(NSError**)error {
return [self checkoutFilesToWorkingDirectory:@[ path ]
fromIndex:index
error:error];
}

- (BOOL)checkoutFilesToWorkingDirectory:(NSArray<NSString*>*)paths fromIndex:(GCIndex*)index error:(NSError**)error {
git_checkout_options options = GIT_CHECKOUT_OPTIONS_INIT;
options.checkout_strategy = GIT_CHECKOUT_FORCE | GIT_CHECKOUT_DONT_UPDATE_INDEX | GIT_CHECKOUT_DISABLE_PATHSPEC_MATCH; // There's no reason to update the index
options.paths.count = 1;
const char* filePath = GCGitPathFromFileSystemPath(path);
options.paths.strings = (char**)&filePath;
options.paths.count = paths.count;
char** pathStrings = malloc(paths.count * sizeof(char*));
options.paths.strings = pathStrings;
for (NSUInteger i = 0; i < paths.count; i++) {
const char* filePath = GCGitPathFromFileSystemPath(paths[i]);
options.paths.strings[i] = (char*)filePath;
}

CALL_LIBGIT2_FUNCTION_RETURN(NO, git_checkout_index, self.private, index.private, &options);
free(pathStrings);
return YES;
}

Expand Down
26 changes: 25 additions & 1 deletion GitUpKit/Extensions/GCRepository+Index-Tests.m
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,33 @@ - (void)testIndex {
XCTAssertTrue([self.repository addFileToIndex:@"test.txt" error:NULL]);
[self assertGitCLTOutputEqualsString:@"A test.txt\n" withRepository:self.repository command:@"status", @"--ignored", @"--porcelain", nil];

// Add multiple files to working directory
NSMutableArray* filePaths = [[NSMutableArray alloc] init];
NSString* expectedGitCLTOutput = [[NSString alloc] init];
for (int i = 0; i < 50; i++) {
NSString* filePath = [NSString stringWithFormat:@"hello_world%02d.txt", i];
[self updateFileAtPath:filePath withString:@"Bonjour le monde!\n"];
[filePaths addObject:filePath];
expectedGitCLTOutput = [expectedGitCLTOutput stringByAppendingFormat:@"A %@\n", filePath];
}
expectedGitCLTOutput = [expectedGitCLTOutput stringByAppendingString:@"A test.txt\n"];

// Add multiple files to index
XCTAssertTrue([self.repository addFilesToIndex:filePaths error:NULL]);
[self assertGitCLTOutputEqualsString:expectedGitCLTOutput withRepository:self.repository command:@"status", @"--ignored", @"--porcelain", nil];

// Add remove multiple files from index
XCTAssertTrue([self.repository removeFilesFromIndex:filePaths error:NULL]);
expectedGitCLTOutput = [expectedGitCLTOutput stringByReplacingOccurrencesOfString:@"A test.txt\n" withString:@""];
expectedGitCLTOutput = [expectedGitCLTOutput stringByReplacingOccurrencesOfString:@"A hello_world" withString:@"?? hello_world"];
expectedGitCLTOutput = [@"A test.txt\n" stringByAppendingString:expectedGitCLTOutput];
[self assertGitCLTOutputEqualsString:expectedGitCLTOutput withRepository:self.repository command:@"status", @"--ignored", @"--porcelain", nil];

// Reset index
XCTAssertTrue([self.repository resetIndexToHEAD:NULL]);
[self assertGitCLTOutputEqualsString:@"?? test.txt\n" withRepository:self.repository command:@"status", @"--ignored", @"--porcelain", nil];
expectedGitCLTOutput = [expectedGitCLTOutput stringByReplacingOccurrencesOfString:@"A test.txt\n" withString:@""];
expectedGitCLTOutput = [expectedGitCLTOutput stringByAppendingString:@"?? test.txt\n"];
[self assertGitCLTOutputEqualsString:expectedGitCLTOutput withRepository:self.repository command:@"status", @"--ignored", @"--porcelain", nil];
}

- (void)testIndex_Lines {
Expand Down
4 changes: 4 additions & 0 deletions GitUpKit/Extensions/GCRepository+Index.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,14 @@
- (BOOL)resetIndexToHEAD:(NSError**)error; // Like git reset --mixed HEAD but does not update reflog

- (BOOL)removeFileFromIndex:(NSString*)path error:(NSError**)error; // git rm --cached {file} - Delete file from index
- (BOOL)removeFilesFromIndex:(NSArray<NSString*>*)paths error:(NSError**)error; // git rm --cached {file} - Delete files from index

- (BOOL)addFileToIndex:(NSString*)path error:(NSError**)error; // git add {file} - Copy file from workdir to index (aka stage file)
- (BOOL)addFilesToIndex:(NSArray<NSString*>*)paths error:(NSError**)error;
- (BOOL)resetFileInIndexToHEAD:(NSString*)path error:(NSError**)error; // git reset --mixed {file} - Copy file from HEAD to index (aka unstage file)
- (BOOL)resetFilesInIndexToHEAD:(NSArray<NSString*>*)paths error:(NSError**)error;
- (BOOL)checkoutFileFromIndex:(NSString*)path error:(NSError**)error; // git checkout {file} - Copy file from index to workdir (aka discard file)
- (BOOL)checkoutFilesFromIndex:(NSArray<NSString*>*)paths error:(NSError**)error;

- (BOOL)addLinesFromFileToIndex:(NSString*)path error:(NSError**)error usingFilter:(GCIndexLineFilter)filter; // git add -p {file} - Copy only some lines of file from workdir to index (aka stage lines)
- (BOOL)resetLinesFromFileInIndexToHEAD:(NSString*)path error:(NSError**)error usingFilter:(GCIndexLineFilter)filter; // git reset -p {file} - Copy only some lines of file from HEAD to index (aka unstage lines)
Expand Down
69 changes: 59 additions & 10 deletions GitUpKit/Extensions/GCRepository+Index.m
Original file line number Diff line number Diff line change
Expand Up @@ -39,22 +39,61 @@ - (BOOL)resetIndexToHEAD:(NSError**)error {
}

- (BOOL)removeFileFromIndex:(NSString*)path error:(NSError**)error {
return [self removeFilesFromIndex:@[ path ] error:error];
}

- (BOOL)removeFilesFromIndex:(NSArray<NSString*>*)paths error:(NSError**)error {
GCIndex* index = [self readRepositoryIndex:error];
if (index == nil) {
return NO;
}
return [self removeFile:path fromIndex:index error:error] && [self writeRepositoryIndex:index error:error];

for (NSString* path in paths) {
if (![self removeFile:path fromIndex:index error:error] || (error && *error != nil)) {
[self writeRepositoryIndex:index error:error];
return NO;
}
}

return [self writeRepositoryIndex:index error:error];
}

- (BOOL)addFileToIndex:(NSString*)path error:(NSError**)error {
return [self addFilesToIndex:@[ path ] error:error];
}

- (BOOL)addFilesToIndex:(NSArray<NSString*>*)paths error:(NSError**)error {
GCIndex* index = [self readRepositoryIndex:error];
if (index == nil) {
return NO;
}
return [self addFileInWorkingDirectory:path toIndex:index error:error] && [self writeRepositoryIndex:index error:error];

BOOL failed = NO;
BOOL shouldWriteRepository = NO;
for (NSString* path in paths) {
if (![self addFileInWorkingDirectory:path toIndex:index error:error] || (error && *error != nil)) {
failed = YES;
break;
}

shouldWriteRepository = YES;
}

if (failed && shouldWriteRepository) {
if (shouldWriteRepository) {
[self writeRepositoryIndex:index error:NULL];
}
return NO;
}

return [self writeRepositoryIndex:index error:error];
}

- (BOOL)resetFileInIndexToHEAD:(NSString*)path error:(NSError**)error {
return [self resetFilesInIndexToHEAD:@[ path ] error:error];
}

- (BOOL)resetFilesInIndexToHEAD:(NSArray<NSString*>*)paths error:(NSError**)error {
GCIndex* index = [self readRepositoryIndex:error];
if (index == nil) {
return NO;
Expand All @@ -63,24 +102,34 @@ - (BOOL)resetFileInIndexToHEAD:(NSString*)path error:(NSError**)error {
if (![self lookupHEADCurrentCommit:&headCommit branch:NULL error:error]) {
return NO;
}
if (headCommit) {
if (![self resetFile:path inIndex:index toCommit:headCommit error:error]) {
return NO;
}
} else {
if (![self removeFile:path fromIndex:index error:error]) {
return NO;

for (NSString* path in paths) {
if (headCommit) {
if (![self resetFile:path inIndex:index toCommit:headCommit error:error]) {
[self writeRepositoryIndex:index error:error];
return NO;
}
} else {
if (![self removeFile:path fromIndex:index error:error]) {
[self writeRepositoryIndex:index error:error];
return NO;
}
}
}

return [self writeRepositoryIndex:index error:error];
}

- (BOOL)checkoutFileFromIndex:(NSString*)path error:(NSError**)error {
return [self checkoutFilesFromIndex:@[ path ] error:error];
}

- (BOOL)checkoutFilesFromIndex:(NSArray<NSString*>*)paths error:(NSError**)error {
GCIndex* index = [self readRepositoryIndex:error];
if (index == nil) {
return NO;
}
return [self checkoutFileToWorkingDirectory:path fromIndex:index error:error];
return [self checkoutFilesToWorkingDirectory:paths fromIndex:index error:error];
}

- (BOOL)addLinesFromFileToIndex:(NSString*)path error:(NSError**)error usingFilter:(GCIndexLineFilter)filter {
Expand Down
3 changes: 3 additions & 0 deletions GitUpKit/Utilities/GIViewController+Utilities.h
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,14 @@ extern NSString* const GIViewController_TerminalTool_iTerm;
- (void)discardSubmoduleAtPath:(NSString*)path resetIndex:(BOOL)resetIndex; // Prompts user

- (void)stageAllChangesForFile:(NSString*)path;
- (void)stageAllChangesForFiles:(NSArray<NSString*>*)paths;
- (void)stageSelectedChangesForFile:(NSString*)path oldLines:(NSIndexSet*)oldLines newLines:(NSIndexSet*)newLines;
- (void)unstageAllChangesForFile:(NSString*)path;
- (void)unstageAllChangesForFiles:(NSArray<NSString*>*)paths;
- (void)unstageSelectedChangesForFile:(NSString*)path oldLines:(NSIndexSet*)oldLines newLines:(NSIndexSet*)newLines;

- (BOOL)discardAllChangesForFile:(NSString*)path resetIndex:(BOOL)resetIndex error:(NSError**)error;
- (BOOL)discardAllChangesForFiles:(NSArray<NSString*>*)paths resetIndex:(BOOL)resetIndex error:(NSError**)error;
- (void)discardAllChangesForFile:(NSString*)path resetIndex:(BOOL)resetIndex; // Prompts user
- (BOOL)discardSelectedChangesForFile:(NSString*)path oldLines:(NSIndexSet*)oldLines newLines:(NSIndexSet*)newLines resetIndex:(BOOL)resetIndex error:(NSError**)error;
- (void)discardSelectedChangesForFile:(NSString*)path oldLines:(NSIndexSet*)oldLines newLines:(NSIndexSet*)newLines resetIndex:(BOOL)resetIndex; // Prompts user
Expand Down
71 changes: 57 additions & 14 deletions GitUpKit/Utilities/GIViewController+Utilities.m
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ + (void)initialize {
GIViewController_TerminalTool : GIViewController_TerminalTool_Terminal,
};
[[NSUserDefaults standardUserDefaults] registerDefaults:defaults];

NSDictionary* installedApps = [GILaunchServicesLocator installedAppsDictionary];
[[NSUserDefaults standardUserDefaults] registerDefaults:installedApps];

Expand Down Expand Up @@ -198,13 +198,34 @@ - (void)discardSubmoduleAtPath:(NSString*)path resetIndex:(BOOL)resetIndex {
}

- (void)stageAllChangesForFile:(NSString*)path {
return [self stageAllChangesForFiles:@[ path ]];
}

- (void)stageAllChangesForFiles:(NSArray<NSString*>*)paths {
NSError* error;
BOOL fileExists = [[NSFileManager defaultManager] fileExistsAtPath:[self.repository absolutePathForFile:path] followLastSymlink:NO];
if ((fileExists && [self.repository addFileToIndex:path error:&error]) || (!fileExists && [self.repository removeFileFromIndex:path error:&error])) {
[self.repository notifyRepositoryChanged];
} else {
[self presentError:error];
NSMutableArray* existingFiles = [NSMutableArray array];
NSMutableArray* nonExistingFiles = [NSMutableArray array];
for (NSString* path in paths) {
if ([[NSFileManager defaultManager] fileExistsAtPath:[self.repository absolutePathForFile:path]]) {
[existingFiles addObject:path];
} else {
[nonExistingFiles addObject:path];
}
}

if (existingFiles.count > 0) {
if (![self.repository addFilesToIndex:existingFiles error:&error]) {
[self presentError:error];
}
}

if (nonExistingFiles.count > 0) {
if (![self.repository removeFilesFromIndex:nonExistingFiles error:&error]) {
[self presentError:error];
}
}

[self.repository notifyRepositoryChanged];
}

- (void)stageSelectedChangesForFile:(NSString*)path oldLines:(NSIndexSet*)oldLines newLines:(NSIndexSet*)newLines {
Expand All @@ -227,8 +248,12 @@ - (void)stageSelectedChangesForFile:(NSString*)path oldLines:(NSIndexSet*)oldLin
}

- (void)unstageAllChangesForFile:(NSString*)path {
[self unstageAllChangesForFiles:@[ path ]];
}

- (void)unstageAllChangesForFiles:(NSArray<NSString*>*)paths {
NSError* error;
if ([self.repository resetFileInIndexToHEAD:path error:&error]) {
if ([self.repository resetFilesInIndexToHEAD:paths error:&error]) {
[self.repository notifyWorkingDirectoryChanged];
} else {
[self presentError:error];
Expand All @@ -255,18 +280,36 @@ - (void)unstageSelectedChangesForFile:(NSString*)path oldLines:(NSIndexSet*)oldL
}

- (BOOL)discardAllChangesForFile:(NSString*)path resetIndex:(BOOL)resetIndex error:(NSError**)error {
return [self discardAllChangesForFiles:@[ path ]
resetIndex:resetIndex
error:error];
}

- (BOOL)discardAllChangesForFiles:(NSArray<NSString*>*)paths resetIndex:(BOOL)resetIndex error:(NSError**)error {
BOOL success = NO;
if (resetIndex) {
GCCommit* commit;
if ([self.repository lookupHEADCurrentCommit:&commit branch:NULL error:error] && [self.repository resetFileInIndexToHEAD:path error:error]) {
if (commit && [self.repository checkTreeForCommit:commit containsFile:path error:NULL]) {
success = [self.repository safeDeleteFileIfExists:path error:error] && [self.repository checkoutFileFromIndex:path error:error];
} else {
success = [self.repository safeDeleteFile:path error:error];
if ([self.repository lookupHEADCurrentCommit:&commit branch:NULL error:error] && [self.repository resetFilesInIndexToHEAD:paths error:error]) {
success = YES;
for (NSString* path in paths) {
if (commit && [self.repository checkTreeForCommit:commit containsFile:path error:NULL]) {
if (![self.repository safeDeleteFileIfExists:path error:error] && [self.repository checkoutFileFromIndex:path error:error]) {
return NO;
}
} else {
if (![self.repository safeDeleteFileIfExists:path error:error]) {
return NO;
}
}
}
}
} else {
success = [self.repository safeDeleteFileIfExists:path error:error] && [self.repository checkoutFileFromIndex:path error:error];
for (NSString* path in paths) {
if (![self.repository safeDeleteFileIfExists:path error:error]) {
return NO;
}
}
success = [self.repository checkoutFilesFromIndex:paths error:error];
}
return success;
}
Expand Down Expand Up @@ -922,7 +965,7 @@ - (void)launchDiffToolWithCommit:(GCCommit*)commit otherCommit:(GCCommit*)otherC
} else if ([identifier isEqualToString:GIViewControllerTool_BeyondCompare]) {
[self _runBeyondCompareWithArguments:@[ [NSString stringWithFormat:@"-title1=%@", oldTitle], [NSString stringWithFormat:@"-title2=%@", newTitle], oldPath, newPath ]];
} else if ([identifier isEqualToString:GIViewControllerTool_P4Merge] || [identifier isEqualToString:GIViewControllerTool_GitTool]) {
; // Handled above
// Handled above
} else if ([identifier isEqualToString:GIViewControllerTool_DiffMerge]) {
[self _runDiffMergeToolWithArguments:@[ [NSString stringWithFormat:@"-t1=%@", oldTitle], [NSString stringWithFormat:@"-t2=%@", newTitle], oldPath, newPath ]];
} else {
Expand Down
Loading

0 comments on commit bd1775a

Please sign in to comment.