[PATCH] D38245: [Sanitizers] Allocator: new "release memory to OS" implementation

[Aleksey Shlyapnikov via Phabricator via llvm-commits]

alekseyshl added a comment.

In https://reviews.llvm.org/D38245#880666, @cryptoad wrote:

> I do not have comments on the code, it looks good to me.
>
> There is a few points I was thinking about, not in the sense of suggestions for this CL, but rather general wondering:
>
> - How about about bumping last_release_at_ns when a region is grown? This would prevent the reclaiming to occur shortly after, and could lower the chances of releasing pages that were just allocated.
> - Could there be enough room in the free array to not have to map memory? eg: the memory between num_freed_chunks*sizeof(CompactPtrT) and mapped_free_array could be enough. I am not sure it would necessarily be a gain except maybe in memory usage. But since the memory is unmapped right after it might be worthless.
> - Did the tests show in any way what could be a good balance timing/cpu wise to set a default to?




1. Bumping last_release_at_ns makes sense, let me think about it a bit more
2. I'd rather not complicate the code just yet, but I like the idea of using whatever is already mapped. Will add a comment about it.
3. This version is pretty much linear to the number of pages allocated, everything else does not matter that much, so the more memory allocated, the slower it is. Comparing to the previous version, even the larger apps I tried spent insignificant amount of time releasing pages. I'd like to collect more diverse data before drawing any conclusions (fuzzer would make a great test app).


https://reviews.llvm.org/D38245