Memoize the OptionsData per BlazeCommand.

This saves the cost of (1) collecting all Options classes and (2) getting all their @Option annotations. Note that there is no savings on reflection costs, since that's already memoized internally by OptionsParser.

This saves ~250us per Blaze invocation.

--
MOS_MIGRATED_REVID=121153156

4 files changed, 69 insertions, 11 deletions

diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
index 5abddfe86b..3e17b47a38 100644
--- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
+++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeCommandDispatcher.java
@@ -18,12 +18,14 @@ import com.google.common.base.Function;
 import com.google.common.base.Joiner;
 import com.google.common.base.Predicates;
 import com.google.common.base.Verify;
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.ListMultimap;
-import com.google.common.collect.Lists;
 import com.google.common.io.Flushables;
 import com.google.devtools.build.lib.Constants;
 import com.google.devtools.build.lib.events.Event;
@@ -40,8 +42,8 @@ import com.google.devtools.build.lib.util.io.DelegatingOutErr;
 import com.google.devtools.build.lib.util.io.OutErr;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
+import com.google.devtools.common.options.OpaqueOptionsData;
 import com.google.devtools.common.options.OptionPriority;
-import com.google.devtools.common.options.OptionsBase;
 import com.google.devtools.common.options.OptionsParser;
 import com.google.devtools.common.options.OptionsParsingException;
 
@@ -146,6 +148,17 @@ public class BlazeCommandDispatcher {
   private final Object commandLock;
   private String currentClientDescription = null;
   private OutputStream logOutputStream = null;
+  private final LoadingCache<BlazeCommand, OpaqueOptionsData> optionsDataCache =
+      CacheBuilder.newBuilder().build(
+          new CacheLoader<BlazeCommand, OpaqueOptionsData>() {
+            @Override
+            public OpaqueOptionsData load(BlazeCommand command) {
+              return OptionsParser.getOptionsData(BlazeCommandUtils.getOptions(
+                  command.getClass(),
+                  runtime.getBlazeModules(),
+                  runtime.getRuleClassProvider()));
+            }
+          });
 
   /**
    * Create a Blaze dispatcher that uses the specified {@code BlazeRuntime} instance, but overrides
@@ -655,10 +668,7 @@ public class BlazeCommandDispatcher {
   protected OptionsParser createOptionsParser(BlazeCommand command)
       throws OptionsParsingException {
     Command annotation = command.getClass().getAnnotation(Command.class);
-    List<Class<? extends OptionsBase>> allOptions = Lists.newArrayList();
-    allOptions.addAll(BlazeCommandUtils.getOptions(
-        command.getClass(), getRuntime().getBlazeModules(), getRuntime().getRuleClassProvider()));
-    OptionsParser parser = OptionsParser.newOptionsParser(allOptions);
+    OptionsParser parser = OptionsParser.newOptionsParser(optionsDataCache.getUnchecked(command));
     parser.setAllowResidue(annotation.allowResidue());
     return parser;
   }
diff --git a/src/main/java/com/google/devtools/common/options/OpaqueOptionsData.java b/src/main/java/com/google/devtools/common/options/OpaqueOptionsData.java
new file mode 100644
index 0000000000..befc122f38
--- /dev/null
+++ b/src/main/java/com/google/devtools/common/options/OpaqueOptionsData.java
@@ -0,0 +1,26 @@
+// Copyright 2016 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//    http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.common.options;
+
+/**
+ * Opaque options data type, returned by {@link OptionsParser#getOptionsData} and consumed by
+ * {@link OptionsParser#newOptionsParser(OpaqueOptionsData)}.
+ */
+public abstract class OpaqueOptionsData {
+  // Package-protected so the existence of subclasses is under our control.
+  OpaqueOptionsData() {
+  }
+}
+
diff --git a/src/main/java/com/google/devtools/common/options/OptionsData.java b/src/main/java/com/google/devtools/common/options/OptionsData.java
index 0038cad953..ac23d639b2 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsData.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsData.java
@@ -37,7 +37,7 @@ import javax.annotation.concurrent.Immutable;
  * Therefore this class can be used internally to cache the results.
  */
 @Immutable
-final class OptionsData {
+final class OptionsData extends OpaqueOptionsData {
 
   /**
    * These are the options-declaring classes which are annotated with
diff --git a/src/main/java/com/google/devtools/common/options/OptionsParser.java b/src/main/java/com/google/devtools/common/options/OptionsParser.java
index e47d219e7d..fd6d91507f 100644
--- a/src/main/java/com/google/devtools/common/options/OptionsParser.java
+++ b/src/main/java/com/google/devtools/common/options/OptionsParser.java
@@ -72,7 +72,20 @@ public class OptionsParser implements OptionsProvider {
   private static final Map<ImmutableList<Class<? extends OptionsBase>>, OptionsData> optionsData =
       Maps.newHashMap();
 
-  private static synchronized OptionsData getOptionsData(
+  /**
+   * Returns {@link OpaqueOptionsData} suitable for passing along to
+   * {@link #newOptionsParser(OpaqueOptionsData optionsData)}.
+   *
+   * This is useful when you want to do the work of analyzing the given {@code optionsClasses}
+   * exactly once, but you want to parse lots of different lists of strings (and thus need to
+   * construct lots of different {@link OptionsParser} instances). 
+   */
+  public static OpaqueOptionsData getOptionsData(
+      ImmutableList<Class<? extends OptionsBase>> optionsClasses) {
+    return getOptionsDataInternal(optionsClasses);
+  }
+
+  private static synchronized OptionsData getOptionsDataInternal(
       ImmutableList<Class<? extends OptionsBase>> optionsClasses) {
     OptionsData result = optionsData.get(optionsClasses);
     if (result == null) {
@@ -87,7 +100,8 @@ public class OptionsParser implements OptionsProvider {
    * ones.
    */
   static Collection<Field> getAllAnnotatedFields(Class<? extends OptionsBase> optionsClass) {
-    OptionsData data = getOptionsData(ImmutableList.<Class<? extends OptionsBase>>of(optionsClass));
+    OptionsData data = getOptionsDataInternal(
+        ImmutableList.<Class<? extends OptionsBase>>of(optionsClass));
     return data.getFieldsForClass(optionsClass);
   }
 
@@ -111,8 +125,16 @@ public class OptionsParser implements OptionsProvider {
    */
   public static OptionsParser newOptionsParser(
       Iterable<? extends Class<? extends OptionsBase>> optionsClasses) {
-    return new OptionsParser(
-        getOptionsData(ImmutableList.<Class<? extends OptionsBase>>copyOf(optionsClasses)));
+    return newOptionsParser(
+        getOptionsDataInternal(ImmutableList.<Class<? extends OptionsBase>>copyOf(optionsClasses)));
+  }
+
+  /**
+   * Create a new {@link OptionsParser}, using {@link OpaqueOptionsData} previously returned from
+   * {@link #getOptionsData}.
+   */
+  public static OptionsParser newOptionsParser(OpaqueOptionsData optionsData) {
+    return new OptionsParser((OptionsData) optionsData);
   }
 
   private final OptionsParserImpl impl;