RFR: 8332527: ZGC: generalize object cloning logic [v2]

[Roberto Castañeda Lozano]

On Tue, 21 May 2024 09:17:16 GMT, Roberto Castañeda Lozano <rcastanedalo at openjdk.org> wrote:

>> This changeset generalize the logic to produce a runtime call to clone a class instance so that it can be shared by other collectors adopting the late barrier expansion model (including G1 in the near future, see [JEP 475](https://openjdk.org/jeps/475)). The changeset moves the logic from `ZBarrierSetC2` to the GC-shared `BarrierSetC2` class and adds support for 32-bits platforms.
>> 
>> #### Testing
>> 
>> - tier1-3 (windows-x64, linux-x64, linux-aarch64, macosx-x64, macosx-aarch64; release and debug mode).
>> - tier4-7 (linux-x64, linux-aarch64; release and debug mode; ZGC tests only).
>> - `compiler/arraycopy` tests (linux-x86-debug) with [an additional patch](https://github.com/openjdk/jdk/commit/ddcf777894e740b8e6ddbbf8821e82a173c23ef4) that implements cloning of large class instances with a runtime clone call rather than arraycopy when using G1 (to exercise the generalized logic on a 32-bits platform).
>
> Roberto Castañeda Lozano has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Applied Axel's suggestions

Thanks Tobias and Axel, as far as I understand the special handling of the case `ac->is_clone_array() && ary_ptr != nullptr` was added by [JDK-8270098](https://bugs.openjdk.java.net/browse/JDK-8270098) (in JDK 18 b10) to deal with reflective `clone()` invocations as in this example:


    public static Object testCloneObject(Method clone, Object obj) throws Exception {
        return clone.invoke(obj);
    }

    String[] array = new String[N];
    (...)
    Method clone = Object.class.getDeclaredMethod("clone");
    clone.setAccessible(true);
    ... = testCloneObject(clone, array);



The code above led, in JDK 18 b10, to an ArrayCopy with `CloneArray` kind but source node of type `java/lang/Object:NotNull *`, which motivated the introduction of special handling for this case. Short after (JDK 18 b22), [JEP 416](https://bugs.openjdk.org/browse/JDK-8271820) was integrated, changing significantly the implementation of this reflective invocation. After JEP 416, the above example and the tests introduced by JDK-8270098 do not require special handling anymore, because the reflective invocation results in a native call to [jdk.internal.reflect.DirectMethodHandleAccessor$NativeAccessor::invoke0](https://github.com/openjdk/jdk/blob/9ca90ccd6bfec76e54e2e870bd706fad5abf233c/src/java.base/share/classes/jdk/internal/reflect/DirectMethodHandleAccessor.java#L268) and is not C2-compiled anymore. I have also run our CI tiers 1-8 asserting that `ary_ptr != nullptr` if `ac->is_clone_array()` holds, without encountering any failure. So there might be an opportunity to simplify the `
 clone_at_expansion` logic by reverting at least part of the special handling introduced by JDK-8270098. However, I think this is out of scope for this RFE, and should be addressed separately.

I propose to remove in this RFE the assumption that `BarrierSetC2::clone_instance_in_runtime` can only be called for instance cloning and limit the RFE to simply moving logic from ZBarrierSetC2 into BarrierSetC2 and adding support for 32-bits platforms, without any other changes to the existing logic. After integrating this RFE, I will then create a RFE for investigating the feasibility of the JDK-8270098 clean-up. @TobiHartmann @xmas92 do you agree?

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

PR Comment: https://git.openjdk.org/jdk/pull/19311#issuecomment-2125041718