RFR: 8253286: Use expand_exact() instead of expand_at() for fixed requests in G1

[Stefan Johansson]

On Fri, 18 Sep 2020 16:22:00 GMT, Thomas Schatzl <tschatzl at openjdk.org> wrote:

>>> we should never fail this expand
>> 
>> I think I am missing some background knowledge. I assume `expand_exact()`, in generally, could fail, but for args,
>> `curr, 1, NULL` here, it will not fail, because ... The "because" part should be in the comments right above the call.
>> As for the post-assert, `_regions.get_by_index(curr) != NULL && is_available(curr)`, I think they are already covered
>> in `expand_exact`.
>> I like `at(curr)->is_free()` better, since the current function, `find_highest_free`, should return a region (by index)
>> that's free.
>
> Looking at this again, I think the change in HeapRegionManager::find_highest_free() is not good.
> 
> While the call should never fail (it is done when allocating archive regions at VM startup when the heap is completely
> empty), the caller handles that - and does not allocate the archive regions at all. Consider the use case where you
> start with a small heap, but a huge CDS archive (maybe even larger than the heap). This should still continue working
> without CDS usage.

@tschatzl, not sure I follow the use case you describe and how using `expand_exact()` will differ from `expand_at()`.
Since we're searching backwards in `HeapRegionManager::find_highest_free()` we know `expand_at()` can only expand
precisely the requested region (all potentially higher regions must be in use already). If for some reason
`expand_at()` would have expanded another region in the old code, this would be a bug, because we return the requested
index.

-------------

PR: https://git.openjdk.java.net/jdk/pull/235