RFR: 8343658: Parallel: Implement block_start for Young generation
Volker Simonis
simonis at openjdk.org
Tue Jan 7 16:07:42 UTC 2025
On Tue, 7 Jan 2025 08:51:21 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:
>> Simple block_start implementation for Parallel young-gen. Related to https://github.com/openjdk/jdk/pull/21870
>>
>> Test: tier1-3
>
> @simonis My intention was simply to align Parallel with the existing implementations in Serial and G1, thereby removing the oddly structured `DebuggingContext` code.
>
> While it’s possible to invest additional effort in object iteration to make failure-reporting more robust, it’s worth noting that the default GCs (Serial and G1) haven’t, to my knowledge, required such measures. Therefore, I wonder if we might be overthinking this without a specific, concrete need.
>
> If you still think this is a step in the wrong direction, I will close the corresponding ticket.
@albertnetymk, I will not veto if you want to proceed but as I've already wrote, the current implementation for Serial is definitely wrong and can lead to secondary crashes during error reporting (which I regularly see in hs_err files) and even wrong, to infinite loops in the worst case. I haven't analyzed the G1 code yet because it is more complex, but I'm pretty sure it is also wrong if called at arbitrary places during VMError reporting (e.g. it depends on `G1HeapRegion::_parsable_bottom` being set correctly which isn't necessarily the case if the VM crashes).
If you proceed with this PR, I suggest to at least use [`LocationPrinter::is_valid_obj()`](https://github.com/openjdk/jdk/blob/ac82a8f89c7066fb1d379b12bcfd68053cb39ba4/src/hotspot/share/gc/shared/locationPrinter.cpp#L33) which internally uses [`Klass::is_valid()`](https://github.com/openjdk/jdk/blob/ac82a8f89c7066fb1d379b12bcfd68053cb39ba4/src/hotspot/share/oops/klass.cpp#L1038) to check the validity of an oop in `MutableSpace::block_start()` instead of just using `cast_to_oop()` and asserting `oopDesc::is_oop()`. This will give us at least some safety belts. Also, if you proceed with this PR, please update the Serial implementation to use `LocationPrinter::is_valid_obj()` as well.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/21919#issuecomment-2575665010
More information about the hotspot-gc-dev
mailing list