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

Speed up BinaryOperatorTransformFunction evaluation to use PredicateEvaluator #14619

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xiangfu0
Copy link
Contributor

@xiangfu0 xiangfu0 commented Dec 6, 2024

For BinaryOperatorTransformFunction, we don't need to read the values then compare but use dictionary.

Further optimization could be using predicate evaluator.

@xiangfu0 xiangfu0 requested a review from Jackie-Jiang December 6, 2024 23:12
@xiangfu0 xiangfu0 force-pushed the BinaryOperatorTransformFunction-optimization branch from 377045c to eaf6a9d Compare December 6, 2024 23:15
@codecov-commenter
Copy link

codecov-commenter commented Dec 6, 2024

Codecov Report

Attention: Patch coverage is 59.06040% with 61 lines in your changes missing coverage. Please review.

Project coverage is 64.00%. Comparing base (59551e4) to head (c2c2707).
Report is 1470 commits behind head on master.

Files with missing lines Patch % Lines
...form/function/BinaryOperatorTransformFunction.java 57.93% 47 Missing and 14 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #14619      +/-   ##
============================================
+ Coverage     61.75%   64.00%   +2.25%     
- Complexity      207     1605    +1398     
============================================
  Files          2436     2700     +264     
  Lines        133233   148938   +15705     
  Branches      20636    22845    +2209     
============================================
+ Hits          82274    95331   +13057     
- Misses        44911    46612    +1701     
- Partials       6048     6995     +947     
Flag Coverage Δ
custom-integration1 100.00% <ø> (+99.99%) ⬆️
integration 100.00% <ø> (+99.99%) ⬆️
integration1 100.00% <ø> (+99.99%) ⬆️
integration2 0.00% <ø> (ø)
java-11 63.97% <57.71%> (+2.26%) ⬆️
java-21 63.89% <59.06%> (+2.27%) ⬆️
skip-bytebuffers-false 64.00% <59.06%> (+2.25%) ⬆️
skip-bytebuffers-true 63.85% <59.06%> (+36.12%) ⬆️
temurin 64.00% <59.06%> (+2.25%) ⬆️
unittests 64.00% <59.06%> (+2.25%) ⬆️
unittests1 56.29% <59.06%> (+9.40%) ⬆️
unittests2 34.41% <0.00%> (+6.67%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@xiangfu0 xiangfu0 force-pushed the BinaryOperatorTransformFunction-optimization branch 4 times, most recently from 9778fbb to 5164df3 Compare December 11, 2024 19:27
Copy link
Contributor

@Jackie-Jiang Jackie-Jiang left a comment

Choose a reason for hiding this comment

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

Is there a way to verify if this optimization is applied?

@xiangfu0 xiangfu0 force-pushed the BinaryOperatorTransformFunction-optimization branch 3 times, most recently from fc21c88 to ff991bf Compare December 13, 2024 00:58
@xiangfu0 xiangfu0 force-pushed the BinaryOperatorTransformFunction-optimization branch 3 times, most recently from 1a1bc83 to 9e0a4d7 Compare December 13, 2024 23:32
@xiangfu0 xiangfu0 force-pushed the BinaryOperatorTransformFunction-optimization branch from 9e0a4d7 to c2c2707 Compare December 14, 2024 01:47
@xiangfu0 xiangfu0 changed the title use dictionary for BinaryOperatorTransformFunction when possible Speed up BinaryOperatorTransformFunction evaluation to use PredicateEvaluator Dec 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants