Skip to content

[MINOR] refactor: avoid unnecessary bitmap clone and AND - #1442

Merged
xianjingfeng merged 3 commits into
apache:masterfrom
EnricoMi:avoid-bitmap-and
Jan 12, 2024
Merged

[MINOR] refactor: avoid unnecessary bitmap clone and AND#1442
xianjingfeng merged 3 commits into
apache:masterfrom
EnricoMi:avoid-bitmap-and

Conversation

@EnricoMi

@EnricoMi EnricoMi commented Jan 11, 2024

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

Add note that processedBlockIds can be a superset of blockIdBitmap. Add quick check if processedBlockIds is equal to blockIdBitmap. In that case, there is no need to clone and AND the bitmaps.

Why are the changes needed?

The logic is equivalent but more performant when processedBlockIds is not a superset of blockIdBitmap (when no other blocks than the blockIdBitmap are retrieved).

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Existing tests in ShuffleReadClientImplTest cover this.

xianjingfeng
xianjingfeng previously approved these changes Jan 11, 2024

@xianjingfeng xianjingfeng left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. Thanks @EnricoMi

@codecov-commenter

codecov-commenter commented Jan 11, 2024

Copy link
Copy Markdown

Codecov Report

Attention: 7 lines in your changes are missing coverage. Please review.

Comparison is base (5d027c0) 53.38% compared to head (545da70) 54.29%.
Report is 1 commits behind head on master.

Files Patch % Lines
.../java/org/apache/uniffle/common/util/RssUtils.java 0.00% 7 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1442      +/-   ##
============================================
+ Coverage     53.38%   54.29%   +0.91%     
- Complexity     2729     2730       +1     
============================================
  Files           422      402      -20     
  Lines         24046    21686    -2360     
  Branches       2051     2051              
============================================
- Hits          12836    11775    -1061     
+ Misses        10412     9185    -1227     
+ Partials        798      726      -72     

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

@zuston zuston left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM. +1

@xianjingfeng
xianjingfeng merged commit 345048f into apache:master Jan 12, 2024
@zuston

zuston commented Jan 12, 2024

Copy link
Copy Markdown
Member

Thanks @EnricoMi for your contribution.

Welcome to Apache Uniffle community 👏🏻

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.

4 participants