[PATCH] D83993: [scudo][standalone] Change the release loop for efficiency purposes

[Kostya Kortchinsky via Phabricator via llvm-commits]

cryptoad added inline comments.


================
Comment at: compiler-rt/lib/scudo/standalone/release.h:270
+            PInRegion += BlockSize;
+            while (PInRegion < RoundedRegionSize) {
+              Counters.incRange(RegionIndex, PInRegion >> PageSizeLog,
----------------
cryptoad wrote:
> pcc wrote:
> > I guess this is equivalent to the code previously on lines 252-253. But I'm confused about why we would have more than one of these. In my mind these blocks would consume as much of the region as possible, which means that at the end we would just have a fraction of a block at most. Is that not the case?
> I went for something generic, and I think you are right in the sense that the loop will only iterate once. I will verify it and update it if it's the case.
I ran some tests, and I was wrong. There cases where there is more than 1 block.
This is because we use AllocatedUser as the region size, which might not be a multiple of a page (it's bound by a maximum # of batches we want to allocate at a time), so there are several circumstances where there would be more than one block in the last page. Here are some examples that got logged on Android:
block size, last block, region size, rounded region size
320, 173e0, 17700, 18000
1c0, 5e800, 5e9c0, 5f000
120, baf60, bb080, bc000

So the loop ends up necessary.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83993/new/

https://reviews.llvm.org/D83993