Alloc: Pool MemRegion, split foreign queues - #26065
Conversation
| # Steal the entire list from `sharedFreeList`: | ||
| tc.freeList = atomicExchangeN(addr a.sharedFreeLists[s], nil, ATOMIC_RELAXED) | ||
| when usesRegionHandles: | ||
| let sharedHead = addr tc.owner.sharedFreeLists[s] |
There was a problem hiding this comment.
Can we do something about this indirection? MemRegion still has the sharedFreeLists field too, we could make good usage of it here, copy it into the MemRegion at creation when you steal it from a dead thread. Or something like that I'm not entirely sure how your stuff works.
There was a problem hiding this comment.
In a private attempt before this one I tried to copy the queues around, but this will run into the halting problem or invalid memory access when you try to make sure you catch the final write.
The flow here is roughly like this with threads A and B:
- A allocates, shares with B
- A terminates, B simulataneously deallocates
- B will have already loaded A's queue address, A swaps the queue pointers
- A will now miss the free cell and leak memory, or B will cause platform-dependent behavior like a crash
- Alternatively: A waits until B finishes, but if B gets suspended during this, A will wait forever thus reaching the halting problem (and if we continue, B will still have the wrong address loaded)
The leak case was triggered with a stress test, so it definitely can happen.
It's annoying, because that private attempt was the fastest, basically zero overhead.
There was a problem hiding this comment.
To explain a bit more about the setup and make sure we are thinking of the same goal, I still had the handle, but the handle contained pointers to the MemRegion's lists instead of carrying the lists in the handle. That made the local cost entirely free because there's no indirection anywhere, while the foreign case has to go through a pointer anyways.
At teardown, the handle's pointers would be heap allocated to safely migrate the local allocator state, but the copy of local -> heap (for migration) is what causes the described problem.
The handle's pointers would get swapped, but a foreign thread already loaded the old queue pointers that were still local to the dying thread.
A solution where you keep track of the "dying" state would run into the exact same scenario, the halting problem ruins it. So the cost of indirection, at least to me, is preferable because it will always make progress.
|
Apart from my remark, this one looks great. :-) |
replace #26020
replace #26035
As before, all clean (no leaks, no issues reported by ASan). We split
MemRegioninto two separate pieces, one that is primarily used as a stable access path (RegionHandle) and the local internal state (MemRegion).The difference this time, is that the only regression I could measure can be found at reclamation of foreign deallocations dealing with sizes around
(SmallChunkSize-smallChunkOverhead() - alignOff.int32) / 2 + 1untilSmallChunkSize-smallChunkOverhead() - alignOff.int32, so chunks that can only fit a single cell (which wastes memory, but that's a separate problem and existed before.)The losses here range from 1%-8% on my machine, but they are impossible to pin down. The tests for this are extremely noise sensitive and try to stress this specific case. Additionally, every tiny layout change can and will mess with the cache and cause regressions too, which is why, currently, the
sharedFreeListsfield inMemRegionis preserved even though it wastes space in threaded builds with destructors.Both the gains and losses, on the path that uses foreign cells, come from the split, big allocations cost a bit less now because it's back to a single atomic swap, small allocations cost more because they need to use the non-local queue which adds indirection.
Additionally, we waste a bit of time and memory at thread creation because the region+handle is initialized at thread creation. Lazy initialization caused more widespread performance regressions. #26043 (virtual threads) would eat that cost and should still be up for consideration.
close #23361
close #20542
close #24988
close #26014