RFR: 8276721: G1: Refine G1EvacFailureObjectsSet::iterate [v2]

Thomas Schatzl tschatzl at openjdk.java.net
Tue Nov 9 09:16:44 UTC 2021

On Tue, 9 Nov 2021 06:50:57 GMT, Hamlin Li <mli at openjdk.org> wrote:

>> Currently, G1EvacFailureObjectsSet::iterate(ObjectClosure* closure) is not a const method, but it looks like a const method. It will be a surprise for reader, i.e. you would not expect that an iteration method is destructive, clearing out the set it just iterated over.
>> We should refine it in some way.
> Hamlin Li has updated the pull request incrementally with one additional commit since the last revision:
>   Fix typos

I do not think this change solves what it was intended to do: `G1EvacFailureObjectsSet:handle_iteration()` is still destructive, and the rest of the changes seem to just be moving around code, and making more stuff visible in the header file, overall complicating the code imho.

What about renaming `G1EvacFailureObjectsSet:iterate` to `G1EvacFailureObjectsSet:iterate_and_drop/empty/remove`? Or, as mentioned earlier, have an explicit `drop/remove/emtpy` method in `G1EvacFailureObjectsSet` and call it appropriately?
The former would be my preferred solution because it is then called distinctively enough from a normal `iterate` method, and does not completely mess up current code.

The cause of all this may have been the decision to put `G1EvacFailureObjectsSet`, a data structure only interesting during (young) gc that needs some per-gc handling, into the permanently valid `HeapRegion` data structure - I'll handle this refactoring though as mentioned. 

Let's sync with @albertnetymk about this before continuing.


Changes requested by tschatzl (Reviewer).

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

More information about the hotspot-gc-dev mailing list