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

[incubator-kie-issues#781] Improve performance of InfixOpNode #5632

Closed
wants to merge 1 commit into from

Conversation

gitgabrio
Copy link
Contributor

Fixes apache/incubator-kie-issues#781

This PR

  1. restore "if-driven logic" inside InfixExecutors
  2. remove usage of mapped functions
  3. remove unused classes

Benchmarks done with updated version of apache/incubator-kie-benchmarks#281

Pre-refactopring code
results_pre.csv

Updated InfixExecutors

results_post.csv

toReturn.put(new ClassIdentifierTuple(Duration.class, OffsetTime.class), (parameters, ctx) ->
((OffsetTime) parameters.getRight()).plus((Duration) parameters.getLeft()));
toReturn.put(new ClassIdentifierTuple(Temporal.class, Temporal.class), (parameters, ctx) -> {
} else if (left instanceof String && right instanceof String) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I'm underestimating the problem, but this if-else chain seems unnecessarily complicated. Give me a bit more time to review/rework it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mariofusco
Thanks, but I think it is better to discuss it together, instead of keep doing the same wheel twice without clear comprehension of what's going on.

@mariofusco
Copy link
Contributor

@gitgabrio If it's ok for you, let's close this PR and replace it with this one #5634

@gitgabrio gitgabrio closed this Dec 20, 2023
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.

DMN - Improve performance of InfixOpNode
2 participants