Faster caching glue

Author: mpkorstanje

cucumber-core/src/main/java/io/cucumber/core/runner/CachingGlue.java

@@ -44,6 +44,7 @@
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 import java.util.TreeMap;
 
@@ -81,6 +82,7 @@ final class CachingGlue implements Glue {
     private final Map<String, CoreStepDefinition> stepDefinitionsByPattern = new TreeMap<>();
 
     private final EventBus bus;
+    private StepTypeRegistry stepTypeRegistry;
 
     CachingGlue(EventBus bus) {
         this.bus = bus;
@@ -228,7 +230,12 @@ List<DocStringTypeDefinition> getDocStringTypeDefinitions() {
         return docStringTypeDefinitions;
     }
 
-    void prepareGlue(StepTypeRegistry stepTypeRegistry) throws DuplicateStepDefinitionException {
+    StepTypeRegistry getStepTypeRegistry() {
+        return null;
+    }
+
+    void prepareGlue(Locale locale) throws DuplicateStepDefinitionException {
+        stepTypeRegistry = new StepTypeRegistry(locale);
         StepExpressionFactory stepExpressionFactory = new StepExpressionFactory(stepTypeRegistry, bus);
 
         // TODO: separate prepared and unprepared glue into different classes
@@ -425,7 +432,6 @@ private List<PickleStepDefinitionMatch> stepDefinitionMatches(URI uri, Step step
     }
 
     void removeScenarioScopedGlue() {
-        stepDefinitionsByPattern.clear();
         removeScenarioScopedGlue(beforeHooks);
         removeScenarioScopedGlue(beforeStepHooks);
         removeScenarioScopedGlue(afterHooks);
@@ -437,18 +443,24 @@ void removeScenarioScopedGlue() {
         removeScenarioScopedGlue(defaultParameterTransformers);
         removeScenarioScopedGlue(defaultDataTableEntryTransformers);
         removeScenarioScopedGlue(defaultDataTableCellTransformers);
+
+        stepDefinitionsByPattern.clear();
+
     }
 
-    private void removeScenarioScopedGlue(Iterable<?> glues) {
+    private boolean removeScenarioScopedGlue(Iterable<?> glues) {
+        boolean dirty = false;
         Iterator<?> glueIterator = glues.iterator();
         while (glueIterator.hasNext()) {
             Object glue = glueIterator.next();
             if (glue instanceof ScenarioScoped) {
                 ScenarioScoped scenarioScoped = (ScenarioScoped) glue;
                 scenarioScoped.dispose();
                 glueIterator.remove();
+                dirty = true;
             }
         }
+        return dirty;
     }
 
 }

cucumber-core/src/main/java/io/cucumber/core/runner/Runner.java

@@ -63,13 +63,12 @@ public EventBus getBus() {
 
     public void runPickle(Pickle pickle) {
         try {
-            StepTypeRegistry stepTypeRegistry = createTypeRegistryForPickle(pickle);
-            snippetGenerators = createSnippetGeneratorsForPickle(stepTypeRegistry);
 
             // Java8 step definitions will be added to the glue here
             buildBackendWorlds();
 
-            glue.prepareGlue(stepTypeRegistry);
+            glue.prepareGlue(localeForPickle(pickle));
+            snippetGenerators = createSnippetGeneratorsForPickle(glue.getStepTypeRegistry());
 
             TestCase testCase = createTestCaseForPickle(pickle);
             testCase.run(bus);
@@ -79,10 +78,9 @@ public void runPickle(Pickle pickle) {
         }
     }
 
-    private StepTypeRegistry createTypeRegistryForPickle(Pickle pickle) {
+    private Locale localeForPickle(Pickle pickle) {
         String language = pickle.getLanguage();
-        Locale locale = new Locale(language);
-        return new StepTypeRegistry(locale);
+        return new Locale(language);
     }
 
     public void runBeforeAllHooks() {

cucumber-java8/src/main/java/io/cucumber/java8/AbstractGlueDefinition.java

@@ -1,7 +1,6 @@
 package io.cucumber.java8;
 
 import io.cucumber.core.backend.Located;
-import io.cucumber.core.backend.ScenarioScoped;
 import io.cucumber.core.backend.SourceReference;
 import net.jodah.typetools.TypeResolver;
 
@@ -14,20 +13,33 @@
 import static java.lang.String.format;
 import static java.util.Objects.requireNonNull;
 
-abstract class AbstractGlueDefinition implements ScenarioScoped, Located {
+abstract class AbstractGlueDefinition implements Located {
 
     private Object body;
-    final Method method;
+    private Method method;
+    private SourceReference sourceReference;
     final StackTraceElement location;
-    SourceReference sourceReference;
 
     AbstractGlueDefinition(Object body, StackTraceElement location) {
+        updateClosure(body);
+        this.location = requireNonNull(location);
+    }
+
+    void updateClosure(AbstractGlueDefinition other) {
+        updateClosure(other.body);
+    }
+
+    private void updateClosure(Object body) {
         this.body = requireNonNull(body);
         this.method = getAcceptMethod(body.getClass());
-        this.location = requireNonNull(location);
     }
 
-    private Method getAcceptMethod(Class<?> bodyClass) {
+    void disposeClosure() {
+        this.body = null;
+        this.method = null;
+    }
+
+    private static Method getAcceptMethod(Class<?> bodyClass) {
         List<Method> acceptMethods = new ArrayList<>();
         for (Method method : bodyClass.getDeclaredMethods()) {
             if (!method.isBridge() && !method.isSynthetic() && "accept".equals(method.getName())) {
@@ -48,9 +60,8 @@ protected Object invokeMethod(Object... args) {
         return Invoker.invoke(this, body, method, args);
     }
 
-    @Override
-    public void dispose() {
-        this.body = null;
+    protected int getParameterCount() {
+        return method.getParameterCount();
     }
 
     @Override
@@ -65,10 +76,14 @@ public final boolean isDefinedAt(StackTraceElement stackTraceElement) {
 
     @Override
     public Optional<SourceReference> getSourceReference() {
+        return Optional.of(requireSourceReference());
+    }
+
+    SourceReference requireSourceReference() {
         if (sourceReference == null) {
             sourceReference = fromStackTraceElement(location);
         }
-        return Optional.of(sourceReference);
+        return sourceReference;
     }
 
     Class<?>[] resolveRawArguments(Class<?> bodyClass, Class<?> body) {

cucumber-java8/src/main/java/io/cucumber/java8/ClosureAwareGlueRegistry.java

@@ -0,0 +1,113 @@
+package io.cucumber.java8;
+
+import io.cucumber.core.backend.DataTableTypeDefinition;
+import io.cucumber.core.backend.DefaultDataTableCellTransformerDefinition;
+import io.cucumber.core.backend.DefaultDataTableEntryTransformerDefinition;
+import io.cucumber.core.backend.DefaultParameterTransformerDefinition;
+import io.cucumber.core.backend.DocStringTypeDefinition;
+import io.cucumber.core.backend.Glue;
+import io.cucumber.core.backend.HookDefinition;
+import io.cucumber.core.backend.ParameterTypeDefinition;
+import io.cucumber.core.backend.StepDefinition;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.function.Consumer;
+
+final class ClosureAwareGlueRegistry implements LambdaGlueRegistry {
+
+    private final List<AbstractGlueDefinition> definitions = new ArrayList<>();
+    private int registered;
+
+    private final Glue glue;
+
+    ClosureAwareGlueRegistry(Glue glue) {
+        this.glue = glue;
+    }
+
+    void startRegistration() {
+        registered = 0;
+    }
+
+    @Override
+    public void addStepDefinition(StepDefinition stepDefinition) {
+        updateOrRegister((Java8StepDefinition) stepDefinition, definitions, glue::addStepDefinition);
+    }
+
+    @Override
+    public void addBeforeStepHookDefinition(HookDefinition beforeStepHook) {
+        updateOrRegister((Java8HookDefinition) beforeStepHook, definitions, glue::addBeforeStepHook);
+
+    }
+
+    @Override
+    public void addAfterStepHookDefinition(HookDefinition afterStepHook) {
+        updateOrRegister((Java8HookDefinition) afterStepHook, definitions, glue::addAfterStepHook);
+    }
+
+    @Override
+    public void addBeforeHookDefinition(HookDefinition beforeHook) {
+        updateOrRegister((Java8HookDefinition) beforeHook, definitions, glue::addBeforeHook);
+    }
+
+    @Override
+    public void addAfterHookDefinition(HookDefinition afterHook) {
+        updateOrRegister((Java8HookDefinition) afterHook, definitions, glue::addAfterHook);
+    }
+
+    @Override
+    public void addDocStringType(DocStringTypeDefinition docStringType) {
+        updateOrRegister((Java8DocStringTypeDefinition) docStringType, definitions, glue::addDocStringType);
+    }
+
+    @Override
+    public void addDataTableType(DataTableTypeDefinition dataTableType) {
+        updateOrRegister((Java8DataTableTypeDefinition) dataTableType, definitions, glue::addDataTableType);
+    }
+
+    @Override
+    public void addParameterType(ParameterTypeDefinition parameterType) {
+        updateOrRegister((Java8ParameterTypeDefinition) parameterType, definitions, glue::addParameterType);
+    }
+
+    @Override
+    public void addDefaultParameterTransformer(DefaultParameterTransformerDefinition defaultParameterTransformer) {
+        updateOrRegister((Java8DefaultParameterTransformerDefinition) defaultParameterTransformer, definitions,
+            glue::addDefaultParameterTransformer);
+    }
+
+    @Override
+    public void addDefaultDataTableCellTransformer(
+            DefaultDataTableCellTransformerDefinition defaultDataTableCellTransformer
+    ) {
+        updateOrRegister((Java8DefaultDataTableCellTransformerDefinition) defaultDataTableCellTransformer, definitions,
+            glue::addDefaultDataTableCellTransformer);
+    }
+
+    @Override
+    public void addDefaultDataTableEntryTransformer(
+            DefaultDataTableEntryTransformerDefinition defaultDataTableEntryTransformer
+    ) {
+        updateOrRegister(
+            (Java8DefaultDataTableEntryTransformerDefinition) defaultDataTableEntryTransformer,
+            definitions,
+            glue::addDefaultDataTableEntryTransformer);
+    }
+
+    private <T extends AbstractGlueDefinition> void updateOrRegister(
+            T definition, List<AbstractGlueDefinition> definitions, Consumer<T> register
+    ) {
+        if (definitions.size() <= registered) {
+            definitions.add(definition);
+            register.accept(definition);
+        } else {
+            AbstractGlueDefinition existing = definitions.get(registered);
+            existing.updateClosure(definition);
+        }
+        registered++;
+    }
+
+    void disposeClosures() {
+        definitions.forEach(AbstractGlueDefinition::disposeClosure);
+    }
+}