RFR: 8332455: Improve G1/ParallelGC tasks to not override array lengths

[Roman Kennke]

On Fri, 5 Jul 2024 13:21:49 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

> Besides the problems discussed below, I also wondered about the rationale for this change. The JBS issue (incorrectly, see below) describes what G1 currently does, but doesn't say anything about why it's a problem. I see this issue has the lilliput label, so maybe it has something to do with that project? Without that information it's hard to discuss any alternatives.

Correct. And sorry about that. Indeed my motivation to explore alternatives come out of the Lilliput project. I am currently working on 'Lilliput2', that is 4-byte-object-headers (and I've already got a version that runs quite well). While the header  (aka mark-word) is only 4-bytes, many GCs still use the whole first word of the object to store the forwarding pointer. This is normally not a problem - the object is already copied and we don't care much about anything in the from-space copy. However, arrays now would have their length-field in the first word, too. Thus overriding the first word of the from-space copy with the forwarding pointer would destroy any information that the GC keeps there (or the other way around).

I knew that in Shenandoah we don't do that, and instead encode the slicing in the tasks (and I heard that ZGC does something similar, but I haven't looked at that, yet), so my goal was to explore if that is feasible for G1, too. At first I only did G1 young STW GC, but digging deeper I realized that G1 concurrent GC and Parallel young GC also use similar approaches (without sharing much). So I also explored if the encoding would work there, too. This is pretty much how I ended up with this PR.

About the address space concern, yes, I am aware of this. We're doing the exact same slicing in Shenandoah since forever, and so far never had a customer who would get to that limit. Our reasoning there was that if one uses this amount of Java heap, using 2x as much native memory for the GC task queues (using the fallback version of the task encoding) would probably don't matter all that much.

But I'll be totally happy to explore other approaches. In-fact, I think at least G1 concurrent mark should be fine, anyway.

> The bug report says G1 array slicing uses the from-space array's length field to track the array slices. That isn't true. It uses the to-space array's length field, since it uses PartialArrayTaskStepper.

Right. I got that wrong. However, it still requires the from-space length to remain intact, which is not the case with Lilliput2.

> ParallelGC uses the from-space length in PSPromotionManager::process_array_chunk, though there is an RFE to change ParallelGC to also use PartialArrayTaskStepper (JDK-8311163).

Ok, that is good. If we could come up with an approach that makes PartialArrayTaskStepper not depend on the from-space length, that would solve my problem.

> I also wish the proposed change was broken up a little bit. Changing all of G1 STW, G1 Concurrent, and Parallel at the same time is hard to review, and I don't think there's a need to do all in one change.

I could do that. But let's first see if we come up with an approach that makes us all happy ;-)

One idea that I just now had (for my particular problem in Lilliput2) is this: we only need the length-field in the from-space copy to keep the actual length of the array, so that we know when to stop, and also to restore the length in the to-space copy. We could copy the length to the from-space offset 8 (instead of 4, which is the length field with Lilliput2), and use that, instead. That should be ok, because surely we don't need to do the slicing for 0-length arrays (in which case we would stomp over the next object), and for larger arrays, we don't care about the from-space elements just like we don't care about the length.

> I have a couple of ideas for alternative mechanisms that I might explore.

Would you mind sharing the ideas?

Thanks,
Roman

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

PR Comment: https://git.openjdk.org/jdk/pull/19282#issuecomment-2211035072