Repository navigation
Meeting: Nov 27, 2019 #70
Description
Action items:
- Give feedback on @TimDiekmann's allocator repository. (
@kabergstrom will championDone by @Wodann) - Review and discuss state of input handling.
- Investigate possibility for standardising more core concepts
- thread pools (@Lokathor)
- Host AreWeGameYet and transfer domains (@ozkriff)
- minimal frame allocator with bump allocation and a reset function.
Activity
Lokathor commentedon Nov 13, 2019
action item: minimal frame allocator with bump allocation and a reset function.
Lokathor commentedon Nov 13, 2019
possiblity: adding "crate assumptions" metadata in crates.io or on the ecosystem guide. Eg: "uses thread local storage", "uses threads", etc
Lokathor commentedon Nov 13, 2019
action item assignment: I will ask around about thread pool unification.
Lokathor commentedon Nov 15, 2019
Started an issue for the minimal bump alloc: #71
Lokathor commentedon Nov 15, 2019
"global thread pools": withoutboats posted this, which seems very related, https://twitter.com/withoutboats/status/1195030095800471552
Lokathor commentedon Nov 26, 2019
Meeting is tomorrow
Reminder
The meeting is scheduled for
18:00 UTCWhich, because of daylight savings changes, is now 1 hour different from the time we were meeting in the summer!
Updated times for some time zones:
--
and so on!
Lokathor commentedon Nov 26, 2019
(We can discuss if we should make the meeting times follow daylight savings and such, but currently the readme's stance is clear that we don't follow daylight savings)
Wodann commentedon Nov 27, 2019
I implemented a bump allocator using the alloc-wg's traits.
To achieve that I proposed some changes in this PR.
Lokathor commentedon Nov 27, 2019
aclysma wanted #44 to get some talk, it's been sitting a while
will probably have to get updated some before we can merge it.
Lokathor commentedon Nov 27, 2019
It was just me and wodann this time.
Notes:
OR Zlibto the license sections of all of your crates!95th commentedon Nov 28, 2019
@Wodann You may be interested in my fork of bumpalo using alloc-wg crate. I didn't need alloc-wg's traits to take immutable self.
Wodann commentedon Nov 28, 2019
@95th Nice! I had another look to see why we might have a discrepancy, but my version of the bump allocator is a generic that accepts any allocator that implements the
BuildAllocRef,DeallocRefandAllocRef.Bumpalojust uses a staticallocfunction.When using a generic, the
AllocRef::alloc(&mut self, ...)function would forceBumpalo'sBump::alloc(...)to be a mutable reference as well. I think that is where it breaks down.95th commentedon Nov 29, 2019
That static fn remained from original implementation. I have implemented alloc wg traits here so you can pass immutable reference
&Bumpin alloc generic collections.For example: https://github.com/95th/bumpalo/blob/15aa51d51f730e7a271cc16917671dd6fdcb445c/tests/string.rs#L8
TimDiekmann commentedon Nov 29, 2019
Is it possible for someone to summarize the meeting in terms of the alloc-wg crate?
If I understood that correctly,
&mut selfseems to have posed a problem, but as @95th pointed out, it still seems possible to implement a bump allocator via a mutable reference (note, that the currentAlloctrait also takes a mutable reference).But I would also be interested to know to what extent the implemented proposals were accepted:
BuildAllocReffeel like? This is by far the biggest change in comparison to theAlloctrait.NonZeroLayout?cc @glandium @Ericson2314 @gnzlbg
SimonSapin commentedon Nov 29, 2019
I’m skeptical the an associated error type is all that useful as a way to return additional diagnostic information. At least I haven’t seen concrete use cases, only what feels like attempts at being maximally generic out of principle. But this isn’t a strong opinion.
I am opposed to the idea that every API that could ever involve allocation should always return a
Result, and that infallible APIs would "naturally" fall out of setting the associated error type to!. This is incompatible with everything that already exists (for exampleVec::pushis stable and doesn’t return aResult), and would be extremely ergonomic.Result<Foo, !>is not the same asFooto callers, even if we add anunwrap-like method guaranteed not to panic.Even without going that far, planning to set an allocator’s associated error type to
!seems to me that it’s handling errors at the wrong layer.Finally,
#[global_allocator]under the hood passes pointers toexternfunctions with a fixed API, so anything type-level doesn’t work there.Having said that, what do you think are advantages or uses of an associated error type?
Lokathor commentedon Nov 29, 2019
(I thought I had recorded the meeting, but I'm a dummy and the audio settings were messed up, and so there was no audio.)
So we got this methods here https://github.com/Wodann/alloc-bump-rs/blob/master/src/lib.rs#L64 which moves a T into the allocator's memory region and return the
&mut Tto it. If you do this, and you havealloc_taccept&mut self, there's no way to allocate two things, because the second thing can't get a&mut selfto allocate since the lifetime of the first&mut Tis still live.&'static mut Tbut that seems error prone.&self, which unblocks this.Then when you have everything taking
&self, there's another problem: when you reset the allocator you can potentially double-assign some memory and then there's UB because you have two&mut Tto the same memory span.reset(and onlyreset) take&mut selfand then you call it once at the top of the frame when you statically know there couldn't have been any lasting out allocations from the allocator yet anyway. Unless you do something silly like assign a bump alloc reference to a location that lasts outside of the frame loop, in which case Rust will take notice and (correctly!) block you from callingreset.I didn't get a hang of the point of
BuildAllocRefduring the meeting. Is it just a weird way to have a reference that isn't a literal reference?In terms of associated errors, if the allocator wants to have errors be useful, just make an extension trait for it that adds a "get last error" method or something. Like an allocator should be returning
*mut u8or*mut c_void, perhapsOption<NonNull<u8>>, orOption<NonNull<c_void>>if you wanna get fancy. But I haven't followed the allog-wg discussions at all so maybe there's some super good reason to not keep it simple, but keeping it simple is what I'd favor, if possible.Wodann commentedon Dec 1, 2019
@95th I understand that your changes allow the bumpalo Bump allocator to use the alloc-wg's
String,Box, etc. types. I think you might be missing my point, though.At the moment, the bumpalo
Bumpallocator uses the staticallocfunction to allocate its chunks. All of theimpl Bumpfunctions still use thatallocfunction. In my implementation, I use theAllocReftrait to allow users to specify their own allocator to use for chunk allocation. Once you take that approach, you cannot haveBump::allocorBump::alloc_withaccept a shared reference, as theAllocRef::allocfunction that it tries to call requires a mutable reference.You can try changing your
Bumpstruct into a generic:struct Bump<A: DeallocRef = Global>that stores theA::BuildAllocfor usage instead of the staticallocfunction. I think you'll run into the same issue as me then.Wodann commentedon Dec 1, 2019
@TimDiekmann I hope this answers your questions.
I implemented the
BuildAllocReftrait for a shared reference, so I was merely returningself. I am not sure if it has any benefits and in what case it would have benefits. E.g. now I am storing a generic that implements theBuildAllocReftrait inside my structure, but I don't see any advantage over just storing anAllocRef.I also had to look at the
Globalallocator implementation to figure out what it was there for and how to interconnect the different traits. As such, I'd say that it wasn't intuitive.I didn't actually return any error information, but one might. I am impartial to having an
OptionorResulttype. Do you have an idea what the performance overhead might be?No complaints. I appreciate the extra guarantee.
Ericson2314 commentedon Dec 1, 2019
I think you can do something like this:
Now you support concurrent access if and only if the underlying chunk allocator supports it. Perfect!
Lokathor commentedon Dec 1, 2019
@TimDiekmann actually now that i think about it, I'd personally like if NonZeroLayout had exactly one NonZero value, because it makes the
Option<NonZeroLayout>clearly have 0 for None and a real value for non-zero. With two internal NonZero values it doesn't work out like that and ends up not playing nice with zeroed memory.TimDiekmann commentedon Dec 7, 2019
I haven't done any performance checks, but as long as the underlying error type is a ZST, this should have no impact at all.
Make `NonZeroLayout` have exactly one non-zero value
Lokathor commentedon Dec 11, 2019
Closing this up so we don't have too many old meetings issues.
If you have more to say please feel free to open a new issue for it (if necessary) or just move discussion to a newer meeting issue.