Skip to content

Conversation

bbrehm
Copy link
Contributor

@bbrehm bbrehm commented Jan 27, 2025

This replaces scala.collection.mutable.LinkedHashMap[String, Int] with a custom dedup table for construction of the string-pool. That brings down serialization on my large test graph from ~2200ms to ~1700ms, i.e. does a hefty chunk.

Main reason why this does so much is that all the string deduplication must be done serially, in order to allow us to get bitwise-reproducible results; and now that everything else is parallel, this means that this becomes a bottleneck if there are enough cores.

I'm pretty unsure whether the speedup is worth the maintenance hassle. If we don't want to merge, then it is fine to leave this PR open indefinitely, in case we want to resurrect it in the future.

@bbrehm bbrehm requested review from ml86 and mpollmeier January 27, 2025 13:13
Copy link
Contributor

@mpollmeier mpollmeier left a comment

Choose a reason for hiding this comment

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

I'm in favor of bringing this in, but first please:

  1. add a unit test

  2. copy the good explanation from the PR description into the code, so it's accessible in the future, and in case we decide it becomes a maintance burden (which I doubt) we can safely revert it / kick it out

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.

2 participants