Re: [OpenJDK 2D-Dev] RFR: 8260695: The java.awt.color.ICC_Profile#getData/getData(int) are not thread safe [v2]
> Both methods are implemented in a similar way. > 1. Requests the size of the profile/tag data > 2. Creates an array of the correct size > 3. Requests the data and copy it to the array > > If the data will be changed concurrently between steps 2. and 3. then we will > get a mismatch between the array and copied data. > > In the fix, all steps above are merged to just one step - return the data > when requested. Sergey Bylokhov has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision: - Merge branch 'master' into JDK-8260695 - cleanup - Merge branch 'JDK-8260695' of https://github.com/mrserb/jdk into JDK-8260695 - Update LCMSProfile.java - Update LCMSProfile.java - Create MTGetData.java - Initial fix - Changes: - all: https://git.openjdk.java.net/jdk/pull/2330/files - new: https://git.openjdk.java.net/jdk/pull/2330/files/162fde0c..a9347ae3 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2330=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2330=00-01 Stats: 15457 lines in 693 files changed: 9560 ins; 3448 del; 2449 mod Patch: https://git.openjdk.java.net/jdk/pull/2330.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2330/head:pull/2330 PR: https://git.openjdk.java.net/jdk/pull/2330
Re: [OpenJDK 2D-Dev] RFR: 6211198: ICC_Profile.getInstance(byte[]): IAE is not specified [v2]
> The specification of the java.awt.color.ICC_Profile.getInstance(byte[]) is > updated. > Its implementation changed over time, and different exceptions were thrown, > but since JDK-8013430 always throws an IllegalArgumentException on null and > invalid data. Sergey Bylokhov has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision: - Merge branch 'master' into JDK-6211198 - Merge branch 'master' into JDK-6211198 - Initial fix - Changes: - all: https://git.openjdk.java.net/jdk/pull/2328/files - new: https://git.openjdk.java.net/jdk/pull/2328/files/f1d171a9..12f673c1 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2328=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2328=00-01 Stats: 15457 lines in 693 files changed: 9560 ins; 3448 del; 2449 mod Patch: https://git.openjdk.java.net/jdk/pull/2328.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2328/head:pull/2328 PR: https://git.openjdk.java.net/jdk/pull/2328
[OpenJDK 2D-Dev] Integrated: 8261200: Some code in the ICC_Profile may not close file streams properly
On Fri, 5 Feb 2021 04:05:12 GMT, Sergey Bylokhov wrote: > In a few places where we use streams the try/with/res blocks are added. This pull request has now been integrated. Changeset: 74d40ab7 Author:Sergey Bylokhov URL: https://git.openjdk.java.net/jdk/commit/74d40ab7 Stats: 105 lines in 2 files changed: 80 ins; 16 del; 9 mod 8261200: Some code in the ICC_Profile may not close file streams properly Reviewed-by: azvegint - PR: https://git.openjdk.java.net/jdk/pull/2421
Re: [OpenJDK 2D-Dev] RFR: 8261200: Some code in the ICC_Profile may not close file streams properly [v2]
On Sat, 6 Feb 2021 02:11:16 GMT, Sergey Bylokhov wrote: >> In a few places where we use streams the try/with/res blocks are added. > > Sergey Bylokhov has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains five additional > commits since the last revision: > > - Update WriteProfileToFile.java > - Merge branch 'master' into JDK-8261200 > - Update ICC_Profile.java > - Update ICC_Profile.java > - Initial fix Marked as reviewed by azvegint (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2421
Re: [OpenJDK 2D-Dev] RFR: 8261200: Some code in the ICC_Profile may not close file streams properly [v2]
On Sat, 6 Feb 2021 01:23:54 GMT, Alexander Zvegintsev wrote: >> Sergey Bylokhov has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains five additional >> commits since the last revision: >> >> - Update WriteProfileToFile.java >> - Merge branch 'master' into JDK-8261200 >> - Update ICC_Profile.java >> - Update ICC_Profile.java >> - Initial fix > > test/jdk/java/awt/color/ICC_Profile/WriteProfileToFile.java line 83: > >> 81: } >> 82: } >> 83: } > > Just a matter of taste, but if you OK with not elaborating why arrays > differs, the method may be shortened with `Arrays.equals()` usage. updated > test/jdk/java/awt/color/ICC_Profile/WriteProfileToFile.java line 43: > >> 41: testViaDataArray(gold); >> 42: testViaFile(gold); >> 43: testViaStream(gold); > > Are we OK with not calling `testViaFile()` and `testViaStream` in case of > `testViaDataArray()` failure? yeah, it intentionally "fails fast". - PR: https://git.openjdk.java.net/jdk/pull/2421
Re: [OpenJDK 2D-Dev] RFR: 8261200: Some code in the ICC_Profile may not close file streams properly
On Fri, 5 Feb 2021 04:05:12 GMT, Sergey Bylokhov wrote: > In a few places where we use streams the try/with/res blocks are added. test/jdk/java/awt/color/ICC_Profile/WriteProfileToFile.java line 43: > 41: testViaDataArray(gold); > 42: testViaFile(gold); > 43: testViaStream(gold); Are we OK with not calling `testViaFile()` and `testViaStream` in case of `testViaDataArray()` failure? - PR: https://git.openjdk.java.net/jdk/pull/2421
Re: [OpenJDK 2D-Dev] RFR: 8261200: Some code in the ICC_Profile may not close file streams properly
On Fri, 5 Feb 2021 04:05:12 GMT, Sergey Bylokhov wrote: > In a few places where we use streams the try/with/res blocks are added. test/jdk/java/awt/color/ICC_Profile/WriteProfileToFile.java line 83: > 81: } > 82: } > 83: } Just a matter of taste, but if you OK with not elaborating why arrays differs, the method may be shortened with `Arrays.equals()` usage. - PR: https://git.openjdk.java.net/jdk/pull/2421
Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline
On Thu, 4 Feb 2021 10:35:02 GMT, Ajit Ghaisas wrote: > **Description :** > This is the implementation of [JEP 382 : New macOS Rendering > Pipeline](https://bugs.openjdk.java.net/browse/JDK-8238361) > It implements a Java 2D internal rendering pipeline for macOS using the Apple > Metal API. > The entire work on this was done under [OpenJDK Project - > Lanai](http://openjdk.java.net/projects/lanai/) > > We iterated through several Early Access (EA) builds and have reached a stage > where it is ready to be integrated to openjdk/jdk. The latest EA build is > available at - https://jdk.java.net/lanai/ > > A new option -Dsun.java2d.metal=true | True needs to be used to use this > pipeline. > > **Testing :** > This implementation has been tested with the tests present at - [Test Plan > for JEP 382: New macOS Rendering > Pipeline](https://bugs.openjdk.java.net/browse/JDK-8251396) > > **Note to reviewers :** > 1) Default rendering pipeline on macOS has not been changed by this PR. > OpenGL still stays as the default rendering pipeline and Metal rendering > pipeline is optional to choose. > > 2) To apply and test this PR - > To enable the metal pipeline you must specify on command line > -Dsun.java2d.metal=true (No message will be printed in this case) or > -Dsun.java2d.metal=True (A message indicating Metal rendering pipeline is > enabled gets printed) The file in `RenderPerfTest` should have a GPLv2 license header (no Classpath). I filed [JDK-8261273](https://bugs.openjdk.java.net/browse/JDK-8261273) and also highlighted a couple examples below. test/jdk/performance/client/RenderPerfTest/Makefile line 10: > 8: # - Redistributions of source code must retain the above copyright > 9: # notice, this list of conditions and the following disclaimer. > 10: # As mentioned in the preliminary review, this file and `build.xml` have a BSD copyright. I think that should be GPLv2 (without classpath exception since this is a test). test/jdk/performance/client/RenderPerfTest/src/renderperf/RenderPerfTest.java line 9: > 7: * published by the Free Software Foundation. Oracle designates this > 8: * particular file as subject to the "Classpath" exception as provided > 9: * by Oracle in the LICENSE file that accompanied this code. Since this a test, this should not have the Classpath exception. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: [OpenJDK 2D-Dev] RFR: 6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified [v2]
On Thu, 4 Feb 2021 04:15:03 GMT, Prasanta Sadhukhan
wrote:
>> Method createStrokedShape(Shape) for java.awt.BasicStroke class throws
>> NullPointerException when passed a null object reference for a input
>> parameter but it's not specified in the spec.
>> Updated spec to illustrate this.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Doc change to @throws
Changes requested by serb (Reviewer).
src/java.desktop/share/classes/java/awt/BasicStroke.java line 297:
> 295: * @param s the {@code Shape} boundary be stroked
> 296: * @return the {@code Shape} of the stroked outline.
> 297: * @throws NullPointerException if {@code Shape} is {@code null}
I think that you mean the parameter {@code s}, not a type Shape.
test/jdk/java/awt/BasicStroke/TestNullShape.java line 40:
> 38: System.out.println("result: false");
> 39: }
> 40: catch(NullPointerException ne) {
You can add a space after catch and move it to one line above.
-
PR: https://git.openjdk.java.net/jdk/pull/2377
[OpenJDK 2D-Dev] RFR: 8261200: Some code in the ICC_Profile may not close file streams properly
In a few places where we use streams the try/with/res blocks are added. - Commit messages: - Update ICC_Profile.java - Update ICC_Profile.java - Initial fix Changes: https://git.openjdk.java.net/jdk/pull/2421/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2421=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8261200 Stats: 109 lines in 2 files changed: 84 ins; 16 del; 9 mod Patch: https://git.openjdk.java.net/jdk/pull/2421.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2421/head:pull/2421 PR: https://git.openjdk.java.net/jdk/pull/2421
Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline
On Thu, 4 Feb 2021 10:35:02 GMT, Ajit Ghaisas wrote:
> **Description :**
> This is the implementation of [JEP 382 : New macOS Rendering
> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8238361)
> It implements a Java 2D internal rendering pipeline for macOS using the Apple
> Metal API.
> The entire work on this was done under [OpenJDK Project -
> Lanai](http://openjdk.java.net/projects/lanai/)
>
> We iterated through several Early Access (EA) builds and have reached a stage
> where it is ready to be integrated to openjdk/jdk. The latest EA build is
> available at - https://jdk.java.net/lanai/
>
> A new option -Dsun.java2d.metal=true | True needs to be used to use this
> pipeline.
>
> **Testing :**
> This implementation has been tested with the tests present at - [Test Plan
> for JEP 382: New macOS Rendering
> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8251396)
>
> **Note to reviewers :**
> 1) Default rendering pipeline on macOS has not been changed by this PR.
> OpenGL still stays as the default rendering pipeline and Metal rendering
> pipeline is optional to choose.
>
> 2) To apply and test this PR -
> To enable the metal pipeline you must specify on command line
> -Dsun.java2d.metal=true (No message will be printed in this case) or
> -Dsun.java2d.metal=True (A message indicating Metal rendering pipeline is
> enabled gets printed)
I left a few additional comments. Overall this looks good to me.
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLGraphicsConfig.m
line 82:
> 80: (JNIEnv *env, jclass mtlgc)
> 81: {
> 82: FILE *f = popen("/usr/sbin/system_profiler SPDisplaysDataType", "r");
How robust is this? It seems like the contents of this could be an
implementation detail and subject to change. Is it documented by Apple?
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLGraphicsConfig.m
line 83:
> 81: {
> 82: FILE *f = popen("/usr/sbin/system_profiler SPDisplaysDataType", "r");
> 83: bool metalSupported = JNI_FALSE;
This code is mixing types; it should be `jboolean metalSupported`
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLMaskFill.m line 26:
> 24: */
> 25:
> 26: #ifndef HEADLESS
I see a few occurrences of `#ifndef HEADLESS` in the metal pipeline. Is this
needed? I don't see any of the other native macos files in Java2D (e.g., the
OpenGL pipeline) doing the same. Will this file ever be compiled with HEADLESS
being undefined?
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLPaints.h line 41:
> 39: /**
> 40: * The MTLPaint class represents paint mode (color, gradient, e.t.c.)
> 41: * */
Minor: `* */` --> `*/`; also typo: `e.t.c.` should be `etc.`
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLSurfaceDataBase.h
line 37:
> 35:
> 36: /**
> 37: * The MTLSDOps structure describes a native OpenGL surface and contains
> all
Should that be "Metal" and not "OpenGL" ?
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLTextRenderer.m line
77:
> 75: * be safe to use this one glyph cache for all screens in a multimon
> 76: * environment, since the glyph cache texture is shared between all
> contexts,
> 77: * and (in theory) OpenGL drivers should be smart enough to manage that
`Metal` drivers?
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLTexurePool.m line
29:
> 27: #import "Trace.h"
> 28:
> 29: #define SCREEN_MEMORY_SIZE_4K (4096*2160*4) //~33,7 mb
This means that a 4k display with a narrower aspect ratio wouldn't fit
(assuming there ever were to be such a thing). What would happen if you
encountered a screen that was, say, 4k * 2.5K?
-
PR: https://git.openjdk.java.net/jdk/pull/2403
[OpenJDK 2D-Dev] Integrated: 8216358: [accessibility] [macos] The focus is invisible when tab to "Image Radio Buttons" and "Image CheckBoxes"
On Wed, 3 Feb 2021 18:50:34 GMT, Alexander Zuev wrote: > 8216358: [accessibility] [macos] The focus is invisible when tab to "Image > Radio Buttons" and "Image CheckBoxes" This pull request has now been integrated. Changeset: 440db35e Author:Alexander Zuev URL: https://git.openjdk.java.net/jdk/commit/440db35e Stats: 117 lines in 2 files changed: 115 ins; 0 del; 2 mod 8216358: [accessibility] [macos] The focus is invisible when tab to "Image Radio Buttons" and "Image CheckBoxes" Reviewed-by: serb, pbansal - PR: https://git.openjdk.java.net/jdk/pull/2384
Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline
On Thu, 4 Feb 2021 10:35:02 GMT, Ajit Ghaisas wrote:
> **Description :**
> This is the implementation of [JEP 382 : New macOS Rendering
> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8238361)
> It implements a Java 2D internal rendering pipeline for macOS using the Apple
> Metal API.
> The entire work on this was done under [OpenJDK Project -
> Lanai](http://openjdk.java.net/projects/lanai/)
>
> We iterated through several Early Access (EA) builds and have reached a stage
> where it is ready to be integrated to openjdk/jdk. The latest EA build is
> available at - https://jdk.java.net/lanai/
>
> A new option -Dsun.java2d.metal=true | True needs to be used to use this
> pipeline.
>
> **Testing :**
> This implementation has been tested with the tests present at - [Test Plan
> for JEP 382: New macOS Rendering
> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8251396)
>
> **Note to reviewers :**
> 1) Default rendering pipeline on macOS has not been changed by this PR.
> OpenGL still stays as the default rendering pipeline and Metal rendering
> pipeline is optional to choose.
>
> 2) To apply and test this PR -
> To enable the metal pipeline you must specify on command line
> -Dsun.java2d.metal=true (No message will be printed in this case) or
> -Dsun.java2d.metal=True (A message indicating Metal rendering pipeline is
> enabled gets printed)
Here is my initial set of mostly minor comments and a couple questions.
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLLayer.h line 33:
> 31: #import "common.h"
> 32:
> 33: #import
JNF has been removed from the jdk, so this must be removed or it will no longer
compile. It has already been fixed in the lanai repo by
[JDK-8261234](https://bugs.openjdk.java.net/browse/JDK-8261234).
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLGraphicsConfig.m
line 38:
> 36: #import
> 37: #import
> 38: #import
JNF has been removed from the jdk, so this must be removed or it will no longer
compile. It has already been fixed in the lanai repo by
[JDK-8261234](https://bugs.openjdk.java.net/browse/JDK-8261234).
make/modules/java.desktop/lib/Awt2dLibraries.gmk line 894:
> 892: SHADERS_SUPPORT_DIR :=
> $(SUPPORT_OUTPUTDIR)/native/java.desktop/libosxui
> 893: SHADERS_AIR := $(SHADERS_SUPPORT_DIR)/shaders.air
> 894: SHADERS_LIB := $(INSTALL_LIBRARIES_HERE)/shaders.metallib
Q: Should 2d (or awt) be in the name of this file, or is a generic name OK?
src/java.desktop/macosx/classes/sun/java2d/metal/MTLGraphicsConfig.java line 2:
> 1: /*
> 2: * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved.
Should be `2019, 2021,` (I missed this file when I fixed up the copyright
notices and years).
src/java.desktop/macosx/classes/sun/java2d/metal/MTLGraphicsConfig.java line 81:
> 79: (PrivilegedAction) () ->
> 80: System.getProperty("java.home", "") + File.separator +
> 81: "lib" + File.separator + "shaders.metallib");
Same question as in the makefile about the name of the shader library file.
src/java.desktop/macosx/classes/sun/java2d/metal/MTLGraphicsConfig.java line
155:
> 153: if (cfginfo != 0L) {
> 154: textureSize = nativeGetMaxTextureSize();
> 155: // 7160609: GL still fails to create a square texture of
> this
This refers to GL. Is this relevant to metal?
src/java.desktop/macosx/classes/sun/java2d/metal/MTLRenderQueue.java line 78:
> 76: * of the MTL pipeline and related classes.
> 77: */
> 78: public static void sync() {
Should this be synchronized?
src/java.desktop/macosx/classes/sun/java2d/opengl/CGLGraphicsConfig.java line
142:
> 140: // TODO : This clamping code is same as in OpenGL.
> 141: // Whether we need such clamping or not in case of Metal
> 142: // will be pursued under 8260644
This change seems wrong. The comment suggests it belong in a metal class or
method, not here where we are using OpenGL.
src/java.desktop/macosx/classes/sun/lwawt/macosx/CPlatformView.java line 193:
> 191: responder.handleScrollEvent(x, y, absX, absY,
> event.getModifierFlags(),
> 192: event.getScrollDeltaX(), event.getScrollDeltaY(),
> 193: event.getScrollPhase());
Minor: This part of the file is otherwise unchanged. Maybe revert this
whitespace-only change? Same applies to the last two changes in this file.
src/java.desktop/macosx/classes/sun/lwawt/macosx/CWarningWindow.java line 227:
> 225: CWrapper.NSWindow.orderWindow(ptr,
> 226: CWrapper.NSWindow.NSWindowAbove,
> 227: ownerPtr);
Minor: This part of the file is otherwise unchanged. Maybe revert this
whitespace-only change? Same applies futher down in a couple places.
Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline
On Thu, 4 Feb 2021 10:35:02 GMT, Ajit Ghaisas wrote:
> **Description :**
> This is the implementation of [JEP 382 : New macOS Rendering
> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8238361)
> It implements a Java 2D internal rendering pipeline for macOS using the Apple
> Metal API.
> The entire work on this was done under [OpenJDK Project -
> Lanai](http://openjdk.java.net/projects/lanai/)
>
> We iterated through several Early Access (EA) builds and have reached a stage
> where it is ready to be integrated to openjdk/jdk. The latest EA build is
> available at - https://jdk.java.net/lanai/
>
> A new option -Dsun.java2d.metal=true | True needs to be used to use this
> pipeline.
>
> **Testing :**
> This implementation has been tested with the tests present at - [Test Plan
> for JEP 382: New macOS Rendering
> Pipeline](https://bugs.openjdk.java.net/browse/JDK-8251396)
>
> **Note to reviewers :**
> 1) Default rendering pipeline on macOS has not been changed by this PR.
> OpenGL still stays as the default rendering pipeline and Metal rendering
> pipeline is optional to choose.
>
> 2) To apply and test this PR -
> To enable the metal pipeline you must specify on command line
> -Dsun.java2d.metal=true (No message will be printed in this case) or
> -Dsun.java2d.metal=True (A message indicating Metal rendering pipeline is
> enabled gets printed)
Looks good except for one whitespace problem
(src/java.desktop/share/native/libawt/java2d/SurfaceData.c)
src/java.desktop/share/native/libawt/java2d/SurfaceData.c line 238:
> 236: {
> 237: SurfaceDataOps *ops = malloc(opsSize);
> 238:
The only whitespace added. Should be removed
-
Changes requested by avu (no project role).
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Tue, 2 Feb 2021 18:35:51 GMT, Gerard Ziemski wrote: >> Anton Kozlov has updated the pull request incrementally with one additional >> commit since the last revision: >> >> support macos_aarch64 in hsdis > > src/hotspot/cpu/aarch64/vm_version_aarch64.hpp line 93: > >> 91: CPU_MARVELL = 'V', >> 92: CPU_INTEL = 'i', >> 93: CPU_APPLE = 'a', > > The `ARM Architecture Reference Manual ARMv8, for ARMv8-A architecture > profile` has 8538 pages, can we be more specific and point to the particular > section of the document where the CPU codes are defined? They are defined in 13.2.95. MIDR_EL1, Main ID Register. Apple's code is not there, but "Arm can assign codes that are not published in this manual. All values not assigned by Arm are reserved and must not be used.". I assume the value was obtained by digging around https://github.com/apple/darwin-xnu/blob/main/osfmk/arm/cpuid.h#L62 - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Wed, 3 Feb 2021 23:29:30 GMT, Gerard Ziemski wrote: >> using ` ```c ` >> https://docs.github.com/en/github/writing-on-github/creating-and-highlighting-code-blocks >> >> I was wrong about `SIGFPE` / `EXC_MASK_ARITHMETIC`, it's used on i386, >> x86_64: >> https://github.com/openjdk/jdk/blob/2be60e37e0e433141b2e3d3e32f8e638a4888e3a/src/hotspot/os_cpu/bsd_x86/os_bsd_x86.cpp#L467-L524 >> and aarch64: >> https://github.com/AntonKozlov/jdk/blob/80827176cbc5f0dd26003cf234a8076f3f557928/src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp#L309-L323 >> (What happened with the formatting here, ugh?) >> >> Your suggestion sounds good otherwise. @AntonKozlov, do you mind to >> integrate that? > > So it should be: > > #if defined(__APPLE__) > // lldb (gdb) installs both standard BSD signal handlers, and mach exception > // handlers. By replacing the existing task exception handler, we disable > lldb's mach > // exception handling, while leaving the standard BSD signal handlers > functional. > // > // EXC_MASK_BAD_ACCESS needed by all architectures for NULL ptr checking > // EXC_MASK_ARITHMETIC needed by all architectures for div by 0 checking > // EXC_MASK_BAD_INSTRUCTION needed by aarch64 to initiate deoptimization > kern_return_t kr; > kr = task_set_exception_ports(mach_task_self(), > EXC_MASK_BAD_ACCESS | EXC_MASK_ARITHMETIC > AARCH64_ONLY(| EXC_MASK_BAD_INSTRUCTION), > MACH_PORT_NULL, > EXCEPTION_STATE_IDENTITY, > MACHINE_THREAD_STATE); > > assert(kr == KERN_SUCCESS, "could not set mach task signal handler"); > #endif Thanks! I've updated the PR with this code, with extra indentation of `AARCH64_ONLY(...)` line, since this is continuation of the first parameter. I'll fix the formatting in os_bsd_arch64.cpp along other changes to `bsd_aarch64` directory - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 6211257: BasicStroke.createStrokedShape(Shape): NPE is not specified [v2]
On Thu, 4 Feb 2021 04:15:03 GMT, Prasanta Sadhukhan
wrote:
>> Method createStrokedShape(Shape) for java.awt.BasicStroke class throws
>> NullPointerException when passed a null object reference for a input
>> parameter but it's not specified in the spec.
>> Updated spec to illustrate this.
>
> Prasanta Sadhukhan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Doc change to @throws
test/jdk/java/awt/BasicStroke/TestNullShape.java line 44:
> 42: return;
> 43: }
> 44: throw new RuntimeException("NPE is expected");
I would suggest moving this into the try-block as it would make the intention
clearer: if NPE is not thrown, it's a failure.
Also catch should be on the same line as the closing brace of try.
-
PR: https://git.openjdk.java.net/jdk/pull/2377
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v12]
> Please review the implementation of JEP 391: macOS/AArch64 Port. > > It's heavily based on existing ports to linux/aarch64, macos/x86_64, and > windows/aarch64. > > Major changes are in: > * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks > JDK-8253817, JDK-8253818) > * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with necessary > adjustments (JDK-8253819) > * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), > required on macOS/AArch64 platform. It's implemented with > pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a > thread, so W^X mode change relates to the java thread state change (for java > threads). In most cases, JVM executes in write-only mode, except when calling > a generated stub like SafeFetch, which requires a temporary switch to > execute-only mode. The same execute-only mode is enabled when a java thread > executes in java or native states. This approach of managing W^X mode turned > out to be simple and efficient enough. > * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941) Anton Kozlov has updated the pull request incrementally with one additional commit since the last revision: Update signal handler part for debugger - Changes: - all: https://git.openjdk.java.net/jdk/pull/2200/files - new: https://git.openjdk.java.net/jdk/pull/2200/files/8652d21d..0d0e9baf Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2200=11 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2200=10-11 Stats: 16 lines in 1 file changed: 5 ins; 8 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/2200.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2200/head:pull/2200 PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Fri, 5 Feb 2021 12:26:27 GMT, Anton Kozlov wrote: >> Marked as reviewed by ihse (Reviewer). > >> I haven't got a MacOS AArch64 system right now. Is it possible to >> enable W^X in Linux in order to kick the tyres? > > I've just got rid of asserts that fired on Linux sometime :) As for W^X like > on macOS, I vaguely remember working with a Linux system with one-way > transition W->X, probably provided by SELinux. But I don't think it allowed > per-thread W^X state. > _Mailing list message from [daniel.daugherty at > oracle.com](mailto:daniel.daughe...@oracle.com) on > [security-dev](mailto:security-...@openjdk.java.net):_ > > On 2/5/21 4:51 AM, Magnus Ihse Bursie wrote: > > @magicus - I'm good with both of these answers. I personally like 'macosx'. > > Dan It's no longer `macosx`, it's just `macos` now - see https://en.wikipedia.org/wiki/MacOS - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On 2/5/21 4:51 AM, Magnus Ihse Bursie wrote: On Tue, 2 Feb 2021 21:20:52 GMT, Daniel D. Daugherty wrote: Anton Kozlov has updated the pull request incrementally with one additional commit since the last revision: support macos_aarch64 in hsdis make/autoconf/flags.m4 line 140: 138: else 139: MACOSX_VERSION_MIN=10.12.0 140: fi Not something that needs to be addressed here, but these changes illustrate that our collective use of macOSX/MACOSX/MacOSX names are tied to the fact that the macOS major version number was at 10 for a very long time. @magicus - Do we have an RFE to rename MACOSX or are we sticking with it and evolving our interpretation of the 'X' from '10' to */splat/asterik? @dcubed-ojdk There is no RFE to renaming "macosx" to "macos". I'm not sure it should be done. We can't follow all marketing trends (Apple recently renamed iOS to iPadOS for the iPad; we can't keep adapting to such schemes). Personally, I like the new name without the "x", but we had already spent some time trying to find and fix all (or at least, most) instances of "osx" in the code, that I don't really think it's worth the effort. If you can drill up enough enthusiasm for such a project, and get any objections down to minimum, I can help implementing it. But I won't be spearheading it. make/common/NativeCompilation.gmk line 1178: 1176: endif 1177: # This only works if the openjdk_codesign identity is present on the system. Let 1178: # silently fail otherwise. Might want to add a comment here: # The '-f' option will replace an existing signature if one exists. We're not really in the habit of adding comments for various command line options. Normally, you can check these with "man" if you are uncertain. If they do something surprising, sure, but here it's more of a "it's needed on aarch64 to work at all", so I don't think a comment will be anything but added clutter. - PR: https://git.openjdk.java.net/jdk/pull/2200 @magicus - I'm good with both of these answers. I personally like 'macosx'. Dan
Re: [OpenJDK 2D-Dev] EA8 build of Project Lanai (Java 2D Metal rendering pipeline for macOS) is now posted
> On Dec 16, 2020, at 11:57 PM, Philip Race wrote: > > To anyone who has a mac still running 10.12 - we don't expect Metal to run > (it requires at least 10.13 > and maybe even later by the time it is final) but we would like confirmation > that nothing in Metal > prevents OpenGL running on older releases. > Note there is currently a hotspot build bug unrelated to Lanai that prevents > running on 10.10 > and maybe 10.11 but 10.12 will be a useful data point. I tried running it on macOS 10.12 under Parallels Desktop. If I don’t ask for metal, it seems to work. If I ask for metal, it fails awkwardly: Failed to load library. error (null)
[OpenJDK 2D-Dev] [11u] Backport of 8247872: Upgrade HarfBuzz to the latest 2.7.2
Hi, I've created a jdk11u backport of JDK-8247872: Upgrade HarfBuzz to the latest 2.7.2 http://cr.openjdk.java.net/~mdoerr/8247872_harfbuzz_11u/webrev.00/ I had to resolve it a bit manually [1]. However, the main problem is that it heavily uses C++11, so easiest solution is to add HARFBUZZ_CFLAGS := -std=c++11. This works for all supported compilers [2] except xlC on AIX and sun studio on Solaris. I could envision different solutions: 1. Change code to work with older C++ standard. I don't want to do this because this seems to be significant effort and is probably error prone (I'm not really familiar with the code and TMP code is hard to adapt). And we may have to repeat it for future HB upgrades. 2. Upgrade compilers: This is possible on AIX. We'd need to backport some build changes and use xlclang++. But I guess there's no solaris studio available which can compile it. Note that the original bug was blocked by JEP 362: Deprecate the Solaris and SPARC Ports. 3. Use configure flag --with-harfbuzz=system on the problematic platforms. Disadvantage: All machines require new enough version of libharfbuzz installed. 4. Skip JDK-8247872 backport. Oracle must have had a reason for backporting it to 11.0.11-oracle, so some jdk11u users will probably want to have it at least on the main platforms. Any comments or ideas will be appreciated. Best regards, Martin [1] Resolution steps: - Manually delete hb-dsalgs.hh. - Awt2dLibraries.gmk is at a different location in 11u. - Integrate a few hunks manually due to minor adaptations for Solaris build: see http://cr.openjdk.java.net/~mdoerr/8247872_harfbuzz_11u/8247872_harfbuzz_failing_hunks.txt - Add HARFBUZZ_CFLAGS := -std=c++11 to Awt2dLibraries.gmk. [2] Supported compilers: https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms
Re: [OpenJDK 2D-Dev] RFR: 8252999: Cleanup: replace .equals("") with .isEmpty() within all codebase
On 10/09/2020 10:07 pm, Dmitriy Dumanskiy wrote: On Thu, 10 Sep 2020 11:21:28 GMT, David Holmes wrote: The code in java.base was updated to use String::isEmpty in JDK 12 (JDK-8215281). There was follow-up in JDK 13 to do the same in the java.desktop module (JDK-8223237). Changing the remaining usages make sense although I see that more more than half are in tests. It would be good to hear from security-dev on the changes to the Apache Santuario code (in java.xml.crypto module) in case it would be better to contribute those upstream instead. Ditto for the Apache Xalan code (in the java.xml module) but it may be significantly forked already that it doesn't matter. I assume you can use JDK-8215014 rather than creating a new issue. This should be broken up to deal with the files in different functional areas under different bugids and PRs. Otherwise the cross-posting to so many lists is prohibitive. Files in different areas need to be reviewed by different teams. Thank you. I have in mind dozens of improvements all over the code like this one. I already did some further review and in most cases, those tiny changes go trough all codebase. I can create PR for every separate module, but in general, it would multiply x5 the number of PRs in total. If you think it's a better way to go, no problem, I can do that. Find a reasonable middle ground. You have around 14 mailing lists cc'd here, for changes on 150 files. It is very unlikely anyone will review all 150, and files in different areas are, by convention, reviewed by Reviewers for those areas. Even if people only look at a subset of files, communicating that to you effectively so you can figure out when all 150 have been reviewed, is not very practical. My initial breakdown would be: - build - desktop (awt/swing/2d) - serviceability/jmx (the SA and JVMTI changes) - security - core-libs Also note that while the original PR email went out to 14 lists, most people trying to reply-all won't be able to as they don't subscribe to all 14 lists, so the review threads will get very fragmented. Cheers, David - - PR: https://git.openjdk.java.net/jdk/pull/29
Re: [OpenJDK 2D-Dev] [11u] Backport of 8247872: Upgrade HarfBuzz to the latest 2.7.2
Hi Christoph, > is there any other reason why harfbuzz 2.7.2 needs to be backported to > OpenJDK 11 beyond the fact that Oracle did it? I don't have other reasons at the moment. I'd be fine with deferring it until anyone has a demand. I'm just evaluating options in order to be prepared. > I think both, having to uplift compiler requirements and/or requiring > additional system dependencies, would be a price too high to pay for this. > And modifying the code to work without c++11 features doesn't seem > feasible. I agree with that. Note that officially supported compilers for "main platforms" (not AIX, not Solaris) don't need upgrades (https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms). It should also be possible to add both versions and modify the build such that it uses the old HarfBuzz version only for old compilers. But I'd vote for that only if we really need the new version on "main platforms". Best regards, Martin > -Original Message- > From: Langer, Christoph > Sent: Donnerstag, 7. Januar 2021 23:42 > To: Severin Gehwolf ; Doerr, Martin > ; jdk-updates-...@openjdk.java.net; 2d- > d...@openjdk.java.net > Subject: RE: [OpenJDK 2D-Dev] [11u] Backport of 8247872: Upgrade HarfBuzz > to the latest 2.7.2 > > Hi, > > is there any other reason why harfbuzz 2.7.2 needs to be backported to > OpenJDK 11 beyond the fact that Oracle did it? > > I think both, having to uplift compiler requirements and/or requiring > additional system dependencies, would be a price too high to pay for this. > And modifying the code to work without c++11 features doesn't seem > feasible. > > Just my .02€ at this point... > > Best regards > Christoph > > > -Original Message- > > From: 2d-dev <2d-dev-r...@openjdk.java.net> On Behalf Of Severin > > Gehwolf > > Sent: Donnerstag, 7. Januar 2021 18:32 > > To: Doerr, Martin ; jdk-updates- > > d...@openjdk.java.net; 2d-dev@openjdk.java.net > > Subject: Re: [OpenJDK 2D-Dev] [11u] Backport of 8247872: Upgrade > HarfBuzz > > to the latest 2.7.2 > > > > On Thu, 2021-01-07 at 17:16 +, Doerr, Martin wrote: > > > Hi, > > > > > > I've created a jdk11u backport of JDK-8247872: Upgrade HarfBuzz to > > > the latest 2.7.2 > > > http://cr.openjdk.java.net/~mdoerr/8247872_harfbuzz_11u/webrev.00/ > > > > > > I had to resolve it a bit manually [1]. > > > However, the main problem is that it heavily uses C++11, so easiest > > > solution is to add HARFBUZZ_CFLAGS := -std=c++11. > > > This works for all supported compilers [2] except xlC on AIX and sun > > > studio on Solaris. > > > > We are building vanilla JDK 11 builds with gcc 4.4.7 which doesn't > > support the -std=c++11 option. We've had issues in that area before. > > See JDK-8256557. > > > > Thanks, > > Severin > > > > > I could envision different solutions: > > > > > > 1. Change code to work with older C++ standard. I don't want to do > > > this because this seems to be significant effort and is probably > > > error prone (I'm not really familiar with the code and TMP code is > > > hard to adapt). And we may have to repeat it for future HB upgrades. > > > 2. Upgrade compilers: This is possible on AIX. We'd need to > > > backport some build changes and use xlclang++. But I guess there's no > > > solaris studio available which can compile it. Note that the original > > > bug was blocked by JEP 362: Deprecate the Solaris and SPARC Ports. > > > 3. Use configure flag --with-harfbuzz=system on the problematic > > > platforms. Disadvantage: All machines require new enough version of > > > libharfbuzz installed. > > > 4. Skip JDK-8247872 backport. Oracle must have had a reason for > > > backporting it to 11.0.11-oracle, so some jdk11u users will probably > > > want to have it at least on the main platforms. > > > > > > Any comments or ideas will be appreciated. > > > > > > Best regards, > > > Martin > > > > > > > > > [1] Resolution steps: > > > - Manually delete hb-dsalgs.hh. > > > - Awt2dLibraries.gmk is at a different location in 11u. > > > - Integrate a few hunks manually due to minor adaptations for Solaris > > > build: see > > > > > > http://cr.openjdk.java.net/~mdoerr/8247872_harfbuzz_11u/8247872_harfb > > uzz_failing_hunks.txt > > > - Add HARFBUZZ_CFLAGS := -std=c++11 to Awt2dLibraries.gmk. > > > > > > [2] Supported compilers: > > > https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms > > > > >
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v11]
> Please review the implementation of JEP 391: macOS/AArch64 Port. > > It's heavily based on existing ports to linux/aarch64, macos/x86_64, and > windows/aarch64. > > Major changes are in: > * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks > JDK-8253817, JDK-8253818) > * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with necessary > adjustments (JDK-8253819) > * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), > required on macOS/AArch64 platform. It's implemented with > pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a > thread, so W^X mode change relates to the java thread state change (for java > threads). In most cases, JVM executes in write-only mode, except when calling > a generated stub like SafeFetch, which requires a temporary switch to > execute-only mode. The same execute-only mode is enabled when a java thread > executes in java or native states. This approach of managing W^X mode turned > out to be simple and efficient enough. > * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941) Anton Kozlov has updated the pull request incrementally with one additional commit since the last revision: Cleanup SA changes - Changes: - all: https://git.openjdk.java.net/jdk/pull/2200/files - new: https://git.openjdk.java.net/jdk/pull/2200/files/80827176..8652d21d Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2200=10 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2200=09-10 Stats: 11 lines in 1 file changed: 3 ins; 8 del; 0 mod Patch: https://git.openjdk.java.net/jdk/pull/2200.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2200/head:pull/2200 PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v3]
On Mon, 25 Jan 2021 22:48:50 GMT, Chris Plummer wrote: >> Anton Kozlov has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Refactor CDS disabling >> - Redo builsys support for aarch64-darwin > > src/jdk.hotspot.agent/macosx/native/libsaproc/MacosxDebuggerLocal.m line 702: > >> 700: primitiveArray = (*env)->GetLongArrayElements(env, registerArray, >> NULL); >> 701: >> 702: #undef REG_INDEX > > I'm not so sure why the #undef and subsequent #define of REG_INDEX is needed > since it seems to just get #define'd back to the same value. We've merged two implementations of SA, this change slipped in. I've cleaned this up. Thanks for noticing! - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Fri, 5 Feb 2021 09:57:54 GMT, Magnus Ihse Bursie wrote: >> Anton Kozlov has updated the pull request incrementally with six additional >> commits since the last revision: >> >> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos >> - Add comments to WX transitions >> >>+ minor change of placements >> - Use macro conditionals instead of empty functions >> - Add W^X to tests >> - Do not require known W^X state >> - Revert w^x in gtests > > Marked as reviewed by ihse (Reviewer). > I haven't got a MacOS AArch64 system right now. Is it possible to > enable W^X in Linux in order to kick the tyres? I've just got rid of asserts that fired on Linux sometime :) As for W^X like on macOS, I vaguely remember working with a Linux system with one-way transition W->X, probably provided by SELinux. But I don't think it allowed per-thread W^X state. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Fri, 5 Feb 2021 09:49:11 GMT, Andrew Haley wrote: >> I reviewed bsd_aarch64.cpp digging bit deeper and left some comments. > >> > Umm, so how does patching work? We don't even know if other threads are >> > executing the code we need to patch. >> >> I thought java can handle that scenario in usual (non W^X systems) just >> fine, so we just believe jvm did everything right and it's safe to rewrite >> some code at specific moment. > > Got it, OK. > This doesn't seem to be an issue anymore, After P.Race have finished with > JDK-8257852, Macarm port can be build without extra ld flags. @VladimirKempik I agree. That concludes the build issues with this PR. So from a build perspective, this is now good to go. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Wed, 3 Feb 2021 20:01:15 GMT, Anton Kozlov wrote: >> Please review the implementation of JEP 391: macOS/AArch64 Port. >> >> It's heavily based on existing ports to linux/aarch64, macos/x86_64, and >> windows/aarch64. >> >> Major changes are in: >> * src/hotspot/cpu/aarch64: support of the new calling convention (subtasks >> JDK-8253817, JDK-8253818) >> * src/hotspot/os_cpu/bsd_aarch64: copy of os_cpu/linux_aarch64 with >> necessary adjustments (JDK-8253819) >> * src/hotspot/share, test/hotspot/gtest: support of write-xor-execute (W^X), >> required on macOS/AArch64 platform. It's implemented with >> pthread_jit_write_protect_np provided by Apple. The W^X mode is local to a >> thread, so W^X mode change relates to the java thread state change (for java >> threads). In most cases, JVM executes in write-only mode, except when >> calling a generated stub like SafeFetch, which requires a temporary switch >> to execute-only mode. The same execute-only mode is enabled when a java >> thread executes in java or native states. This approach of managing W^X mode >> turned out to be simple and efficient enough. >> * src/jdk.hotspot.agent: serviceability agent implementation (JDK-8254941) > > Anton Kozlov has updated the pull request incrementally with six additional > commits since the last revision: > > - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos > - Add comments to WX transitions > >+ minor change of placements > - Use macro conditionals instead of empty functions > - Add W^X to tests > - Do not require known W^X state > - Revert w^x in gtests Marked as reviewed by ihse (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Tue, 2 Feb 2021 21:20:52 GMT, Daniel D. Daugherty wrote: >> Anton Kozlov has updated the pull request incrementally with one additional >> commit since the last revision: >> >> support macos_aarch64 in hsdis > > make/autoconf/flags.m4 line 140: > >> 138: else >> 139: MACOSX_VERSION_MIN=10.12.0 >> 140: fi > > Not something that needs to be addressed here, but these changes > illustrate that our collective use of macOSX/MACOSX/MacOSX names > are tied to the fact that the macOS major version number was at 10 > for a very long time. > > @magicus - Do we have an RFE to rename MACOSX or are we sticking > with it and evolving our interpretation of the 'X' from '10' to > */splat/asterik? @dcubed-ojdk There is no RFE to renaming "macosx" to "macos". I'm not sure it should be done. We can't follow all marketing trends (Apple recently renamed iOS to iPadOS for the iPad; we can't keep adapting to such schemes). Personally, I like the new name without the "x", but we had already spent some time trying to find and fix all (or at least, most) instances of "osx" in the code, that I don't really think it's worth the effort. If you can drill up enough enthusiasm for such a project, and get any objections down to minimum, I can help implementing it. But I won't be spearheading it. > make/common/NativeCompilation.gmk line 1178: > >> 1176: endif >> 1177: # This only works if the openjdk_codesign identity is >> present on the system. Let >> 1178: # silently fail otherwise. > > Might want to add a comment here: > # The '-f' option will replace an existing signature if one exists. We're not really in the habit of adding comments for various command line options. Normally, you can check these with "man" if you are uncertain. If they do something surprising, sure, but here it's more of a "it's needed on aarch64 to work at all", so I don't think a comment will be anything but added clutter. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v10]
On Thu, 4 Feb 2021 23:05:56 GMT, Gerard Ziemski wrote: >> Anton Kozlov has updated the pull request incrementally with six additional >> commits since the last revision: >> >> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos >> - Add comments to WX transitions >> >>+ minor change of placements >> - Use macro conditionals instead of empty functions >> - Add W^X to tests >> - Do not require known W^X state >> - Revert w^x in gtests > > I reviewed bsd_aarch64.cpp digging bit deeper and left some comments. > > Umm, so how does patching work? We don't even know if other threads are > > executing the code we need to patch. > > I thought java can handle that scenario in usual (non W^X systems) just fine, > so we just believe jvm did everything right and it's safe to rewrite some > code at specific moment. Got it, OK. - PR: https://git.openjdk.java.net/jdk/pull/2200
