Skip to content

Commit 1b1c4f1

Browse files
author
Julien Kronegg
committed
fix: finished faster prepare glue for #2902 
1 parent 39a24aa commit 1b1c4f1

File tree

3 files changed

+95
-55
lines changed

3 files changed

+95
-55
lines changed

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

Lines changed: 72 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,9 @@ final class CachingGlue implements Glue {
8484

8585
private final EventBus bus;
8686
private StepTypeRegistry stepTypeRegistry;
87+
private Locale locale = null;
88+
private StepExpressionFactory stepExpressionFactory = null;
89+
private boolean dirtyCache = false;
8790

8891
CachingGlue(EventBus bus) {
8992
this.bus = bus;
@@ -104,6 +107,7 @@ public void addAfterAllHook(StaticHookDefinition afterAllHook) {
104107
@Override
105108
public void addStepDefinition(StepDefinition stepDefinition) {
106109
stepDefinitions.add(stepDefinition);
110+
dirtyCache = true;
107111
}
108112

109113
@Override
@@ -133,11 +137,13 @@ public void addAfterStepHook(HookDefinition hookDefinition) {
133137
@Override
134138
public void addParameterType(ParameterTypeDefinition parameterType) {
135139
parameterTypeDefinitions.add(parameterType);
140+
dirtyCache = true;
136141
}
137142

138143
@Override
139144
public void addDataTableType(DataTableTypeDefinition dataTableType) {
140145
dataTableTypeDefinitions.add(dataTableType);
146+
dirtyCache = true;
141147
}
142148

143149
@Override
@@ -163,6 +169,7 @@ public void addDefaultDataTableCellTransformer(
163169
@Override
164170
public void addDocStringType(DocStringTypeDefinition docStringType) {
165171
docStringTypeDefinitions.add(docStringType);
172+
dirtyCache = true;
166173
}
167174

168175
List<StaticHookDefinition> getBeforeAllHooks() {
@@ -232,21 +239,36 @@ List<DocStringTypeDefinition> getDocStringTypeDefinitions() {
232239
}
233240

234241
StepTypeRegistry getStepTypeRegistry() {
235-
return null;
242+
return stepTypeRegistry;
236243
}
237244

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

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

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

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

292316
afterStepHooks.forEach(this::emitHook);
293317
afterHooks.forEach(this::emitHook);
@@ -449,20 +473,33 @@ private List<PickleStepDefinitionMatch> stepDefinitionMatches(URI uri, Step step
449473
}
450474

451475
void removeScenarioScopedGlue() {
452-
removeScenarioScopedGlue(beforeHooks);
453-
removeScenarioScopedGlue(beforeStepHooks);
454-
removeScenarioScopedGlue(afterHooks);
455-
removeScenarioScopedGlue(afterStepHooks);
456-
removeScenarioScopedGlue(stepDefinitions);
457-
removeScenarioScopedGlue(dataTableTypeDefinitions);
458-
removeScenarioScopedGlue(docStringTypeDefinitions);
459-
removeScenarioScopedGlue(parameterTypeDefinitions);
460-
removeScenarioScopedGlue(defaultParameterTransformers);
461-
removeScenarioScopedGlue(defaultDataTableEntryTransformers);
462-
removeScenarioScopedGlue(defaultDataTableCellTransformers);
463-
464-
stepDefinitionsByPattern.clear();
465-
476+
boolean dirty = false;
477+
dirty |= removeScenarioScopedGlue(beforeHooks);
478+
dirty |= removeScenarioScopedGlue(beforeStepHooks);
479+
dirty |= removeScenarioScopedGlue(afterHooks);
480+
dirty |= removeScenarioScopedGlue(afterStepHooks);
481+
if (removeScenarioScopedGlue(stepDefinitions)) {
482+
dirty = true;
483+
dirtyCache = true;
484+
}
485+
if (removeScenarioScopedGlue(dataTableTypeDefinitions)) {
486+
dirty = true;
487+
dirtyCache = true;
488+
}
489+
if (removeScenarioScopedGlue(docStringTypeDefinitions)) {
490+
dirty = true;
491+
dirtyCache = true;
492+
}
493+
if (removeScenarioScopedGlue(parameterTypeDefinitions)) {
494+
dirty = true;
495+
dirtyCache = true;
496+
}
497+
dirty |= removeScenarioScopedGlue(defaultParameterTransformers);
498+
dirty |= removeScenarioScopedGlue(defaultDataTableEntryTransformers);
499+
dirty |= removeScenarioScopedGlue(defaultDataTableCellTransformers);
500+
if (dirty) {
501+
stepDefinitionsByPattern.clear();
502+
}
466503
}
467504

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

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

Lines changed: 4 additions & 1 deletion
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() {

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

Lines changed: 19 additions & 19 deletions
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)