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

EvaluationValue optimization #471

Merged
merged 5 commits into from
May 19, 2024
Merged

Conversation

oswaldobapvicjr
Copy link
Contributor

This PR optimizes the existing method EvaluationValue.nullValue() to return a shared, pre-built EvaluationValue instance (and not create a new object at each call).

Since it's an immutable object, it is safe to reuse the same instance without issues (similar to Collections.emptyList(), or BigDecimal.ZERO, for example).

By reusing the same object, we avoid GC overhead.

@melontini
Copy link

I think booleans should be constant too, since they are only 2 possible states.

@oswaldobapvicjr
Copy link
Contributor Author

oswaldobapvicjr commented May 9, 2024

I found something strange in the constructor EvaluationValue(Object, ExpressionConfiguration) too:

  public EvaluationValue(Object value, ExpressionConfiguration configuration) {

    EvaluationValue converted =
        configuration.getEvaluationValueConverter().convertObject(value, configuration);

    this.value = converted.getValue();
    this.dataType = converted.getDataType();
  }

Here, the converted is already an EvaluationValue, why duplicate it into another object?
Can we act on it too, @uklimaschewski ?

A simple proposal could be to deprecate this constructor and replace it with a factory method, for convenience:

  public static EvaluationValue of(Object value, ExpressionConfiguration configuration) {
    return configuration.getEvaluationValueConverter().convertObject(value, configuration);
  }

Copy link

sonarcloud bot commented May 9, 2024

Quality Gate Passed Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@oswaldobapvicjr oswaldobapvicjr changed the title Optimize EvaluationValue.nullValue() EvaluationValue optimization for null and boolean values May 9, 2024
@oswaldobapvicjr oswaldobapvicjr changed the title EvaluationValue optimization for null and boolean values EvaluationValue optimization May 9, 2024
@uklimaschewski uklimaschewski merged commit 57ba749 into ezylang:main May 19, 2024
2 checks passed
@uklimaschewski
Copy link
Collaborator

Thank you very much

@oswaldobapvicjr oswaldobapvicjr deleted the pr-7 branch May 19, 2024 11:38
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 this pull request may close these issues.

3 participants