Skip to content

Conversation

@LuciferYang
Copy link
Contributor

What changes were proposed in this pull request?

This pr aims upgrade RoaringBitmap to 0.9.28

Why are the changes needed?

This version bring bug fix and optimization, for example, andCardinality/orCardinality without materialising a bitmap, optimized andCardinality/orCardinality performance.

The changes between 0.9.25 and 0.9.28 as follows:

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Pass Github Actions

@github-actions github-actions bot added the BUILD label May 30, 2022
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

Thank you, @LuciferYang .
Could you run MapStatusesSerDeserBenchmark like #29233?

@LuciferYang
Copy link
Contributor Author

Thank you, @LuciferYang . Could you run MapStatusesSerDeserBenchmark like #29233?

ok

@github-actions github-actions bot added the CORE label May 30, 2022
@LuciferYang
Copy link
Contributor Author

3da048a update bench result of MapStatusesSerDeserBenchmark, but I found that the cpu frequency of the machine changed for Java8 & Java 11:

  • Java 11: 8370C CPU @ 2.80GHz -> 8272CL CPU @ 2.60GHz
  • Java 8: 8272CL CPU @ 2.60GHz -> 8370C CPU @ 2.80GHz

need to re-run to keep them the same @dongjoon-hyun ?

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM. Thank you, @LuciferYang .
The difference is okay because we cannot control it.
Merged to master for Apache Spark 3.4.

@LuciferYang
Copy link
Contributor Author

Thank you, @LuciferYang . Could you run MapStatusesSerDeserBenchmark like #29233?

@dongjoon-hyun Sorry to bother you, I used Intellij Profiler to run MapStatusesSerDeserBenchmark today. But I found that MapStatusesSerDeserBenchmark doesn't seem to use RoaringBitmap related APIs.

It seems that MapStatusesSerDeserBenchmark uses CompressedMapStatus to test serDe, if HighlyCompressedMapStatus is used, RoaringBitmap related APIs will be tested?

tracker.registerMapOutput(shuffleId, i,
new CompressedMapStatus(BlockManagerId(s"node$i", s"node$i.spark.apache.org", 1000),
Array.fill(blockSize) {
// Creating block size ranging from 0byte to 1GB
(r.nextDouble() * 1024 * 1024 * 1024).toLong
}, i))
}

@dongjoon-hyun
Copy link
Member

Thank you for pinging me, @LuciferYang . Let me check it.

@dongjoon-hyun
Copy link
Member

Oh, you are right. Sorry for misleading you so far, @LuciferYang . :(
Yes, RoaringBitmap is used in HighlyCompressedMapStatus only.

@LuciferYang
Copy link
Contributor Author

Oh, you are right. Sorry for misleading you so far, @LuciferYang . :(
Yes, RoaringBitmap is used in HighlyCompressedMapStatus only.

doesn't matter ~ :)

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