Skip to content

[SPARK-47384][BUILD] Upgrade RoaringBitmap to 1.0.5 - #45507

Closed
panbingkun wants to merge 2 commits into
apache:masterfrom
panbingkun:SPARK-47384
Closed

panbingkun wants to merge 2 commits into
apache:masterfrom
panbingkun:SPARK-47384

Conversation

@panbingkun

@panbingkun panbingkun commented Mar 14, 2024 •

Copy link
Copy Markdown
Contributor

What changes were proposed in this pull request?

The pr aims to upgrade RoaringBitmap from 1.0.1 to 1.0.5.

Why are the changes needed?

Release notes: https://github.com/RoaringBitmap/RoaringBitmap/releases/tag/1.0.5
This version includes some bugs fixed, eg:

  • fix roaringbitmap - batchiterator's advanceIfNeeded to handle run lengths of zero by
  • fix RangeBitmap#between bug in full section after empty section

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass GA.

Was this patch authored or co-authored using generative AI tooling?

No.

@github-actions github-actions Bot added the BUILD label Mar 14, 2024
@panbingkun

panbingkun commented Mar 14, 2024 •

Copy link
Copy Markdown
Contributor Author

Let it run through GA first, and then I will add the benchmark(org.apache.spark.MapStatusesConvertBenchmark) results.
JDK 17: https://github.com/panbingkun/spark/actions/runs/8275959150
JDK 21: https://github.com/panbingkun/spark/actions/runs/8275963971

@github-actions github-actions Bot added the CORE label Mar 14, 2024
@panbingkun
panbingkun marked this pull request as ready for review March 14, 2024 11:47
Num Maps: 50000 Fetch partitions:1500 1767 1830 55 0.0 1767363076.0 0.4X
Num Maps: 50000 Fetch partitions:500 714 716 2 0.0 713899011.0 1.0X
Num Maps: 50000 Fetch partitions:1000 1602 1647 59 0.0 1602358288.0 0.4X
Num Maps: 50000 Fetch partitions:1500 2517 2538 22 0.0 2517027078.0 0.3X

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.

Although we care only the relative ratio, it seems that this release is a little slower than before?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have noticed this as well. It may be caused by some fluctuations. Wait a little bit and I will run it a few more times to see the performance data.

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.

New result seems to be better. Thank you for re-run. Let's merge this.

================================================================================================
MapStatuses Convert Benchmark
================================================================================================

OpenJDK 64-Bit Server VM 17.0.10+7-LTS on Linux 6.5.0-1016-azure
AMD EPYC 7763 64-Core Processor
MapStatuses Convert:                      Best Time(ms)   Avg Time(ms)   Stdev(ms)    Rate(M/s)   Per Row(ns)   Relative
------------------------------------------------------------------------------------------------------------------------
Num Maps: 50000 Fetch partitions:500                719            731          18          0.0   719070889.0       1.0X
Num Maps: 50000 Fetch partitions:1000              1456           1468          11          0.0  1455501002.0       0.5X
Num Maps: 50000 Fetch partitions:1500              2207           2223          18          0.0  2207061425.0       0.3X

@panbingkun

Copy link
Copy Markdown
Contributor Author

Just for the record, new the benchmark(org.apache.spark.MapStatusesConvertBenchmark) results.
JDK17: https://github.com/panbingkun/spark/actions/runs/8282758077
JDK21: https://github.com/panbingkun/spark/actions/runs/8282763722

@dongjoon-hyun

Copy link
Copy Markdown
Member

Merged to master for Apache Spark 4.0.0.

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.

2 participants