bsalloc: improve cache #171
Conversation
|
Hi @teg, thanks for the PR! @ezrosent and I are pretty busy right now, so we probably won't be able to get to this for a bit, but we really appreciate the PR! In the meantime, if you're interested in contributing beyond this PR, feel free to check out the issues and let me know if you have any questions about anything. |
|
Thanks @joshlf! Do you have a preferred communication channel for general questions? For now I thought I'd just file some PRs for stuff I notice as I try to wrap my head around the code, but take your time reviewing it. |
The underlying MapAlloc allocator will round up the allocation (to the nearest page), but the cache was only servicing requests smaller than the allocation that was requested, rather than the allocation actually given. Use `usable_size()` to use the allocated size, rather than the requested size. In practice (at least on Linux), this changes small objects from 1k to 4k, and leaves the size of large objects unchanged.
rng() is a multiple of the alignment of `usize` (in particular of the memory address of an `usize`). When taking this modulo 4 (and 8, on 64 bit), the result is always 0. This means that at most two entries in the caches are utilized. Divide the output by the alignment of `usize`, which preserves the overall entropy, but moves it into the lower bits (which are the ones that are used).
Further to the previous patch, the address taken as `seed`, is a fixed offset from the stack frame alignment. On OSX this is fixed to 16 bytes, and on other systems 16 bytes is the default. Divide `seed` by 16, rather than simply the alignment of `usize`. As before, this does not reduce the overall entropy (assuming default stack-frame alignment), but moves more entropy into the lower bits.
|
Comments on issues/PRs are fine, and email works fine too (hello@joshlf.com) for longer-form or more open discussion. |
|
Hey, sorry for the delay. The code looks good! If you wouldn't mind squashing the last two commits (the ones about the rng) into one, we'll merge! |
|
I won't be working on this. |
I noticed some minor things as I'm looking through the code.