Faster caching glue

[mpkorstanje]

🤔 What's changed?

Work in progress. Parking this in a pull request so I won't forget about it.

Currently the CachingGlue is rather limited in what it can cache. Step definitions are derived from parameter types, and because these can be Disposable, this means that we dispose of the whole cache after each scenario.

But glue can be reused between scenarios if

• The step definitions are non-disposable.
• The step definitions are the same as the previous scenario
• The the language of the scenario is the same.

⚡️ What's your motivation?

🏷️ What kind of change is this?

• 📖 Documentation (improvements without changing code)
• 🏦 Refactoring/debt/DX (improvement to code design, tooling, etc. without changing behaviour)
• 🐛 Bug fix (non-breaking change which fixes a defect)
• ⚡ New feature (non-breaking change which adds new behaviour)
• 💥 Breaking change (incompatible changes to the API)

♻️ Anything particular you want feedback on?

📋 Checklist:

• I agree to respect and uphold the Cucumber Community Code of Conduct
• I've changed the behaviour of the code
  • I have added/updated tests to cover my changes.
• My change requires a change to the documentation.
  • I have updated the documentation accordingly.
• Users should know about my change
  • I have added an entry to the "Unreleased" section of the CHANGELOG, linking to this pull request.

---

This text was originally generated from a template, then edited by hand. You can modify the template here.

[jkronegg]

That would be great! (CachingGlue.prepareGlue() lasts 50% of the total test duration on my usual test project, ~600 test scenarios)

[jkronegg]

I think the glue could also be reused if:

• the parameterTypeDefinitions do not change
• the docStringTypeDefinitions do not change
• the dataTableTypeDefinitions do not change

I pushed the branch faster-caching-glue-julien to tentatively implement the rules you proposed, but I ran into an issue: if the step definitions are cached, the StepDefined event is not emitted as duplicate anymore, making one test to fail, io.cucumber.core.runtime.RuntimeTest#generates_events_for_glue_and_scenario_scoped_glue (which expects multiple StepDefined events to be thrown). I'm not sure if the test-case can be changed... (is this an API change?)

I have a Cucumber plugin which gets a lot of StepDefined events: I need to remove the duplicates, so it would not be an issue when the duplicate events are not emitted.

When changing this test-case to await only 2 StepDefined events instead of 4, the build passes and on my usual test project (~600 test scenarios, 230 step definitions), I get incredible results:

version	CachingGlue.prepareGlue() impact on test project (IntelliJ profiler)	total Cucumber test duration on test project
7.20.1	54% of execution time	6.1 seconds
faster-caching-glue-julien	0.5% of execution time	3.3 seconds

More or less twice faster... 😁

[mpkorstanje]

Unfortunately, what I don't like about my current solution is that it involves way too much code. It is too easy to mess it up and piles complexity on top of complexity. And most of that complexity is unnecessary in my opinion.

If we only had to consider cucumber-java this would be a trivial problem. All step definitions are static so we would only have to use the locale as the cache key. But the workarounds used to make cucumber-java8 work add a lot of complexity. It was originally hacked in and now about one third of the users use it. So we can't get rid of it easily, quickly, and possibly not at all.

With cucumber-java8 step definitions are registered just after the test has started. So each step definition is unique for each scenario. And because step definitions retain a reference to a lambda, so they're state full and must be disposed between scenarios.

And that makes for a really complicated cache key.

Then to make matters worse, each time a step definition is defined Cucumber emits a message. This generates a lot (scenarios * step definitions) messages. These messages contain an identifier that is used to emit another a message when a step is executed. So we have more state being created when a scenario is executed, and that state should then be reused for others scenarios. And that possibly happens in parallel.

So what I'm hoping to do is change the design of Cucumber so step definitions can be discovered at different times during the execution process. Each of these would be treated as a distinct set. This would then allow a cache key to be shaped like cache-key = (locale, {glue, glue, glue, ...}).

Then to ensure that lambda step definitions consistently re-create the same key, they'd have to be created without any state. Which means the reference to the lambda they invoke must be stored in some execution context that is bound to the scenario scope instead (something like JUnit 5).

And then, once we can ensure the same step definitions is created each time, we can also ensure a message for each step definition is only emitted once. The the messages would be emitted when the cache entry is created.

So I'm looking for an architecture where if done correctly, the solution to those problems just falls out of it.

[jkronegg]

When your start caching data, this means that your are not following the Don't Repeat Yourself principle anymore. But of course sometimes you don't have the choice...

Regarding the cucumber-java8 usage: it's maybe imported by mistake, but not used in practice (I did this at the beginning: "oh java8, that's modern, i'll take this one"). Can we detect this pattern?