Marlin renderer patches for jdk8u integration

[Andrew John Hughes]

On 12/02/2020 17:09, Laurent Bourgès wrote:

snip...

> 
> Here is the beginning of my review, giving bug ids, my comment for
> patches m02 (fix-8u-request submitted) to m12.
> All these bug patches are mostly the same, only few difference in chunks
> or imports (trivial).
> 
> I can start a new RFR (webrev) thread for every modified patch (as
> explained in the workflow).
> 
> I uploaded all patch files (unshuffled for 8u and proposed):
> http://cr.openjdk.java.net/~lbourges/marlin8u/marlin-8.03_to_12/
> 

I'm a little confused. When you write "Approved" below, do you mean that
the bug has jdk8u-fix-yes and is ready for push? I don't see them in the
approved-and-waiting queue, your previous e-mail suggested they needed
approval and some still need review.


> ----------------------------
> 
> 2. m02.8145055.patch Review:
> 
> ----------------------------
> 
> Output: OK
> 
> Status: Approved
> 
> Comments:
> 
> Same patch
> 

Fine to skip review and go straight to jdk8u-fix-request.

> ----------------------------
> 
> 3. m03.8144630.patch Review:
> 
> ----------------------------
> 
> Output: OK
> 
> Status: Approved
> 
> 
> Comments:
> 
> RendererStats.java:
> 
> import sun.awt.util.ThreadGroupUtils; => import sun.misc.ThreadGroupUtils;
> 
> 
> Identation issues in diff => same
> 

Needs a quick RFR.

> 
> ----------------------------
> 
> 4. m04.8144446.patch Review:
> 
> ----------------------------
> 
> Output: OK
> 
> Status: Approved
> 
> 
> Comments:
> 
> Same patch
> 


Fine to skip review and go straight to jdk8u-fix-request.

> 
> ----------------------------
> 
> 5. m05.8144445.patch Review:
> 
> ----------------------------
> 
> Output: OK
> 
> Status: Approved
> 
> 
> Comments:
> 
> Same patch
> 


Fine to skip review and go straight to jdk8u-fix-request.

> 
> ----------------------------
> 
> 6. m06.8144526.patch Review:
> 
> ----------------------------
> 
> Status: Approved
> 
> 
> Comments:
> 
> MarlinUtils.java:
> 
> import jdk.internal.misc.JavaLangAccess; => import sun.misc.JavaLangAccess;
> 
> import jdk.internal.misc.SharedSecrets; => import sun.misc.SharedSecrets;
> 

Will need a quick RFR.

> 
> ----------------------------
> 
> 7. m07.8144654.patch Review:
> 
> ----------------------------
> 
> Output: OK
> 
> Status: Approved
> 
> 
> Comments:
> 
> Same patch
> 


Fine to skip review and go straight to jdk8u-fix-request.

> 
> ----------------------------
> 
> 8. m08.8144718.patch Review:
> 
> ----------------------------
> 
> Output: OK
> 
> Status: Approved
> 
> 
> Comments:
> 
> Same patch
> 


Fine to skip review and go straight to jdk8u-fix-request.

> 
> ----------------------------
> 
> 9. m09.8149338.patch Review:
> 
> Status: Approved
> ----------------------------
> 
> Identation issues in diff => same
> 

Did you have to alter the patch from a later OpenJDK version to get the
one for 8u? If so, it needs review.

> 
> ----------------------------
> 
> 10. m10.8148886.patch Review:
> 
> ----------------------------
> 
> Fix 2 conflicts with (skipped patch 8147443: Use the Common Cleaner in
> Marlin OffHeapArray)
> 
> 
> RendererContext.java:
> 
> Remove lines{
> 
> // Smallest object used as Cleaner's parent reference
> 
> final Object cleanerObj = new Object();
> 
> }
> 
> 
> Version.java:
> 
> marlin-0.7.3-Unsafe-OpenJDK => marlin-0.7.2
> 

Will need review. Please mention why 8147443 is being skipped.

> 
> ----------------------------
> 
> 11. m11.8144938.patch Review:
> 
> ----------------------------
> 
> CrashNaNTest.java : diff looks different but raw/new files are the same
> + patched output files are the same.
> 
> OK
> 

Sounds like it needs review to confirm.

> 
> ----------------------------
> 
> 12. m12.8159093.patch Review:
> 
> ----------------------------
> 
> OffHeapArray:
> 
> diff are different on before / after sections (no Cleaner + reference
> processing)
> 
> 
> RendererContext.java:
> 
> fix chunks (lines) only
> 
> OK
> 

Will need review.

> ---
> 
> Cheers,
> Laurent

Assuming these need to be committed in this order, we can approve & push
8145055 (I'll look into that now) and you should post the RFR for 8144630.

Thanks,
-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
https://keybase.io/gnu_andrew