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

[Albert Mingkun Yang]

On Fri, 18 Sep 2020 19:53:15 GMT, Stefan Johansson <sjohanss at openjdk.org> wrote:

>> 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.

> it depends on what you mean by fail

Sth like `expand_exact(curr, (uint)-1, NULL)`. (Ofc, this is a contrived example.) After a closer look at the
surrounding loop, I see it's actually iterating `curr` from `max_length()-1` to 0 with some early returns. (Seems to me
a for-loop makes more sense.) Since `curr` always contains a value in this valid range, making it available (expanding
one region) should always succeed.

> 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.

Maybe this is obvious to veterans, but making it explicit is more friendly for newcomers, IMO.

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

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