RFR: 8352969: G1: Improve testability of optional collections [v10]

[Albert Mingkun Yang]

On Thu, 11 Sep 2025 12:30:29 GMT, Guanqiang Han <ghan at openjdk.org> wrote:

>> src/hotspot/share/gc/g1/g1CollectionSet.cpp line 417:
>> 
>>> 415:   G1CSetCandidateGroupList* from_marking_groups = &candidates()->from_marking_groups();
>>> 416: 
>>> 417: #ifdef ASSERT
>> 
>> I'd suggest removing `#ifdef ASSERT`; the compiler should be able to remove unreachable code in an optimized build.
>
> Hi @albertnetymk , thanks for the suggestion! While the compiler may indeed optimize away unreachable branches, 
> keeping the `#ifdef ASSERT` here makes it clear that this block is intended for development/debugging 
> and also helps with readability and maintainability for future reviewers. 
> I’m happy to adjust if there’s a preferred convention you’d like me to follow.

If the code guarded by `#ifdef` is well isolated from the actual code, I'd agree separating them improves readability. In this case, the debug code is kind of mixed with the real code (especially the one inside the for-loop), having `#ifdef` breaks the contiguity of the flow, IMO.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/26880#discussion_r2340730607