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

[Chore] Upgrade dashmap to v6 #1820

Merged
merged 5 commits into from
Aug 13, 2024
Merged

[Chore] Upgrade dashmap to v6 #1820

merged 5 commits into from
Aug 13, 2024

Conversation

Copy link

github-actions bot commented Aug 11, 2024

Test Results

102 files  ±0  102 suites  ±0   23m 23s ⏱️ -9s
 84 tests ±0   84 ✅ ±0  0 💤 ±0  0 ❌ ±0 
217 runs  ±0  217 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 3db3201. ± Comparison against base commit 5db1cba.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@tillrohrmann tillrohrmann left a comment

Choose a reason for hiding this comment

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

LGTM. +1 for merging.

- Design for lazy serialization and deserialization
- In-memory loglet now skips serialization entirely
- LogEntry api enables future key-filtering (upcoming)
- Fixed benchmarks
- Decouple local-loglet's storage layout from Bifrost
- Pave for in-memory record cache
- Simplified loglet's API, everything now goes through the append_batch API
- Support for passing payloads through an Arc since our payloads are pretty large in inefficient to pass through mpsc channels by value. Using Arc internally in bifrost allows near-zero cost clones on retries, caching of records, and zero-cost delivery to readers with no memory copies or deserialization.
- Removes requirement for metadata store's value to be `Clone` since StorageEncode is now object-safe

As a result of all recent changes, benchmarks show nearly _almost_ double the append throughput, although impact on production workloads will likely be small at this stage
- Upgrade derive_more
- Use derive_more as much as possible (it has a comprehensive list of powerful derive macros)
  - Some `strum::Display` macros still exist whenever we need parity to serde's kebab-case style.
  - Replaces `strum::EnumIs` with `derive_more::IsVariant`
  - Replaces `strum::FromRepr` with `derive_more::TryFrom`
- Clean ups: strum_macros -> strum (using `derive` feature on `strum` instead of a separate crate. Easier to grep, and one version to maintain)
- Remove unused deps in bifrost to speed up builds
- Removed a couple of Debug fields in bifrost where we were printing a value under mutex
dashmap v6.0.1 brings nice performance improvements anywhere in 5-40% range
@AhmedSoliman AhmedSoliman merged commit 3db3201 into main Aug 13, 2024
19 checks passed
@AhmedSoliman AhmedSoliman deleted the pr1820 branch August 13, 2024 16:03
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