cucumber-jvm icon indicating copy to clipboard operation
cucumber-jvm copied to clipboard

Codegen generates inefficient Java code for `Long` and `Boolean` mandatory parameters

Open jkronegg opened this issue 8 months ago • 6 comments

👓 What did you see?

For Long parameters, the codegen generates Java code which looks like this:

private final Long seconds;
private final Long nanos;

public Duration(
    Long seconds,
    Long nanos
) {
    this.seconds = requireNonNull(seconds, "Duration.seconds cannot be null");
    this.nanos = requireNonNull(nanos, "Duration.nanos cannot be null");
}

The parameters of type Long are most of the time required to be non-null, but still we are storing them into a object, not a primitive type.

This occurs for the following generated types:

  • Duration (both seconds and nanos fields)
  • Location (line field but not column field)
  • TestCaseStarted (attempt field)
  • Timestamp (both seconds and nanos fields)

Note that Group contains a non-mandatory field start.

The Boolean type is also impacted by the same issue, i.e. for the generated classes :

  • ParameterType (preferForRegularExpressionMatch and useForSnippets fields)
  • TestCaseFinished (willBeRetried field)
  • TestRunFinished (success field)

✅ What did you expect to see?

If the java.java.erb code generator see a Long which is mandatory, it should generate a long primitive type, so that no conversion is done. This will avoid back and forth Long <-> long conversions, so will lead to faster code.

The same apply to Boolean type.

This would probably cause a breaking change because all interfaces to the impacted classes will change.

📦 Which tool/library version are you using?

gherkin 31.0.0 (which uses messages-27.2.0)

🔬 How could we reproduce it?

N/A (Look at the generated code).

📚 Any additional context?

The performance impact has been detected while working on https://github.com/cucumber/gherkin/issues/361, using IntelliJ Profiler (it's hard to see the real impact because there is a lot of generated classes which suffer from this syndrome).

jkronegg avatar Feb 06 '25 13:02 jkronegg