-
-
Notifications
You must be signed in to change notification settings - Fork 706
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
Add SharedAllocatorList
#10594
Add SharedAllocatorList
#10594
Conversation
Thanks for your pull request, @0xEAB! Bugzilla referencesYour PR doesn't reference any Bugzilla issue. If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog. Testing this PR locallyIf you don't have a local development environment setup, you can use Digger to test this PR: dub run digger -- build "master + phobos#10594" |
dc04dbf
to
971b4a1
Compare
AllocatorList!(Factory, BookkeepingAllocator) impl; | ||
SpinLock lock = SpinLock(SpinLock.Contention.brief); | ||
|
||
// Remove this when 'https://github.com/dlang/druntime/pull/2156' gets merged |
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.
does this have a corresponding dmd PR? is it still open?
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.
I haven’t been able to find any successor to that druntime PR.
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.
Should we rather rephrase that note or simply remove it?
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.
Theoretically, there’s dlang/dmd#17128.
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.
So, I’ve replaced this comment in 45a094f.
971b4a1
to
4732be3
Compare
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.
Not that you need do it here, but if this allocator is advertised as thread safe then the unit tests should not be system, we should @trusted
the minimal interface and have the rest be safe.
I suppose this would need a purpose-built shim as “allocators must be entirely |
Co-authored-by: Elias Batek <[email protected]>
d701391
to
97f3432
Compare
That only lists |
Revamp of #6465; recreated by applying the changes to current ~master.
The unresolved reviews of the original PR haven’t been addressed yet.