Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Expression creation performance #185

Closed
jkronegg opened this issue Nov 28, 2022 · 2 comments · Fixed by #187
Closed

Improve Expression creation performance #185

jkronegg opened this issue Nov 28, 2022 · 2 comments · Fixed by #187

Comments

@jkronegg
Copy link
Contributor

👓 What did you see?

I have a project tested with Cucumber 7.9.0:

  • about 400 test scenarios
  • about 150 step defs
  • about 30 parameter types

The Cucumber tests run in about 10.2 seconds on average : that's not so bad (we have plain Java, no Spring, no Mockito). But we wanted to have an even faster feedback loop for the developer.

Thus, we made some profiling and found that ExpressionFactory.createExpression(String) was eating a lot of CPU (this method is called more than 64'000 times for our project).

✅ What did you expect to see?

We improved the code with two different variants and made some benchmark using JMH microbenchmark framework :

  • creationExpression0 is the original method
  • creationExpression1 is a small update in which we replaced some regexp conditions by startsWith/endsWith conditions
  • creationExpression2 is a full rewrite with regexp conditions replaced by character testing conditions

The benchmark results are the following :

Benchmark Mode Cnt Score Error Units
ExpressionFactoryBenchmark.createExpression0 thrpt 25 234230,747 ± 4693,108 ops/s
ExpressionFactoryBenchmark.createExpression1 thrpt 25 347290,522 ± 4297,255 ops/s
ExpressionFactoryBenchmark.createExpression2 thrpt 25 356142,244 ± 9833,928 ops/s

When using the createExpression2 variant on our project, the cucumber tests run in 9.0 seconds on average. That's a 1.2 second improvement (12%).

Thus, we suggest to replace current ExpressionFactory.createExpression(String) implementation by the one from our createExpression2.

You can find in annex the three different variants, unit testing to ensure that all three variants behave the same and the JMH benchmark code.

cucumberexpressions.zip

📦 Which tool/library version are you using?

I'm using Cucumber 7.9.0 with the following Maven dependencies:

    <dependency>
        <groupId>io.cucumber</groupId>
        <artifactId>cucumber-java</artifactId>
        <version>${cucumber.version}</version>
        <scope>test</scope>
        <exclusions>
            <exclusion><!-- version 1.1.2 from cucumber conflicts with version 1.1.0 from junit -->
                <groupId>org.apiguardian</groupId>
                <artifactId>apiguardian-api</artifactId>
            </exclusion>
        </exclusions>
    </dependency>
    <dependency>
        <groupId>io.cucumber</groupId>
        <artifactId>cucumber-junit-platform-engine</artifactId>
        <version>${cucumber.version}</version>
        <scope>test</scope>
        <exclusions>
            <exclusion><!-- conflicts with the version from junit-platform-suite -->
                <groupId>org.junit.platform</groupId>
                <artifactId>junit-platform-engine</artifactId>
            </exclusion>
        </exclusions>
    </dependency>
    <dependency>
        <groupId>io.cucumber</groupId>
        <artifactId>cucumber-picocontainer</artifactId>
        <version>${cucumber.version}</version>
        <scope>test</scope>
    </dependency>

🔬 How could we reproduce it?

The JMH micro benchmark is provided in ExpressionFactoryBenchmark.

Steps to reproduce the behavior:

  1. Create a blank Maven project with the pom.xml above and JUnit5
  2. Run the micro benchmark
@mpkorstanje mpkorstanje transferred this issue from cucumber/common Nov 28, 2022
@mpkorstanje
Copy link
Contributor

Cheers. Looks usable.

Somewhat related to cucumber/cucumber-jvm#2035 which explains that expressions are rebuild for every scenario.

@mpkorstanje
Copy link
Contributor

@jkronegg since fixing cucumber/cucumber-jvm#2035 is a very long term project, would you like to send a PR with your improvements?

The optimized methods look a bit messy but with sufficient comments to explain what has been optimized I reckon the optimizations wil stick around.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants