RFR: 8361055: Serial: Inline SerialHeap::process_roots [v3]

[Kim Barrett]

On Tue, 15 Jul 2025 08:26:02 GMT, Albert Mingkun Yang <ayang at openjdk.org> wrote:

> > Interleaving comments in each place mentioning the missing calls, and an
> > explanatory comment in each place about the overall task would be helpful.
> 
> Added comments.

Sorry, but I'm not finding the new comments especially informative. They seem
like "what" rather than "why" comments. The code seems relatively obvious
about "what". It's the rationale that isn't obvious. Not only about what is
there in each place, but also about what is not.

With the old code, having it all in one place (*) with using-context
controlling the details, that "why" information was implicit but wasn't too
hard to puzzle out. With the proposed scattering that seems much harder (to
me, at least).

Though in the old code the names of the scanning options (and even the name of
the type) aren't all that great. Especially now that option flags are never
combined. (I think there used to be more flags and more detailed control? I
haven't dug through git history to remind myself of how we got here.) If
instead of the scan options we had something like an enum for "young
collection" and "full collection marking" and "full collection pointer
adjustment", I think that would be clearer than the existing code.

To be clear, I'm not dead set against the approach being proposed by this PR.
But such a change needs to be an improvement, and I don't think the current
proposal meets that requirement. Not even compared to the existing code, let
alone compared to tidying things up along the lines described in the previous
paragraph.

(*) The WeakProcessor::oops_do in fullgc pointer adjustment phase is perhaps
an outlier?  Maybe it should have been in process_roots, with a relevant
option? That would have retained the option combining, or perhaps triggered a
move away from that style. Though this also harkens back to a long ago
discussion / dispute about the meaning of "root".

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

PR Comment: https://git.openjdk.org/jdk/pull/26038#issuecomment-3076810608