RFR: JDK-8260200 G1: Remove unnecessary update in FreeRegionList::remove_starting_at [v2]

[Stefan Johansson]

On Fri, 22 Jan 2021 11:39:33 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> src/hotspot/share/gc/g1/heapRegionSet.cpp line 252:
>> 
>>> 250:   } else {
>>> 251:     assert_free_region_list(_tail != first, "invariant");
>>> 252:   }
>> 
>> Since these checks no longer does anything other than assertions I think it would be nice to hide it in a helper that for production builds will do nothing using `NOT_DEBUG_RETURN`.
>
> Thanks for reviewing.
> I just realized I made the change in another way from what you suggested exactly, seems it has same effect. Not sue if my current change with "if ASSERT" is OK, please kindly let me know if you don't think so.

You are correct that when executing it will have the same effect, but I'm more concerned about the readability of the code in this case. So please move it to a separate function, something like `verify_region_to_remove()`. Looking a bit closer at the code I think this function should be able to be called from each round of the loop as well, I mean that's what was done before and by doing so we could get the loop a bit cleaner as well. Or have I missed anything? In that case you would not need to call it before the loop, but just inside it.

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

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