Allocator: Handle basic dead threads with unshared memory #23878
Conversation
SirOlaf
changed the title
|
I am hoping to come back to this with a more universal solution that allows allocators to adopt pages, but for most scenarios this one is perfectly usable. Will review what I did here again first though |
|
Gonna need a deeper look after all, there’s some code I really want to make work |
|
Can you undraft this? Every little progress on this front is greatly appreciated. |
|
Sure, just really threw me off when I discovered that my use case still doesn't work (as it also messes with it by reallocating a sequence or string, I guess that shouldn't work but still got me) |
| @@ -908,7 +912,7 @@ proc rawAlloc(a: var MemRegion, requestedSize: int): pointer = | |||
| if c.free >= size: | |||
| # Because removals from `a.freeSmallChunks[s]` only happen in the other alloc branch and during dealloc, | |||
| # we must not add it to the list if it cannot be used the next time a pointer of `size` bytes is needed. | |||
| listAdd(a.freeSmallChunks[s], c) | |||
| a.freeSmallChunks[s] = c | |||
There was a problem hiding this comment.
This makes me nervous. What is the justification for it?
There was a problem hiding this comment.
There is always only a single active chunk which receives cells when they get freed, so there is no reason for chunks to keep acting as a list in the way they did before.
There was a problem hiding this comment.
I was thinking of using the list behavior for the remaining threading issues to easily find chunks that must stay alive, but not sure if I can figure out how to do that yet.
Either way that was the original reason for making the change after realizing it's technically unused right now
Ref: #23361
Fixes the first example in referenced issue. The second example requires that pages can be inherited as creating a string allocates which then causes a cell to be moved into a foreign thread.
nextandprevfields ofSmallChunkare kept around for now as I suspect they will help for the more complicated case.