RepoView: Make flushing inserter a no-op
Flushing is already done automatically at the right time by the BatchUpdate implementations, and the RevWalk provided to callers can already read unflushed objects. A BatchUpdateOp implementation calling flush on the inserter is doing something wrong, and flushing too much can seriously impact DFS performance when it happens in a loop. Make flushing (and closing) a no-op rather than slapping implementors' wrists by throwing an exception. Even if the implementor is well- intentioned, we can't guarantee that all downstream library functions are going to know not to flush their inserters. For example, JGit's Merger used to be aggressive about flushing until I fixed it[1]. This was the most glaring example, but I would be utterly unsurprised to find more. Also don't use InMemoryInserter for this purpose, as it actually flushes too little. The purpose of that class is to avoid writing a pack to DFS when the internal buffer in DfsInserter exceeds some limit, but in BatchUpdate, it's fine to write out an intermediate pack in this case, we don't have to buffer the whole thing in memory. [1] https://git.eclipse.org/r/12504 Change-Id: I2b139e50af213c4b683c25d30b190bc547304f72
This commit is contained in:
@@ -332,11 +332,6 @@ public abstract class BatchUpdate implements AutoCloseable {
|
||||
return repoView.getRevWalk();
|
||||
}
|
||||
|
||||
protected ObjectInserter getObjectInserter() throws IOException {
|
||||
initRepository();
|
||||
return repoView.getInserter();
|
||||
}
|
||||
|
||||
public Map<String, ReceiveCommand> getRefUpdates() {
|
||||
return repoView != null ? repoView.getCommands().getCommands() : ImmutableMap.of();
|
||||
}
|
||||
|
@@ -184,7 +184,7 @@ class NoteDbBatchUpdate extends BatchUpdate {
|
||||
private class RepoContextImpl extends ContextImpl implements RepoContext {
|
||||
@Override
|
||||
public ObjectInserter getInserter() throws IOException {
|
||||
return getRepoView().getInserter();
|
||||
return getRepoView().getInserterWrapper();
|
||||
}
|
||||
|
||||
@Override
|
||||
|
@@ -48,12 +48,14 @@ public class RepoView {
|
||||
private final Repository repo;
|
||||
private final RevWalk rw;
|
||||
private final ObjectInserter inserter;
|
||||
private final ObjectInserter inserterWrapper;
|
||||
private final ChainedReceiveCommands commands;
|
||||
private final boolean closeRepo;
|
||||
|
||||
RepoView(GitRepositoryManager repoManager, Project.NameKey project) throws IOException {
|
||||
repo = repoManager.openRepository(project);
|
||||
inserter = repo.newObjectInserter();
|
||||
inserterWrapper = new NonFlushingInserter(inserter);
|
||||
rw = new RevWalk(inserter.newReader());
|
||||
commands = new ChainedReceiveCommands(repo);
|
||||
closeRepo = true;
|
||||
@@ -68,6 +70,7 @@ public class RepoView {
|
||||
this.repo = checkNotNull(repo);
|
||||
this.rw = checkNotNull(rw);
|
||||
this.inserter = checkNotNull(inserter);
|
||||
inserterWrapper = new NonFlushingInserter(inserter);
|
||||
commands = new ChainedReceiveCommands(repo);
|
||||
closeRepo = false;
|
||||
}
|
||||
@@ -198,7 +201,34 @@ public class RepoView {
|
||||
return inserter;
|
||||
}
|
||||
|
||||
ObjectInserter getInserterWrapper() {
|
||||
return inserterWrapper;
|
||||
}
|
||||
|
||||
ChainedReceiveCommands getCommands() {
|
||||
return commands;
|
||||
}
|
||||
|
||||
private static class NonFlushingInserter extends ObjectInserter.Filter {
|
||||
private final ObjectInserter delegate;
|
||||
|
||||
private NonFlushingInserter(ObjectInserter delegate) {
|
||||
this.delegate = delegate;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected ObjectInserter delegate() {
|
||||
return delegate;
|
||||
}
|
||||
|
||||
@Override
|
||||
public void flush() {
|
||||
// Do nothing.
|
||||
}
|
||||
|
||||
@Override
|
||||
public void close() {
|
||||
// Do nothing; the delegate is closed separately.
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@@ -155,7 +155,7 @@ class ReviewDbBatchUpdate extends BatchUpdate {
|
||||
private class RepoContextImpl extends ContextImpl implements RepoContext {
|
||||
@Override
|
||||
public ObjectInserter getInserter() throws IOException {
|
||||
return getRepoView().getInserter();
|
||||
return getRepoView().getInserterWrapper();
|
||||
}
|
||||
|
||||
@Override
|
||||
|
Reference in New Issue
Block a user