-
Notifications
You must be signed in to change notification settings - Fork 2
feat(core): add new dart allocation interfaces #311
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
base: master
Are you sure you want to change the base?
Conversation
This should heavily simplify dart pre-allocation for parallel task, though the system still needs improvements.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #311 +/- ##
==========================================
+ Coverage 84.81% 85.40% +0.59%
==========================================
Files 68 69 +1
Lines 9942 10219 +277
==========================================
+ Hits 8432 8728 +296
+ Misses 1510 1491 -19 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a significant change; can we discuss it more this week?
To clarify what this PR is aiming at in the long run,
@@ -22,6 +22,11 @@ impl UnusedDarts { | |||
self.0.extend((0..len).map(|_| TVar::new(false))); | |||
} | |||
|
|||
/// Extend internal storage capacity using the passed value for new items | |||
pub fn extend_from_val(&mut self, len: usize, val: bool) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pub fn extend_from_val(&mut self, len: usize, val: bool) { | |
pub fn extend_with(&mut self, len: usize, val: bool) { |
Not sure about the semantic you want to follow, but this looks closer to what people are doing externally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, I have no strong opinion on the naming, doing what others do is probably best
I'll need to properly update everything in a single commit if I want to rebase this tho
sure, I also started to look into CRDTs so maybe this can tie things in. |
Description
should be rebased
Scope: core
Type of change: feat
Content description: see commits
Necessary follow-up
There are a few different things possible for improvements before integrating some form of CRDT:
CMap
s and only allow aligned block allocations