Keep track of per-cycle mutator/collector allocs. Fix mutator/collector alloc region overlap in traversal.

[Aleksey Shipilev]

On 03/16/2018 10:58 AM, Roman Kennke wrote:
> Am 13.03.2018 um 17:24 schrieb Aleksey Shipilev:
>> On 03/09/2018 03:29 PM, Roman Kennke wrote:
>>> Differential:
>>> http://cr.openjdk.java.net/~rkennke/traversal-alloc-region/webrev.01.diff/
>>> Full patch:
>>> http://cr.openjdk.java.net/~rkennke/traversal-alloc-region/webrev.01/
>>
>> I think this is a simple solution, but its simplicity is deceptive.
>>
>> See, this solution:
>>  1. Adds pause time, because we walk regions during the pause to reset counters now;
>>  2. Turns the statistics counters into functionality-critical bits of data, and e.g. their racy
>> updates would cause functional bugs;
>>  3. Makes weird tests on allocation fast-path;
>>  4. Undermines current free set performance mechanics. Current code relies on density of bitmaps
>> performance-wise: if we cannot allocate in the region, we move the bitmap boundaries right away.
>> Current patch requires scanning the freeset for fitting regions when GC and mutator allocs meet each
>> other, because there can now be full regions of different type within the boundaries.
>>
>> The better solution, in my mind, would be biasing the freeset regions to the particular type of
>> allocation. That is, freeset can maintain two bitmaps and two sets of boundaries instead of one, and
>> use those for mutator/GC allocations. Once one bitmapped-type is depleted, the allocation path may
>> decide to "borrow" from another-type bitmap, if the borrowed region is empty, or even borrow
>> non-empty region if runtime option allows that (and that option would be "false" for traversal). It
>> solves all four problems with the patch above.
> 
> This seems overly rigid to me.
> 
> It seems easier to do what I had in mind originally, way back when I
> first implemented the separate handling of collector vs. mutator allocs
> in traversal: set a flag in ShHeapRegion to indiciate whether a region
> has been used for mutator or collector alloc, or no alloc at all, and
> then test that in the allocation search-loop.
> 
> Like that:
> http://cr.openjdk.java.net/~rkennke/traversal-alloc-region/webrev.02/
> 
> WDYT?

Well, I believe it still does not solve (4) above. And arguably, does not solve (3), because
reaching for the region is something we want to avoid, unless we are sure we are ready to allocate
in the region. This is the whole reason to maintain the bitmap view of "free" regions that tells us
sure-thing targets.

At times, it is important to step back, and look if we a bit of redesign gives you the properties
you want, without hacking the special cases into the hot paths. For example, split bitmaps naturally
separate mutator and GC allocs, and along with fixing the issues above, they give us 90% of the work
for evac-reserve.

We can push your fix, but we need to agree there are more comprehensive ways to fix this.

-Aleksey