Replace arc-swap with hazarc#2472
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2472 +/- ##
==========================================
- Coverage 72.60% 72.58% -0.03%
==========================================
Files 390 390
Lines 63374 63364 -10
==========================================
- Hits 46015 45990 -25
- Misses 17359 17374 +15 ☔ View full report in Codecov by Sentry. |
ccba348 to
55db769
Compare
fuzzypixelz
left a comment
There was a problem hiding this comment.
I think two things are worth discussing:
- Is there evidence to support the claim of better performance for zenoh's use case(s)?
- How confident are we that
hazarc—being a relatively new crate—is battle-tested and proven? Admittedly, it already has one reverse dependency on crates.io. So I searched for the circumstances that led the project chose to usehazarchoping for a independent analysis, but alas all I found was a pull request titled "Performance fix" where the author replacesRwLockwithhazarc::AtomicArcwithout citing their reasoning.
If your answer to (1) is convincing enough (i.e. we stand to gain a lot), I'd be willing to take a risk and ignore (2) since I have good confidence in your work.
Ideally, I would need to do a deep-dive into hazarc myself to answer (2) but I am no expert on the topic and would need to spend significant time and effort to build up a good understanding of the design space.
With no peformance data, no independent analysis of hazarc's design and implementation and little time to commit to doing an analysis myself in the near future, I am not in a position to review nor merge this pull request.
Description
Replace arc-swap with hazarc
What does this PR do?
Replace arc-swap with hazarc
Why is this change needed?
hazarc has better performance characteristic, especially regarding loading of
Noneand is more robustly tested.Related Issues
None
🏷️ Label-Based Checklist
Based on the labels applied to this PR, please complete these additional requirements:
Labels:
dependencies,internal📦 Dependency Updates
Since this PR updates dependencies:
Best practice: Keep dependency updates focused and test thoroughly.
🏠 Internal Change
This PR is marked as internal (not user-facing):
Lighter review: Internal changes may have lighter review requirements.
Instructions:
- [ ]to- [x])This checklist updates automatically when labels change, but preserves your checked boxes.