SubmitStrategy: Eagerly evaluate server ident

Calling get() multiple times may return skewed results, and can
clutter code. Running submit strategies is already pretty expensive;
don't worry about the cost of one more eager allocation.

Change-Id: I7b0b74ab376656f7e48d6343162af0a86765201a
This commit is contained in:
Dave Borowitz
2016-01-06 16:29:58 -05:00
parent 9e75e3dd23
commit cc13364a62
6 changed files with 13 additions and 19 deletions

View File

@@ -150,7 +150,7 @@ public class CherryPick extends SubmitStrategy {
args.mergeUtil.createCherryPickCommitMessage(toMerge); args.mergeUtil.createCherryPickCommitMessage(toMerge);
PersonIdent committer = args.caller.newCommitterIdent( PersonIdent committer = args.caller.newCommitterIdent(
ctx.getWhen(), args.serverIdent.get().getTimeZone()); ctx.getWhen(), args.serverIdent.getTimeZone());
try { try {
newCommit = args.mergeUtil.createCherryPickFromCommit( newCommit = args.mergeUtil.createCherryPickFromCommit(
args.repo, args.inserter, mergeTip.getCurrentTip(), toMerge, args.repo, args.inserter, mergeTip.getCurrentTip(), toMerge,
@@ -230,8 +230,7 @@ public class CherryPick extends SubmitStrategy {
if (args.rw.isMergedInto(mergeTip.getCurrentTip(), toMerge)) { if (args.rw.isMergedInto(mergeTip.getCurrentTip(), toMerge)) {
mergeTip.moveTipTo(toMerge, toMerge); mergeTip.moveTipTo(toMerge, toMerge);
} else { } else {
PersonIdent myIdent = PersonIdent myIdent = new PersonIdent(args.serverIdent, ctx.getWhen());
new PersonIdent(args.serverIdent.get(), ctx.getWhen());
CodeReviewCommit result = args.mergeUtil.mergeOneCommit(myIdent, CodeReviewCommit result = args.mergeUtil.mergeOneCommit(myIdent,
myIdent, args.repo, args.rw, args.inserter, myIdent, args.repo, args.rw, args.inserter,
args.canMergeFlag, args.destBranch, mergeTip.getCurrentTip(), args.canMergeFlag, args.destBranch, mergeTip.getCurrentTip(),

View File

@@ -43,11 +43,10 @@ public class MergeAlways extends SubmitStrategy {
} }
while (!sorted.isEmpty()) { while (!sorted.isEmpty()) {
CodeReviewCommit mergedFrom = sorted.remove(0); CodeReviewCommit mergedFrom = sorted.remove(0);
PersonIdent serverIdent = args.serverIdent.get();
PersonIdent caller = args.caller.newCommitterIdent( PersonIdent caller = args.caller.newCommitterIdent(
serverIdent.getWhen(), serverIdent.getTimeZone()); args.serverIdent.getWhen(), args.serverIdent.getTimeZone());
CodeReviewCommit newTip = CodeReviewCommit newTip =
args.mergeUtil.mergeOneCommit(caller, serverIdent, args.mergeUtil.mergeOneCommit(caller, args.serverIdent,
args.repo, args.rw, args.inserter, args.canMergeFlag, args.repo, args.rw, args.inserter, args.canMergeFlag,
args.destBranch, mergeTip.getCurrentTip(), mergedFrom); args.destBranch, mergeTip.getCurrentTip(), mergedFrom);
mergeTip.moveTipTo(newTip, mergedFrom); mergeTip.moveTipTo(newTip, mergedFrom);

View File

@@ -49,11 +49,10 @@ public class MergeIfNecessary extends SubmitStrategy {
// For every other commit do a pair-wise merge. // For every other commit do a pair-wise merge.
while (!sorted.isEmpty()) { while (!sorted.isEmpty()) {
CodeReviewCommit mergedFrom = sorted.remove(0); CodeReviewCommit mergedFrom = sorted.remove(0);
PersonIdent serverIdent = args.serverIdent.get();
PersonIdent caller = args.caller.newCommitterIdent( PersonIdent caller = args.caller.newCommitterIdent(
serverIdent.getWhen(), serverIdent.getTimeZone()); args.serverIdent.getWhen(), args.serverIdent.getTimeZone());
branchTip = branchTip =
args.mergeUtil.mergeOneCommit(caller, serverIdent, args.mergeUtil.mergeOneCommit(caller, args.serverIdent,
args.repo, args.rw, args.inserter, args.canMergeFlag, args.repo, args.rw, args.inserter, args.canMergeFlag,
args.destBranch, branchTip, mergedFrom); args.destBranch, branchTip, mergedFrom);
mergeTip.moveTipTo(branchTip, mergedFrom); mergeTip.moveTipTo(branchTip, mergedFrom);

View File

@@ -38,7 +38,6 @@ import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent;
import java.io.IOException; import java.io.IOException;
import java.util.Collection; import java.util.Collection;
@@ -164,7 +163,7 @@ public class RebaseIfNecessary extends SubmitStrategy {
// Racy read of patch set is ok; see comments in RebaseChangeOp. // Racy read of patch set is ok; see comments in RebaseChangeOp.
args.db.patchSets().get(toMerge.getPatchsetId()), args.db.patchSets().get(toMerge.getPatchsetId()),
mergeTip.getCurrentTip().name()) mergeTip.getCurrentTip().name())
.setCommitterIdent(args.serverIdent.get()) .setCommitterIdent(args.serverIdent)
.setRunHooks(false) .setRunHooks(false)
.setValidatePolicy(CommitValidators.Policy.NONE); .setValidatePolicy(CommitValidators.Policy.NONE);
try { try {
@@ -243,12 +242,11 @@ public class RebaseIfNecessary extends SubmitStrategy {
mergeTip.moveTipTo(toMerge, toMerge); mergeTip.moveTipTo(toMerge, toMerge);
acceptMergeTip(mergeTip); acceptMergeTip(mergeTip);
} else { } else {
PersonIdent myIdent = args.serverIdent.get();
// TODO(dborowitz): Can't use repo from ctx due to canMergeFlag. // TODO(dborowitz): Can't use repo from ctx due to canMergeFlag.
CodeReviewCommit newTip = args.mergeUtil.mergeOneCommit( CodeReviewCommit newTip = args.mergeUtil.mergeOneCommit(
myIdent, myIdent, args.repo, args.rw, args.inserter, args.serverIdent, args.serverIdent, args.repo, args.rw,
args.canMergeFlag, args.destBranch, mergeTip.getCurrentTip(), args.inserter, args.canMergeFlag, args.destBranch,
toMerge); mergeTip.getCurrentTip(), toMerge);
mergeTip.moveTipTo(newTip, toMerge); mergeTip.moveTipTo(newTip, toMerge);
} }
args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag, args.mergeUtil.markCleanMerges(args.rw, args.canMergeFlag,

View File

@@ -30,7 +30,6 @@ import com.google.gerrit.server.git.MergeSorter;
import com.google.gerrit.server.git.MergeTip; import com.google.gerrit.server.git.MergeTip;
import com.google.gerrit.server.git.MergeUtil; import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.inject.Provider;
import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.PersonIdent;
@@ -52,7 +51,7 @@ import java.util.Set;
*/ */
public abstract class SubmitStrategy { public abstract class SubmitStrategy {
static class Arguments { static class Arguments {
protected final Provider<PersonIdent> serverIdent; protected final PersonIdent serverIdent;
protected final ReviewDb db; protected final ReviewDb db;
protected final BatchUpdate.Factory batchUpdateFactory; protected final BatchUpdate.Factory batchUpdateFactory;
protected final ChangeControl.GenericFactory changeControlFactory; protected final ChangeControl.GenericFactory changeControlFactory;
@@ -68,7 +67,7 @@ public abstract class SubmitStrategy {
protected final MergeSorter mergeSorter; protected final MergeSorter mergeSorter;
protected final IdentifiedUser caller; protected final IdentifiedUser caller;
Arguments(Provider<PersonIdent> serverIdent, ReviewDb db, Arguments(PersonIdent serverIdent, ReviewDb db,
BatchUpdate.Factory batchUpdateFactory, BatchUpdate.Factory batchUpdateFactory,
ChangeControl.GenericFactory changeControlFactory, Repository repo, ChangeControl.GenericFactory changeControlFactory, Repository repo,
CodeReviewRevWalk rw, ObjectInserter inserter, RevFlag canMergeFlag, CodeReviewRevWalk rw, ObjectInserter inserter, RevFlag canMergeFlag,

View File

@@ -86,7 +86,7 @@ public class SubmitStrategyFactory {
throws IntegrationException, NoSuchProjectException { throws IntegrationException, NoSuchProjectException {
ProjectState project = getProject(destBranch); ProjectState project = getProject(destBranch);
SubmitStrategy.Arguments args = new SubmitStrategy.Arguments( SubmitStrategy.Arguments args = new SubmitStrategy.Arguments(
myIdent, db, batchUpdateFactory, changeControlFactory, myIdent.get(), db, batchUpdateFactory, changeControlFactory,
repo, rw, inserter, canMergeFlag, alreadyAccepted, repo, rw, inserter, canMergeFlag, alreadyAccepted,
destBranch,approvalsUtil, mergeUtilFactory.create(project), caller); destBranch,approvalsUtil, mergeUtilFactory.create(project), caller);
switch (submitType) { switch (submitType) {