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.

@albertnetymk, it depends on what you mean by fail, but not really, after a call to `expand_exact()` all regions in the
given range are "available". There are no guarantee that all are free, but the HeapRegion-object has been created and
the memory has been committed. In this case, since the regions was not available right before the call (and we only
expand a single region), we know that it must now be both available and free. The new assert better shows the intent
and maybe a comment like this will make it even clearer? Suggestion:

    // Found uncommitted and free region, expand to make it available for use.
    expand_exact(curr, 1, NULL);

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

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