Marlin renderer patches for jdk8u integration
Hohensee, Paul
hohensee at amazon.com
Thu Feb 13 00:40:21 UTC 2020
Rather than go through a pile of separate reviews, here’s a review for all the ones for which Andrew asked a review. I used the patches for the first 12 issues on the list at http://cr.openjdk.java.net/~lbourges/marlin-jdk8/marlin-jdk8.0/.
8143849: has been pushed to 8u.
8145055: approved by Andrew.
8144630: lgtm.
8144446: approved by Andrew.
8144445: approved by Andrew.
8144526: lgtm.
8144654: approved by Andrew.
8144718: approved by Andrew.
8149338: lgtm. The patch looks clean to me.
8148886: lgtm. The cleanerObj definition is in the diff context lines, not in the actual diffs, so the patch applies cleanly to RendererContext.java, net of context. 8147443 uses the Cleaner and associated classes that replaced finalizers. They haven’t been backported to 8u.
8144938: lgtm. I don’t see any differences in the diffs. :)
8159093: lgtm. I don’t see any context differences. RenderContext.java isn’t in the patch.
Paul
From: Laurent Bourgès <bourges.laurent at gmail.com>
Date: Wednesday, February 12, 2020 at 9:12 AM
To: Andrew Hughes <gnu.andrew at redhat.com>
Cc: "Hohensee, Paul" <hohensee at amazon.com>, jdk8u-dev <jdk8u-dev at openjdk.java.net>
Subject: Re: Marlin renderer patches for jdk8u integration
Hi Andrew,
On 10/02/2020 08:48, Laurent Bourgès wrote:
> Hi David & Paul,
>
> Could you help me in preparing & reviewing patches (20 remain to be
> integrated in 8u) to be faster ?
>
> Most of them apply cleanly so it consists in adding jdk8u-fix-request, wait
> for yes then push in 8u-dev.
>
> 2nd pending request:
> https://bugs.openjdk.java.net/browse/JDK-8145055
>
> Only few large patches will need a RFR for 8u.
>
> Cheers,
> Laurent
>
Give me a list of the bug IDs and I'll look into them.
I can push after approval. I'm already aware that's necessary.
Thanks,
--
Andrew :)
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/
----------------------------
2. m02.8145055.patch Review:
----------------------------
Output: OK
Status: Approved
Comments:
Same patch
----------------------------
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
----------------------------
4. m04.8144446.patch Review:
----------------------------
Output: OK
Status: Approved
Comments:
Same patch
----------------------------
5. m05.8144445.patch Review:
----------------------------
Output: OK
Status: Approved
Comments:
Same patch
----------------------------
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;
----------------------------
7. m07.8144654.patch Review:
----------------------------
Output: OK
Status: Approved
Comments:
Same patch
----------------------------
8. m08.8144718.patch Review:
----------------------------
Output: OK
Status: Approved
Comments:
Same patch
----------------------------
9. m09.8149338.patch Review:
Status: Approved
----------------------------
Identation issues in diff => same
----------------------------
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
----------------------------
11. m11.8144938.patch Review:
----------------------------
CrashNaNTest.java : diff looks different but raw/new files are the same + patched output files are the same.
OK
----------------------------
12. m12.8159093.patch Review:
----------------------------
OffHeapArray:
diff are different on before / after sections (no Cleaner + reference processing)
RendererContext.java:
fix chunks (lines) only
OK
---
Cheers,
Laurent
More information about the jdk8u-dev
mailing list