Make command line option parsing ~6500% faster

Guice does not like to pass an Injector as part of an @Assisted
invocation of an object. More recent versions of Guice are logging
a warning advising a performance boost of ~6500% is possible if
the class stops accepting Injector in the constructor and instead
uses a more concrete type.

In this case Guice was very correct about the code being slow.
Every option declared required a linear scan through the entire
Injector stack looking for any candidate binding that might match
the OptionHandlerFactory signature. This scan ran on each request,
which is incredibly slow.

Build the map once in the injector and retain as a singleton.

Change-Id: Ib620e7bf1e24241bd373d2cb55c89c3fe3284645
This commit is contained in:
Shawn Pearce
2013-08-05 15:19:57 -07:00
parent f4a6330088
commit 242a604b9f
8 changed files with 120 additions and 37 deletions

View File

@@ -18,8 +18,6 @@ import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.server.config.FactoryModule;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.args4j.AccountGroupIdHandler;
import com.google.gerrit.server.args4j.AccountGroupUUIDHandler;
import com.google.gerrit.server.args4j.AccountIdHandler;
@@ -28,11 +26,12 @@ import com.google.gerrit.server.args4j.ObjectIdHandler;
import com.google.gerrit.server.args4j.PatchSetIdHandler;
import com.google.gerrit.server.args4j.ProjectControlHandler;
import com.google.gerrit.server.args4j.SocketAddressHandler;
import com.google.gerrit.server.config.FactoryModule;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.util.cli.CmdLineParser;
import com.google.gerrit.util.cli.OptionHandlerUtil;
import com.google.gerrit.util.cli.OptionHandlers;
import org.eclipse.jgit.lib.ObjectId;
import org.kohsuke.args4j.spi.OptionHandler;
import java.net.SocketAddress;
@@ -44,6 +43,7 @@ public class CmdLineParserModule extends FactoryModule {
@Override
protected void configure() {
factory(CmdLineParser.Factory.class);
bind(OptionHandlers.class);
registerOptionHandler(Account.Id.class, AccountIdHandler.class);
registerOptionHandler(AccountGroup.Id.class, AccountGroupIdHandler.class);

View File

@@ -16,9 +16,11 @@ package com.google.gerrit.server.args4j;
import com.google.gerrit.common.ProjectUtil;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectControl;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
import org.kohsuke.args4j.CmdLineException;
@@ -27,17 +29,26 @@ import org.kohsuke.args4j.OptionDef;
import org.kohsuke.args4j.spi.OptionHandler;
import org.kohsuke.args4j.spi.Parameters;
import org.kohsuke.args4j.spi.Setter;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
public class ProjectControlHandler extends OptionHandler<ProjectControl> {
private final ProjectControl.Factory projectControlFactory;
private static final Logger log = LoggerFactory
.getLogger(ProjectControlHandler.class);
private final ProjectControl.GenericFactory projectControlFactory;
private final Provider<CurrentUser> user;
@Inject
public ProjectControlHandler(
final ProjectControl.Factory projectControlFactory,
final ProjectControl.GenericFactory projectControlFactory,
Provider<CurrentUser> user,
@Assisted final CmdLineParser parser, @Assisted final OptionDef option,
@Assisted final Setter<ProjectControl> setter) {
super(parser, option, setter);
this.projectControlFactory = projectControlFactory;
this.user = user;
}
@Override
@@ -59,13 +70,21 @@ public class ProjectControlHandler extends OptionHandler<ProjectControl> {
}
String nameWithoutSuffix = ProjectUtil.stripGitSuffix(projectName);
Project.NameKey nameKey = new Project.NameKey(nameWithoutSuffix);
final ProjectControl control;
try {
Project.NameKey nameKey = new Project.NameKey(nameWithoutSuffix);
control = projectControlFactory.validateFor(nameKey, ProjectControl.OWNER | ProjectControl.VISIBLE);
control = projectControlFactory.validateFor(
nameKey,
ProjectControl.OWNER | ProjectControl.VISIBLE,
user.get());
} catch (NoSuchProjectException e) {
throw new CmdLineException(owner, e.getMessage());
} catch (IOException e) {
log.warn("Cannot load project " + nameWithoutSuffix, e);
throw new CmdLineException(
owner,
new NoSuchProjectException(nameKey).getMessage());
}
setter.addValue(control);

View File

@@ -29,6 +29,7 @@ import com.google.gerrit.rules.PrologModule;
import com.google.gerrit.rules.RulesCache;
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.CmdLineParserModule;
import com.google.gerrit.server.FileTypeRegistry;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.InternalUser;
@@ -163,6 +164,7 @@ public class GerritGlobalModule extends FactoryModule {
install(ChangeCache.module());
install(new AccessControlModule());
install(new CmdLineParserModule());
install(new EmailModule());
install(new GitModule());
install(new PrologModule());