Skip to content

Commit 9d5118c

Browse files
author
Julien Kronegg
committed
fix: finished faster prepare glue for #2902 
1 parent 45c9abb commit 9d5118c

File tree

3 files changed

+95
-55
lines changed

3 files changed

+95
-55
lines changed

Diff for: ‎cucumber-core/src/main/java/io/cucumber/core/runner/CachingGlue.java

+72-35
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,9 @@ final class CachingGlue implements Glue {
8383

8484
private final EventBus bus;
8585
private StepTypeRegistry stepTypeRegistry;
86+
private Locale locale = null;
87+
private StepExpressionFactory stepExpressionFactory = null;
88+
private boolean dirtyCache = false;
8689

8790
CachingGlue(EventBus bus) {
8891
this.bus = bus;
@@ -103,6 +106,7 @@ public void addAfterAllHook(StaticHookDefinition afterAllHook) {
103106
@Override
104107
public void addStepDefinition(StepDefinition stepDefinition) {
105108
stepDefinitions.add(stepDefinition);
109+
dirtyCache = true;
106110
}
107111

108112
@Override
@@ -132,11 +136,13 @@ public void addAfterStepHook(HookDefinition hookDefinition) {
132136
@Override
133137
public void addParameterType(ParameterTypeDefinition parameterType) {
134138
parameterTypeDefinitions.add(parameterType);
139+
dirtyCache = true;
135140
}
136141

137142
@Override
138143
public void addDataTableType(DataTableTypeDefinition dataTableType) {
139144
dataTableTypeDefinitions.add(dataTableType);
145+
dirtyCache = true;
140146
}
141147

142148
@Override
@@ -162,6 +168,7 @@ public void addDefaultDataTableCellTransformer(
162168
@Override
163169
public void addDocStringType(DocStringTypeDefinition docStringType) {
164170
docStringTypeDefinitions.add(docStringType);
171+
dirtyCache = true;
165172
}
166173

167174
List<StaticHookDefinition> getBeforeAllHooks() {
@@ -231,21 +238,36 @@ List<DocStringTypeDefinition> getDocStringTypeDefinitions() {
231238
}
232239

233240
StepTypeRegistry getStepTypeRegistry() {
234-
return null;
241+
return stepTypeRegistry;
235242
}
236243

237244
void prepareGlue(Locale locale) throws DuplicateStepDefinitionException {
238-
stepTypeRegistry = new StepTypeRegistry(locale);
239-
StepExpressionFactory stepExpressionFactory = new StepExpressionFactory(stepTypeRegistry, bus);
245+
boolean firstTime = stepTypeRegistry == null;
246+
boolean languageChanged = !locale.equals(this.locale);
247+
boolean mustRebuildCache = false;
248+
if (firstTime || languageChanged || this.dirtyCache) {
249+
// conditions changed => invalidate the glue cache
250+
this.locale = locale;
251+
stepTypeRegistry = new StepTypeRegistry(locale);
252+
stepExpressionFactory = new StepExpressionFactory(stepTypeRegistry, bus);
253+
stepDefinitionsByPattern.clear();
254+
stepPatternByStepText.clear();
255+
mustRebuildCache = true;
256+
this.dirtyCache = false; // since we must rebuild the cache, it will
257+
// not be dirty the next time
258+
}
240259

241260
// TODO: separate prepared and unprepared glue into different classes
242-
parameterTypeDefinitions.forEach(ptd -> {
243-
ParameterType<?> parameterType = ptd.parameterType();
244-
stepTypeRegistry.defineParameterType(parameterType);
245-
emitParameterTypeDefined(ptd);
246-
});
247-
dataTableTypeDefinitions.forEach(dtd -> stepTypeRegistry.defineDataTableType(dtd.dataTableType()));
248-
docStringTypeDefinitions.forEach(dtd -> stepTypeRegistry.defineDocStringType(dtd.docStringType()));
261+
if (mustRebuildCache) {
262+
// parameters changed from the previous scenario => re-register them
263+
parameterTypeDefinitions.forEach(ptd -> {
264+
ParameterType<?> parameterType = ptd.parameterType();
265+
stepTypeRegistry.defineParameterType(parameterType);
266+
emitParameterTypeDefined(ptd);
267+
});
268+
dataTableTypeDefinitions.forEach(dtd -> stepTypeRegistry.defineDataTableType(dtd.dataTableType()));
269+
docStringTypeDefinitions.forEach(dtd -> stepTypeRegistry.defineDocStringType(dtd.docStringType()));
270+
}
249271

250272
if (defaultParameterTransformers.size() == 1) {
251273
DefaultParameterTransformerDefinition definition = defaultParameterTransformers.get(0);
@@ -276,17 +298,19 @@ void prepareGlue(Locale locale) throws DuplicateStepDefinitionException {
276298
beforeHooks.forEach(this::emitHook);
277299
beforeStepHooks.forEach(this::emitHook);
278300

279-
stepDefinitions.forEach(stepDefinition -> {
280-
StepExpression expression = stepExpressionFactory.createExpression(stepDefinition);
281-
CoreStepDefinition coreStepDefinition = new CoreStepDefinition(bus.generateId(), stepDefinition,
282-
expression);
283-
CoreStepDefinition previous = stepDefinitionsByPattern.get(stepDefinition.getPattern());
284-
if (previous != null) {
285-
throw new DuplicateStepDefinitionException(previous, stepDefinition);
286-
}
287-
stepDefinitionsByPattern.put(coreStepDefinition.getExpression().getSource(), coreStepDefinition);
288-
emitStepDefined(coreStepDefinition);
289-
});
301+
if (mustRebuildCache) {
302+
stepDefinitions.forEach(stepDefinition -> {
303+
StepExpression expression = stepExpressionFactory.createExpression(stepDefinition);
304+
CoreStepDefinition coreStepDefinition = new CoreStepDefinition(bus.generateId(), stepDefinition,
305+
expression);
306+
CoreStepDefinition previous = stepDefinitionsByPattern.get(stepDefinition.getPattern());
307+
if (previous != null) {
308+
throw new DuplicateStepDefinitionException(previous, stepDefinition);
309+
}
310+
stepDefinitionsByPattern.put(coreStepDefinition.getExpression().getSource(), coreStepDefinition);
311+
emitStepDefined(coreStepDefinition);
312+
});
313+
}
290314

291315
afterStepHooks.forEach(this::emitHook);
292316
afterHooks.forEach(this::emitHook);
@@ -432,20 +456,33 @@ private List<PickleStepDefinitionMatch> stepDefinitionMatches(URI uri, Step step
432456
}
433457

434458
void removeScenarioScopedGlue() {
435-
removeScenarioScopedGlue(beforeHooks);
436-
removeScenarioScopedGlue(beforeStepHooks);
437-
removeScenarioScopedGlue(afterHooks);
438-
removeScenarioScopedGlue(afterStepHooks);
439-
removeScenarioScopedGlue(stepDefinitions);
440-
removeScenarioScopedGlue(dataTableTypeDefinitions);
441-
removeScenarioScopedGlue(docStringTypeDefinitions);
442-
removeScenarioScopedGlue(parameterTypeDefinitions);
443-
removeScenarioScopedGlue(defaultParameterTransformers);
444-
removeScenarioScopedGlue(defaultDataTableEntryTransformers);
445-
removeScenarioScopedGlue(defaultDataTableCellTransformers);
446-
447-
stepDefinitionsByPattern.clear();
448-
459+
boolean dirty = false;
460+
dirty |= removeScenarioScopedGlue(beforeHooks);
461+
dirty |= removeScenarioScopedGlue(beforeStepHooks);
462+
dirty |= removeScenarioScopedGlue(afterHooks);
463+
dirty |= removeScenarioScopedGlue(afterStepHooks);
464+
if (removeScenarioScopedGlue(stepDefinitions)) {
465+
dirty = true;
466+
dirtyCache = true;
467+
}
468+
if (removeScenarioScopedGlue(dataTableTypeDefinitions)) {
469+
dirty = true;
470+
dirtyCache = true;
471+
}
472+
if (removeScenarioScopedGlue(docStringTypeDefinitions)) {
473+
dirty = true;
474+
dirtyCache = true;
475+
}
476+
if (removeScenarioScopedGlue(parameterTypeDefinitions)) {
477+
dirty = true;
478+
dirtyCache = true;
479+
}
480+
dirty |= removeScenarioScopedGlue(defaultParameterTransformers);
481+
dirty |= removeScenarioScopedGlue(defaultDataTableEntryTransformers);
482+
dirty |= removeScenarioScopedGlue(defaultDataTableCellTransformers);
483+
if (dirty) {
484+
stepDefinitionsByPattern.clear();
485+
}
449486
}
450487

451488
private boolean removeScenarioScopedGlue(Iterable<?> glues) {

Diff for: ‎cucumber-core/src/main/java/io/cucumber/core/runner/Runner.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@
2121
import java.net.URI;
2222
import java.util.ArrayList;
2323
import java.util.Collection;
24+
import java.util.HashMap;
2425
import java.util.List;
2526
import java.util.Locale;
27+
import java.util.Map;
2628
import java.util.Objects;
2729
import java.util.stream.Collectors;
2830

@@ -39,6 +41,7 @@ public final class Runner {
3941
private final Collection<? extends Backend> backends;
4042
private final Options runnerOptions;
4143
private final ObjectFactory objectFactory;
44+
private final Map<String, Locale> localeCache = new HashMap<>();
4245
private List<SnippetGenerator> snippetGenerators;
4346

4447
public Runner(
@@ -80,7 +83,7 @@ public void runPickle(Pickle pickle) {
8083

8184
private Locale localeForPickle(Pickle pickle) {
8285
String language = pickle.getLanguage();
83-
return new Locale(language);
86+
return localeCache.computeIfAbsent(language, (lang) -> new Locale(language));
8487
}
8588

8689
public void runBeforeAllHooks() {

Diff for: ‎cucumber-core/src/test/java/io/cucumber/core/runner/CachingGlueTest.java

+19-19
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import io.cucumber.core.gherkin.Feature;
1717
import io.cucumber.core.gherkin.Step;
1818
import io.cucumber.core.runtime.TimeServiceEventBus;
19-
import io.cucumber.core.stepexpression.StepTypeRegistry;
2019
import io.cucumber.cucumberexpressions.ParameterByTypeTransformer;
2120
import io.cucumber.cucumberexpressions.ParameterType;
2221
import io.cucumber.datatable.DataTable;
@@ -33,6 +32,7 @@
3332
import java.time.Clock;
3433
import java.util.ArrayList;
3534
import java.util.List;
35+
import java.util.Locale;
3636
import java.util.Optional;
3737
import java.util.UUID;
3838
import java.util.stream.Collectors;
@@ -53,7 +53,7 @@
5353

5454
class CachingGlueTest {
5555

56-
private final StepTypeRegistry stepTypeRegistry = new StepTypeRegistry(ENGLISH);
56+
public static final Locale LANGUAGE = ENGLISH;
5757
private final CachingGlue glue = new CachingGlue(new TimeServiceEventBus(Clock.systemUTC(), UUID::randomUUID));
5858

5959
@Test
@@ -70,7 +70,7 @@ void throws_duplicate_error_on_dupe_stepdefs() {
7070

7171
DuplicateStepDefinitionException exception = assertThrows(
7272
DuplicateStepDefinitionException.class,
73-
() -> glue.prepareGlue(stepTypeRegistry));
73+
() -> glue.prepareGlue(LANGUAGE));
7474
assertThat(exception.getMessage(), equalTo("Duplicate step definitions in foo.bf:10 and bar.bf:90"));
7575
}
7676

@@ -81,7 +81,7 @@ void throws_on_duplicate_default_parameter_transformer() {
8181

8282
DuplicateDefaultParameterTransformers exception = assertThrows(
8383
DuplicateDefaultParameterTransformers.class,
84-
() -> glue.prepareGlue(stepTypeRegistry));
84+
() -> glue.prepareGlue(LANGUAGE));
8585
assertThat(exception.getMessage(), equalTo("" +
8686
"There may not be more then one default parameter transformer. Found:\n" +
8787
" - mocked default parameter transformer\n" +
@@ -95,7 +95,7 @@ void throws_on_duplicate_default_table_entry_transformer() {
9595

9696
DuplicateDefaultDataTableEntryTransformers exception = assertThrows(
9797
DuplicateDefaultDataTableEntryTransformers.class,
98-
() -> glue.prepareGlue(stepTypeRegistry));
98+
() -> glue.prepareGlue(LANGUAGE));
9999
assertThat(exception.getMessage(), equalTo("" +
100100
"There may not be more then one default data table entry. Found:\n" +
101101
" - mocked default data table entry transformer\n" +
@@ -109,7 +109,7 @@ void throws_on_duplicate_default_table_cell_transformer() {
109109

110110
DuplicateDefaultDataTableCellTransformers exception = assertThrows(
111111
DuplicateDefaultDataTableCellTransformers.class,
112-
() -> glue.prepareGlue(stepTypeRegistry));
112+
() -> glue.prepareGlue(LANGUAGE));
113113
assertThat(exception.getMessage(), equalTo("" +
114114
"There may not be more then one default table cell transformers. Found:\n" +
115115
" - mocked default data table cell transformer\n" +
@@ -135,7 +135,7 @@ void removes_glue_that_is_scenario_scoped() {
135135
glue.addDefaultDataTableCellTransformer(new MockedDefaultDataTableCellTransformer());
136136
glue.addDefaultDataTableEntryTransformer(new MockedDefaultDataTableEntryTransformer());
137137

138-
glue.prepareGlue(stepTypeRegistry);
138+
glue.prepareGlue(LANGUAGE);
139139

140140
assertAll(
141141
() -> assertThat(glue.getStepDefinitions().size(), is(equalTo(1))),
@@ -191,7 +191,7 @@ void returns_match_from_cache_if_single_found() throws AmbiguousStepDefinitionsE
191191
StepDefinition stepDefinition2 = new MockedStepDefinition("^pattern2");
192192
glue.addStepDefinition(stepDefinition1);
193193
glue.addStepDefinition(stepDefinition2);
194-
glue.prepareGlue(stepTypeRegistry);
194+
glue.prepareGlue(LANGUAGE);
195195

196196
URI uri = URI.create("file:path/to.feature");
197197
String stepText = "pattern1";
@@ -219,7 +219,7 @@ void returns_match_from_cache_for_step_with_table() throws AmbiguousStepDefiniti
219219
StepDefinition stepDefinition2 = new MockedStepDefinition("^pattern2", DataTable.class);
220220
glue.addStepDefinition(stepDefinition1);
221221
glue.addStepDefinition(stepDefinition2);
222-
glue.prepareGlue(stepTypeRegistry);
222+
glue.prepareGlue(LANGUAGE);
223223

224224
URI uri = URI.create("file:path/to.feature");
225225
String stepText = "pattern1";
@@ -260,7 +260,7 @@ void returns_match_from_cache_for_ste_with_doc_string() throws AmbiguousStepDefi
260260
StepDefinition stepDefinition2 = new MockedStepDefinition("^pattern2", String.class);
261261
glue.addStepDefinition(stepDefinition1);
262262
glue.addStepDefinition(stepDefinition2);
263-
glue.prepareGlue(stepTypeRegistry);
263+
glue.prepareGlue(LANGUAGE);
264264

265265
URI uri = URI.create("file:path/to.feature");
266266
String stepText = "pattern1";
@@ -304,7 +304,7 @@ void returns_fresh_match_from_cache_after_evicting_scenario_scoped() throws Ambi
304304

305305
StepDefinition stepDefinition1 = new MockedScenarioScopedStepDefinition("^pattern1");
306306
glue.addStepDefinition(stepDefinition1);
307-
glue.prepareGlue(stepTypeRegistry);
307+
glue.prepareGlue(LANGUAGE);
308308

309309
PickleStepDefinitionMatch pickleStepDefinitionMatch = glue.stepDefinitionMatch(uri, pickleStep1);
310310
assertThat(((CoreStepDefinition) pickleStepDefinitionMatch.getStepDefinition()).getStepDefinition(),
@@ -314,7 +314,7 @@ void returns_fresh_match_from_cache_after_evicting_scenario_scoped() throws Ambi
314314

315315
StepDefinition stepDefinition2 = new MockedScenarioScopedStepDefinition("^pattern1");
316316
glue.addStepDefinition(stepDefinition2);
317-
glue.prepareGlue(stepTypeRegistry);
317+
glue.prepareGlue(LANGUAGE);
318318

319319
PickleStepDefinitionMatch pickleStepDefinitionMatch2 = glue.stepDefinitionMatch(uri, pickleStep1);
320320
assertThat(((CoreStepDefinition) pickleStepDefinitionMatch2.getStepDefinition()).getStepDefinition(),
@@ -347,7 +347,7 @@ void disposes_of_scenario_scoped_beans() {
347347
MockedDefaultParameterTransformer defaultParameterTransformer = new MockedDefaultParameterTransformer();
348348
glue.addDefaultParameterTransformer(defaultParameterTransformer);
349349

350-
glue.prepareGlue(stepTypeRegistry);
350+
glue.prepareGlue(LANGUAGE);
351351
glue.removeScenarioScopedGlue();
352352

353353
assertThat(stepDefinition.isDisposed(), is(true));
@@ -371,15 +371,15 @@ void returns_no_match_after_evicting_scenario_scoped() throws AmbiguousStepDefin
371371

372372
StepDefinition stepDefinition1 = new MockedScenarioScopedStepDefinition("^pattern1");
373373
glue.addStepDefinition(stepDefinition1);
374-
glue.prepareGlue(stepTypeRegistry);
374+
glue.prepareGlue(LANGUAGE);
375375

376376
PickleStepDefinitionMatch pickleStepDefinitionMatch = glue.stepDefinitionMatch(uri, pickleStep1);
377377
assertThat(((CoreStepDefinition) pickleStepDefinitionMatch.getStepDefinition()).getStepDefinition(),
378378
is(equalTo(stepDefinition1)));
379379

380380
glue.removeScenarioScopedGlue();
381381

382-
glue.prepareGlue(stepTypeRegistry);
382+
glue.prepareGlue(LANGUAGE);
383383

384384
PickleStepDefinitionMatch pickleStepDefinitionMatch2 = glue.stepDefinitionMatch(uri, pickleStep1);
385385
assertThat(pickleStepDefinitionMatch2, nullValue());
@@ -393,7 +393,7 @@ void throws_ambiguous_steps_def_exception_when_many_patterns_match() {
393393
glue.addStepDefinition(stepDefinition1);
394394
glue.addStepDefinition(stepDefinition2);
395395
glue.addStepDefinition(stepDefinition3);
396-
glue.prepareGlue(stepTypeRegistry);
396+
glue.prepareGlue(LANGUAGE);
397397

398398
URI uri = URI.create("file:path/to.feature");
399399

@@ -480,7 +480,7 @@ void emits_hook_messages_to_bus() {
480480
glue.addBeforeStepHook(new MockedScenarioScopedHookDefinition());
481481
glue.addAfterStepHook(new MockedScenarioScopedHookDefinition());
482482

483-
glue.prepareGlue(stepTypeRegistry);
483+
glue.prepareGlue(LANGUAGE);
484484
assertThat(events.size(), is(4));
485485
}
486486

@@ -497,7 +497,7 @@ void parameterTypeDefinition_without_source_reference_emits_parameterType_with_e
497497
glue.addParameterType(new MockedParameterTypeDefinition());
498498

499499
// When
500-
glue.prepareGlue(stepTypeRegistry);
500+
glue.prepareGlue(LANGUAGE);
501501

502502
// Then
503503
assertThat(events.size(), is(1));
@@ -519,7 +519,7 @@ void parameterTypeDefinition_with_source_reference_emits_parameterType_with_non_
519519
glue.addParameterType(new MockedParameterTypeDefinitionWithSourceReference());
520520

521521
// When
522-
glue.prepareGlue(stepTypeRegistry);
522+
glue.prepareGlue(LANGUAGE);
523523

524524
// Then
525525
assertThat(events.size(), is(1));

0 commit comments

Comments
 (0)