Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v25]
On 12/03/2021 5:12 pm, Anton Kozlov wrote:
On Fri, 12 Mar 2021 05:24:10 GMT, David Holmes wrote:
Anton Kozlov has updated the pull request incrementally with one additional
commit since the last revision:
8262903: [macos_aarch64] Thread::current() called on detached thread
src/hotspot/share/runtime/safefetch.inline.hpp line 35:
33: inline int SafeFetch32(int* adr, int errValue) {
34: assert(StubRoutines::SafeFetch32_stub(), "stub not yet generated");
35: MACOS_AARCH64_ONLY(ThreadWXEnable wx(WXExec, Thread::current()));
I think you may have to use `Thread::current_or_null_safe()` here in case this
gets called from a signal handling context - see vmError.cpp testing for
TestSafeFetchInErrorHandler. Same possibly for SafeFetchN.
I'm not sure about expected behavior then. We may crash trying to execute the
generated code, since we may have no WXExec. If we switch to WXExec, we would
need to go back to a previous W^X state, but we don't know which one without
the thread.
The NULL check is only part of it. In a signal handling context
Thread::current() is not safe to call, you have to use
Thread::current_or_null_safe().
BTW, the test passes, probably that's why it didn't get attention. All
non-trivial actions in the current implementation of
`pd_hotspot_signal_handler`
(hhttps://github.com/openjdk/jdk/pull/2200/files#diff-9dcc5bcf908e2f8eb00b2c2837d667687a7540936d8f538ee1ea14e31ad50b40R215-R324)
assume non-NULL thread. So AFAICS, we should always have a thread when the
SafeFetch is called.
Okay but you still need to use Thread::current_or_null_safe().
Cheers,
David
Probably a fix to the https://bugs.openjdk.java.net/browse/JDK-8262903 could
just move ThreadWXEnable under the `if`. But now after
https://github.com/openjdk/jdk/pull/2200/commits/f6fb01b24f525e578692a1c6f2ff0a55b8233576is
ThreadWXEnable allows optional W^X state change, like `MutexLocker` allows
optional locking.
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Fri, 12 Mar 2021 01:01:09 GMT, Sergey Bylokhov wrote:
>> Ajit Ghaisas 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 47 additional
>> commits since the last revision:
>>
>> - Lanai PR#214 - 8263324 - avu
>> - Merge branch 'master' into 8260931_lanai_JEP_branch
>> - Lanai PR#213 - 8263325 - avu
>> - Lanai PR#212 - 8259825 - aghaisas
>> - Lanai PR#211 - 8262882 - aghaisas
>> - Merge branch 'master' into 8260931_lanai_JEP_branch
>> - Lanai PR#210 - 8263159 - jdv
>> - Lanai PR#209 - 8262936 - jdv
>> - Lanai PR#208 - 8262928 - jdv
>> - Lanai PR#207 - 8262750 - jdv
>> - ... and 37 more:
>> https://git.openjdk.java.net/jdk/compare/774f3477...369c3d21
>
> src/java.desktop/macosx/classes/sun/java2d/metal/MTLGraphicsConfig.java line
> 148:
>
>> 146: rq.lock();
>> 147: try {
>> 148: // getMTLConfigInfo() creates and destroys temporary
>
> Does it really create and destroy temporary surfaces/contexts?
No, it doesn't. I have updated JDK-8263363 to include this correction.
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v25]
On Fri, 12 Mar 2021 05:24:10 GMT, David Holmes wrote:
>> Anton Kozlov has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> 8262903: [macos_aarch64] Thread::current() called on detached thread
>
> src/hotspot/share/runtime/safefetch.inline.hpp line 35:
>
>> 33: inline int SafeFetch32(int* adr, int errValue) {
>> 34: assert(StubRoutines::SafeFetch32_stub(), "stub not yet generated");
>> 35: MACOS_AARCH64_ONLY(ThreadWXEnable wx(WXExec, Thread::current()));
>
> I think you may have to use `Thread::current_or_null_safe()` here in case
> this gets called from a signal handling context - see vmError.cpp testing for
> TestSafeFetchInErrorHandler. Same possibly for SafeFetchN.
I'm not sure about expected behavior then. We may crash trying to execute the
generated code, since we may have no WXExec. If we switch to WXExec, we would
need to go back to a previous W^X state, but we don't know which one without
the thread.
BTW, the test passes, probably that's why it didn't get attention. All
non-trivial actions in the current implementation of
`pd_hotspot_signal_handler`
(hhttps://github.com/openjdk/jdk/pull/2200/files#diff-9dcc5bcf908e2f8eb00b2c2837d667687a7540936d8f538ee1ea14e31ad50b40R215-R324)
assume non-NULL thread. So AFAICS, we should always have a thread when the
SafeFetch is called.
Probably a fix to the https://bugs.openjdk.java.net/browse/JDK-8262903 could
just move ThreadWXEnable under the `if`. But now after
https://github.com/openjdk/jdk/pull/2200/commits/f6fb01b24f525e578692a1c6f2ff0a55b8233576is
ThreadWXEnable allows optional W^X state change, like `MutexLocker` allows
optional locking.
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
> On 12-Mar-2021, at 10:30 AM, Sergey Bylokhov wrote: > > On Fri, 12 Mar 2021 00:09:54 GMT, Kevin Rushforth wrote: > >>> Ajit Ghaisas 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 47 additional >>> commits since the last revision: >>> >>> - Lanai PR#214 - 8263324 - avu >>> - Merge branch 'master' into 8260931_lanai_JEP_branch >>> - Lanai PR#213 - 8263325 - avu >>> - Lanai PR#212 - 8259825 - aghaisas >>> - Lanai PR#211 - 8262882 - aghaisas >>> - Merge branch 'master' into 8260931_lanai_JEP_branch >>> - Lanai PR#210 - 8263159 - jdv >>> - Lanai PR#209 - 8262936 - jdv >>> - Lanai PR#208 - 8262928 - jdv >>> - Lanai PR#207 - 8262750 - jdv >>> - ... and 37 more: >>> https://git.openjdk.java.net/jdk/compare/9d82be31...369c3d21 >> >> Latest changes look good. I confirmed that this PR has all of the content >> from the lanai repo. > >> By default we don?t do LCD anti-aliasing for text, only when we set Text >> Rendering Hint to use LCD antialiasing we take sub-pixel rendering path. > > And it actually produces LCD glyphs? not the grayscale? We get glyphs which take more than a byte to represent each pixel. Based on which we take LCD sub-pixel rendering path. > > - > > PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
> On 12-Mar-2021, at 9:29 AM, Scott Palmer wrote:
>
>
>> On Mar 11, 2021, at 9:53 PM, Sergey Bylokhov wrote:
>>
>> On Fri, 12 Mar 2021 02:29:04 GMT, Jayathirth D V wrote:
>>
src/java.desktop/macosx/classes/sun/java2d/metal/MTLSurfaceData.java line
323:
> 321: * more code just to support a few uncommon cases.
> 322: */
> 323: public boolean canRenderLCDText(SunGraphics2D sg2d) {
Just curious, can we render LCD on 10.14+ via metal? Does it work fine?
>>>
>>> Yes Sergey it works fine in 10.14+ systems via metal. Most of the JCK
>>> manual tests use LCD text on UI Components and it is recently verified in
>>> 10.14+ systems for EA10.
>>
>> Ok, for some reason I thought that the new macOS stopped providing LCD
>> glyphs.
>>
>
> Are the glyphs different or just the way they are rasterized?
>
> Newer versions of macOS don’t do LCD text and have gone back to plain
> grey-scale anti-aliasing.
> Java 2D should default to NOT doing LCD anti-aliasing for text on macOS if it
> wants to fit in with the look of native applications.
> (I’m not sure if that applies to non-retina displays.)
>
By default we don’t do LCD anti-aliasing for text, only when we set Text
Rendering Hint to use LCD antialiasing we take sub-pixel rendering path.
> Scott
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v25]
On Thu, 11 Mar 2021 14:07:43 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 one additional
> commit since the last revision:
>
> 8262903: [macos_aarch64] Thread::current() called on detached thread
src/hotspot/share/runtime/safefetch.inline.hpp line 35:
> 33: inline int SafeFetch32(int* adr, int errValue) {
> 34: assert(StubRoutines::SafeFetch32_stub(), "stub not yet generated");
> 35: MACOS_AARCH64_ONLY(ThreadWXEnable wx(WXExec, Thread::current()));
I think you may have to use `Thread::current_or_null_safe()` here in case this
gets called from a signal handling context - see vmError.cpp testing for
TestSafeFetchInErrorHandler. Same possibly for SafeFetchN.
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Fri, 12 Mar 2021 00:09:54 GMT, Kevin Rushforth wrote: >> Ajit Ghaisas 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 47 additional >> commits since the last revision: >> >> - Lanai PR#214 - 8263324 - avu >> - Merge branch 'master' into 8260931_lanai_JEP_branch >> - Lanai PR#213 - 8263325 - avu >> - Lanai PR#212 - 8259825 - aghaisas >> - Lanai PR#211 - 8262882 - aghaisas >> - Merge branch 'master' into 8260931_lanai_JEP_branch >> - Lanai PR#210 - 8263159 - jdv >> - Lanai PR#209 - 8262936 - jdv >> - Lanai PR#208 - 8262928 - jdv >> - Lanai PR#207 - 8262750 - jdv >> - ... and 37 more: >> https://git.openjdk.java.net/jdk/compare/9d82be31...369c3d21 > > Latest changes look good. I confirmed that this PR has all of the content > from the lanai repo. > By default we don?t do LCD anti-aliasing for text, only when we set Text > Rendering Hint to use LCD antialiasing we take sub-pixel rendering path. And it actually produces LCD glyphs? not the grayscale? - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Fri, 12 Mar 2021 01:04:22 GMT, Sergey Bylokhov wrote: >> Ajit Ghaisas 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 47 additional >> commits since the last revision: >> >> - Lanai PR#214 - 8263324 - avu >> - Merge branch 'master' into 8260931_lanai_JEP_branch >> - Lanai PR#213 - 8263325 - avu >> - Lanai PR#212 - 8259825 - aghaisas >> - Lanai PR#211 - 8262882 - aghaisas >> - Merge branch 'master' into 8260931_lanai_JEP_branch >> - Lanai PR#210 - 8263159 - jdv >> - Lanai PR#209 - 8262936 - jdv >> - Lanai PR#208 - 8262928 - jdv >> - Lanai PR#207 - 8262750 - jdv >> - ... and 37 more: >> https://git.openjdk.java.net/jdk/compare/23f9642d...369c3d21 > > src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLPaints.m line 813: > >> 811: initTemplatePipelineDescriptors(); >> 812: // This block is not reached in current implementation. >> 813: // Texture paint XOR mode rendering uses a tile based rendering >> using a SW pipe (similar to OGL) > > Do we have a bugid to implement this later? This is not exactly a TODO which we intend to implement later. Hence, there is no bugid. Metal does not support XOR mode rendering. I have kept this code as to know the code path to take in case we need to implement it in future. > src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLSurfaceDataBase.h > line 59: > >> 57: * The offset in pixels of the Metal viewport origin from the lower-left >> 58: * corner of the heavyweight drawable. For example, a top-level frame on >> 59: * Windows XP has lower-left insets of (4,4). The Metal viewport origin > > Do we really use these fields? "Windows XP"?? This will be fixed as a follow-on issue - JDK-8263486. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Fri, 12 Mar 2021 00:48:58 GMT, Sergey Bylokhov wrote:
>> Ajit Ghaisas 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 47 additional
>> commits since the last revision:
>>
>> - Lanai PR#214 - 8263324 - avu
>> - Merge branch 'master' into 8260931_lanai_JEP_branch
>> - Lanai PR#213 - 8263325 - avu
>> - Lanai PR#212 - 8259825 - aghaisas
>> - Lanai PR#211 - 8262882 - aghaisas
>> - Merge branch 'master' into 8260931_lanai_JEP_branch
>> - Lanai PR#210 - 8263159 - jdv
>> - Lanai PR#209 - 8262936 - jdv
>> - Lanai PR#208 - 8262928 - jdv
>> - Lanai PR#207 - 8262750 - jdv
>> - ... and 37 more:
>> https://git.openjdk.java.net/jdk/compare/aca5b193...369c3d21
>
> src/java.desktop/macosx/native/libawt_lwawt/awt/AWTSurfaceLayers.m line 73:
>
>> 71: // Updates back buffer size of the layer if it's an OpenGL/Metal layer
>> 72: // including all OpenGL/Metal sublayers
>> 73: + (void) repaintLayersRecursively:(CALayer*)aLayer {
>
> The operation of this code is still being investigated. @prsadhuk please add
> a bugid here.
JDK-8263485
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
> On Mar 11, 2021, at 9:53 PM, Sergey Bylokhov wrote:
>
> On Fri, 12 Mar 2021 02:29:04 GMT, Jayathirth D V wrote:
>
>>> src/java.desktop/macosx/classes/sun/java2d/metal/MTLSurfaceData.java line
>>> 323:
>>>
321: * more code just to support a few uncommon cases.
322: */
323: public boolean canRenderLCDText(SunGraphics2D sg2d) {
>>>
>>> Just curious, can we render LCD on 10.14+ via metal? Does it work fine?
>>
>> Yes Sergey it works fine in 10.14+ systems via metal. Most of the JCK manual
>> tests use LCD text on UI Components and it is recently verified in 10.14+
>> systems for EA10.
>
> Ok, for some reason I thought that the new macOS stopped providing LCD glyphs.
>
Are the glyphs different or just the way they are rasterized?
Newer versions of macOS don’t do LCD text and have gone back to plain
grey-scale anti-aliasing.
Java 2D should default to NOT doing LCD anti-aliasing for text on macOS if it
wants to fit in with the look of native applications.
(I’m not sure if that applies to non-retina displays.)
Scott
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Fri, 12 Mar 2021 02:29:04 GMT, Jayathirth D V wrote:
>> src/java.desktop/macosx/classes/sun/java2d/metal/MTLSurfaceData.java line
>> 323:
>>
>>> 321: * more code just to support a few uncommon cases.
>>> 322: */
>>> 323: public boolean canRenderLCDText(SunGraphics2D sg2d) {
>>
>> Just curious, can we render LCD on 10.14+ via metal? Does it work fine?
>
> Yes Sergey it works fine in 10.14+ systems via metal. Most of the JCK manual
> tests use LCD text on UI Components and it is recently verified in 10.14+
> systems for EA10.
Ok, for some reason I thought that the new macOS stopped providing LCD glyphs.
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Fri, 12 Mar 2021 00:42:35 GMT, Sergey Bylokhov wrote:
>> Ajit Ghaisas 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 47 additional
>> commits since the last revision:
>>
>> - Lanai PR#214 - 8263324 - avu
>> - Merge branch 'master' into 8260931_lanai_JEP_branch
>> - Lanai PR#213 - 8263325 - avu
>> - Lanai PR#212 - 8259825 - aghaisas
>> - Lanai PR#211 - 8262882 - aghaisas
>> - Merge branch 'master' into 8260931_lanai_JEP_branch
>> - Lanai PR#210 - 8263159 - jdv
>> - Lanai PR#209 - 8262936 - jdv
>> - Lanai PR#208 - 8262928 - jdv
>> - Lanai PR#207 - 8262750 - jdv
>> - ... and 37 more:
>> https://git.openjdk.java.net/jdk/compare/53b0bea8...369c3d21
>
> src/java.desktop/macosx/classes/sun/java2d/metal/MTLSurfaceData.java line 323:
>
>> 321: * more code just to support a few uncommon cases.
>> 322: */
>> 323: public boolean canRenderLCDText(SunGraphics2D sg2d) {
>
> Just curious, can we render LCD on 10.14+ via metal? Does it work fine?
Yes Sergey it works fine in 10.14+ systems via metal. Most of the JCK manual
tests use LCD text on UI Components and it is recently verified in 10.14+
systems for EA10.
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Thu, 11 Mar 2021 18:00:15 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)
>>
>> 3) Review comments (including some preliminary informal review comments) are
>> tracked with JBS issues - https://bugs.openjdk.java.net/issues/?filter=40598
>
> Ajit Ghaisas 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 47 additional
> commits since the last revision:
>
> - Lanai PR#214 - 8263324 - avu
> - Merge branch 'master' into 8260931_lanai_JEP_branch
> - Lanai PR#213 - 8263325 - avu
> - Lanai PR#212 - 8259825 - aghaisas
> - Lanai PR#211 - 8262882 - aghaisas
> - Merge branch 'master' into 8260931_lanai_JEP_branch
> - Lanai PR#210 - 8263159 - jdv
> - Lanai PR#209 - 8262936 - jdv
> - Lanai PR#208 - 8262928 - jdv
> - Lanai PR#207 - 8262750 - jdv
> - ... and 37 more:
> https://git.openjdk.java.net/jdk/compare/5e61bf57...369c3d21
src/java.desktop/macosx/classes/sun/java2d/metal/MTLSurfaceData.java line 323:
> 321: * more code just to support a few uncommon cases.
> 322: */
> 323: public boolean canRenderLCDText(SunGraphics2D sg2d) {
Just curious, can we render LCD on 10.14+ via metal? Does it work fine?
src/java.desktop/macosx/native/libawt_lwawt/awt/AWTSurfaceLayers.m line 73:
> 71: // Updates back buffer size of the layer if it's an OpenGL/Metal layer
> 72: // including all OpenGL/Metal sublayers
> 73: + (void) repaintLayersRecursively:(CALayer*)aLayer {
The operation of this code is still being investigated. @prsadhuk please add a
bugid here.
src/java.desktop/macosx/classes/sun/java2d/metal/MTLGraphicsConfig.java line
148:
> 146: rq.lock();
> 147: try {
> 148: // getMTLConfigInfo() creates and destroys temporary
Does it really create and destroy temporary surfaces/contexts?
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLPaints.m line 813:
> 811: initTemplatePipelineDescriptors();
> 812: // This block is not reached in current implementation.
> 813: // Texture paint XOR mode rendering uses a tile based rendering
> using a SW pipe (similar to OGL)
Do we have a bugid to implement this later?
src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLSurfaceDataBase.h
line 59:
> 57: * The offset in pixels of the Metal viewport origin from the lower-left
> 58: * corner of the heavyweight drawable. For example, a top-level frame on
> 59: * Windows XP has lower-left insets of (4,4). The Metal viewport origin
Do we really use these fields? "Windows XP"??
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Wed, 10 Mar 2021 11:43:43 GMT, Ajit Ghaisas wrote:
>> src/java.desktop/macosx/classes/sun/lwawt/macosx/CWarningWindow.java line
>> 304:
>>
>>> 302: };
>>> 303: }
>>> 304: public MTLLayer createMTLLayer() {
>>
>> TODO to test that this functionality works
>
> Testing was done under JDK-8261141.
JDK-8261630
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Tue, 9 Mar 2021 20:16:25 GMT, Alexey Ushakov wrote: >> src/java.desktop/macosx/classes/sun/java2d/metal/MTLBlitLoops.java line 499: >> >>> 497: } >>> 498: >>> 499: // We can convert argb_pre data from MTL surface in two places: >> >> Does anybody check that this is true for the metal pipeline? or It is just >> copied from the OGL? > > I fixed some conversion logic within JDK-8256331. OK, just to confirm. I wrote that text for OGL because it was the fastest way to transfer+convert the data from the video card to the memory. And as far as I understand the metal has the same limitation? It is not possible to convert it to some non-ARGB/Pre format on the fly while transferring the pixles? >> src/java.desktop/macosx/classes/sun/java2d/metal/MTLSurfaceData.java line >> 146: >> >>> 144: return MTLSurfaceRTT; >>> 145: default: >>> 146: return MTLSurface; >> >> Do we really support 3 different types of surface? I guess only two of them >> are different: textures used for manageable images, and surfaces used by the >> volatile image. > > It's true but I don't see any problem returning a more generic type as a > default here tbd - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8252833: Correct "no comment" warnings from javadoc in java.smartcardio module
On Thu, 11 Mar 2021 14:53:22 GMT, Roger Riggs wrote: >> Disable the "missing" target for java.smartcardio from doclint. > > Please assign a new bug and title to ignore/suppress the warnings. > The original issue 8252833 should be left open. > Thanks Closing, as it's unclear which approach should be taken. - PR: https://git.openjdk.java.net/jdk/pull/2930
Withdrawn: 8252833: Correct "no comment" warnings from javadoc in java.smartcardio module
On Thu, 11 Mar 2021 01:13:12 GMT, Bradford Wetmore wrote: > Disable the "missing" target for java.smartcardio from doclint. This pull request has been closed without being integrated. - PR: https://git.openjdk.java.net/jdk/pull/2930
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Thu, 11 Mar 2021 18:00:15 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) >> >> 3) Review comments (including some preliminary informal review comments) are >> tracked with JBS issues - https://bugs.openjdk.java.net/issues/?filter=40598 > > Ajit Ghaisas 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 47 additional > commits since the last revision: > > - Lanai PR#214 - 8263324 - avu > - Merge branch 'master' into 8260931_lanai_JEP_branch > - Lanai PR#213 - 8263325 - avu > - Lanai PR#212 - 8259825 - aghaisas > - Lanai PR#211 - 8262882 - aghaisas > - Merge branch 'master' into 8260931_lanai_JEP_branch > - Lanai PR#210 - 8263159 - jdv > - Lanai PR#209 - 8262936 - jdv > - Lanai PR#208 - 8262928 - jdv > - Lanai PR#207 - 8262750 - jdv > - ... and 37 more: > https://git.openjdk.java.net/jdk/compare/204b2bcc...369c3d21 Latest changes look good. I confirmed that this PR has all of the content from the lanai repo. - Marked as reviewed by kcr (Author). PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v12]
On Tue, 9 Mar 2021 17:55:12 GMT, Anton Kozlov wrote:
>> src/hotspot/share/runtime/thread.cpp line 2515:
>>
>>> 2513: void JavaThread::check_special_condition_for_native_trans(JavaThread
>>> *thread) {
>>> 2514: // Enable WXWrite: called directly from interpreter native wrapper.
>>> 2515: MACOS_AARCH64_ONLY(ThreadWXEnable wx(WXWrite, thread));
>>
>> FWIW, I personally think that adding these MACOS_AARCH64_ONLY usages at the
>> call sites increase the line-noise in the affected functions. I think I
>> would have preferred a version:
>> ThreadWXEnable(WXMode new_mode, Thread* thread = NULL) {
>> MACOS_AARCH64_ONLY(initialize(new_mode, thread);) {}
>> void initialize(...); // Implementation in thread_bsd_aarch64.cpp (alt.
>> inline.hpp)
>> With that said, I'm fine with taking this discussion as a follow-up.
>
> The former version used no such macros. I like that now it's clear the W^X
> management is relevant to macos/aarch64 only. I see the point to move the
> pre-processor condition into the class implementation. But I think it will
> bring a bit of inconsistency, as the rest of W^X implementation is explicitly
> guarded by preprocessor conditionals. I've also tried to push macro
> conditionals as far as possible down to implementation, providing a kind of
> generalized W^X interface. That required a few artificial decisions, e.g. how
> would we call the mode we execute on the rest of platforms with write and
> execute allowed, WXWriteExec?.. I abandoned that attempt.
I think we would use the same names, but I haven't given it more thought. I
might take a look at this after this has been integrated.
>> src/hotspot/share/runtime/thread.hpp line 848:
>>
>>> 846: void init_wx();
>>> 847: WXMode enable_wx(WXMode new_state);
>>> 848: #endif // __APPLE__ && AARCH64
>>
>> Now that this is only compiled into macOS/AArch64, could this be moved over
>> to thread_bsd_aarch64.hpp? The same goes for the associated functions.
>
> The thread_bsd_aarch64.hpp describes a part of JavaThread, while this block
> belongs to Thread for now. Since W^X is an attribute of any operating system
> thread, I assumed Thread to be the right place for W^X bookkeeping.
>
> In most cases, we manage W^X state of JavaThread. But sometimes a GC thread
> needs the WXWrite state, or safefetch is called from non-JavaThread. Probably
> this can be dealt with (e.g. GCThread to always have the WXWrite state). But
> such change would be much more than a simple refactoring and it would require
> a significant amount of testing. Ideally, I would like to investigate this as
> a follow-up change, or at least after other fixes to this PR.
Good point about Thread vs JavaThread. Yes, this can be looked into as
follow-up cleanups.
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v25]
On Thu, 11 Mar 2021 14:07:43 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 one additional > commit since the last revision: > > 8262903: [macos_aarch64] Thread::current() called on detached thread Marked as reviewed by stefank (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Thu, 11 Mar 2021 18:00:15 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) >> >> 3) Review comments (including some preliminary informal review comments) are >> tracked with JBS issues - https://bugs.openjdk.java.net/issues/?filter=40598 > > Ajit Ghaisas 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 47 additional > commits since the last revision: > > - Lanai PR#214 - 8263324 - avu > - Merge branch 'master' into 8260931_lanai_JEP_branch > - Lanai PR#213 - 8263325 - avu > - Lanai PR#212 - 8259825 - aghaisas > - Lanai PR#211 - 8262882 - aghaisas > - Merge branch 'master' into 8260931_lanai_JEP_branch > - Lanai PR#210 - 8263159 - jdv > - Lanai PR#209 - 8262936 - jdv > - Lanai PR#208 - 8262928 - jdv > - Lanai PR#207 - 8262750 - jdv > - ... and 37 more: > https://git.openjdk.java.net/jdk/compare/ada1a022...369c3d21 Marked as reviewed by prr (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2403
Integrated: 8263465: JDK-8236847 causes tier1 build failure on linux-aarch64
On Thu, 11 Mar 2021 18:41:10 GMT, Yumin Qi wrote: > Hi, Please review > > JDK-8236847 changes failed on build linux-aarch64 on xcross build. The > reason is we check BUILD_CDS_ARCHIVE which is not correct in such case. We > should check ENABLE_CDS instead. > > Thanks > Yumin This pull request has now been integrated. Changeset: 15daccac Author:Yumin Qi URL: https://git.openjdk.java.net/jdk/commit/15daccac Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8263465: JDK-8236847 causes tier1 build failure on linux-aarch64 Reviewed-by: iklam, erikj, dcubed - PR: https://git.openjdk.java.net/jdk/pull/2946
Re: RFR: 8263465: JDK-8236847 causes tier1 build failure on linux-aarch64
On Thu, 11 Mar 2021 18:42:53 GMT, Ioi Lam wrote: >> Hi, Please review >> >> JDK-8236847 changes failed on build linux-aarch64 on xcross build. The >> reason is we check BUILD_CDS_ARCHIVE which is not correct in such case. We >> should check ENABLE_CDS instead. >> >> Thanks >> Yumin > > LGTM Thanks to @iklam @erikj79 @dcubed-ojdk ! - PR: https://git.openjdk.java.net/jdk/pull/2946
Re: RFR: 8263465: JDK-8236847 causes tier1 build failure on linux-aarch64
On Thu, 11 Mar 2021 18:41:10 GMT, Yumin Qi wrote: > Hi, Please review > > JDK-8236847 changes failed on build linux-aarch64 on xcross build. The > reason is we check BUILD_CDS_ARCHIVE which is not correct in such case. We > should check ENABLE_CDS instead. > > Thanks > Yumin Looks good to me. This change can be integrated until the trivial fix rules. - Marked as reviewed by dcubed (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2946
Re: RFR: 8263465: JDK-8236847 causes tier1 build failure on linux-aarch64
On Thu, 11 Mar 2021 18:41:10 GMT, Yumin Qi wrote: > Hi, Please review > > JDK-8236847 changes failed on build linux-aarch64 on xcross build. The > reason is we check BUILD_CDS_ARCHIVE which is not correct in such case. We > should check ENABLE_CDS instead. > > Thanks > Yumin Marked as reviewed by erikj (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2946
RFR: 8263465: JDK-8236847 causes tier1 build failure on linux-aarch64
Hi, Please review JDK-8236847 changes failed on build linux-aarch64 on xcross build. The reason is we check BUILD_CDS_ARCHIVE which is not correct in such case. We should check ENABLE_CDS instead. Thanks Yumin - Commit messages: - 8263465: JDK-8236847 causes tier1 build failure on linux-aarch64 Changes: https://git.openjdk.java.net/jdk/pull/2946/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2946=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8263465 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2946.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2946/head:pull/2946 PR: https://git.openjdk.java.net/jdk/pull/2946
Re: RFR: 8263465: JDK-8236847 causes tier1 build failure on linux-aarch64
On Thu, 11 Mar 2021 18:41:10 GMT, Yumin Qi wrote: > Hi, Please review > > JDK-8236847 changes failed on build linux-aarch64 on xcross build. The > reason is we check BUILD_CDS_ARCHIVE which is not correct in such case. We > should check ENABLE_CDS instead. > > Thanks > Yumin LGTM - Marked as reviewed by iklam (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2946
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v12]
On Thu, 11 Mar 2021 07:40:47 GMT, Alexey Ushakov wrote: >> Ajit Ghaisas 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 45 additional >> commits since the last revision: >> >> - Lanai PR#213 - 8263325 - avu >> - Lanai PR#212 - 8259825 - aghaisas >> - Lanai PR#211 - 8262882 - aghaisas >> - Merge branch 'master' into 8260931_lanai_JEP_branch >> - Lanai PR#210 - 8263159 - jdv >> - Lanai PR#209 - 8262936 - jdv >> - Lanai PR#208 - 8262928 - jdv >> - Lanai PR#207 - 8262750 - jdv >> - Merge branch 'master' into 8260931_lanai_JEP_branch >> - Lanai PR#206 - 8262729 - aghaisas >> - ... and 35 more: >> https://git.openjdk.java.net/jdk/compare/d1e5ff8f...3980ecbd > > Marked as reviewed by avu (no project role). We have addressed the review comments (except for a few minor follow-on issues) This version of the PR is the one which we are planning to integrate into the JDK. Please review and approve. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
> **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) > > 3) Review comments (including some preliminary informal review comments) are > tracked with JBS issues - https://bugs.openjdk.java.net/issues/?filter=40598 Ajit Ghaisas 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 47 additional commits since the last revision: - Lanai PR#214 - 8263324 - avu - Merge branch 'master' into 8260931_lanai_JEP_branch - Lanai PR#213 - 8263325 - avu - Lanai PR#212 - 8259825 - aghaisas - Lanai PR#211 - 8262882 - aghaisas - Merge branch 'master' into 8260931_lanai_JEP_branch - Lanai PR#210 - 8263159 - jdv - Lanai PR#209 - 8262936 - jdv - Lanai PR#208 - 8262928 - jdv - Lanai PR#207 - 8262750 - jdv - ... and 37 more: https://git.openjdk.java.net/jdk/compare/1c8fee73...369c3d21 - Changes: - all: https://git.openjdk.java.net/jdk/pull/2403/files - new: https://git.openjdk.java.net/jdk/pull/2403/files/3980ecbd..369c3d21 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2403=12 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2403=11-12 Stats: 2420 lines in 128 files changed: 1246 ins; 368 del; 806 mod Patch: https://git.openjdk.java.net/jdk/pull/2403.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2403/head:pull/2403 PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v12]
On Mon, 8 Feb 2021 18:47:09 GMT, Sergey Bylokhov wrote:
>> 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?
>
> I suggest fixing this before the integration.
Fixed with JDK-8259825.
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Tue, 9 Mar 2021 23:15:57 GMT, Sergey Bylokhov wrote: >> Also, MTLSD_WINDOW is supported > > How it is used? Can we really draw to the window directly? It looks like it > copied from the OGL where it is unused since CLayer integration. Something is > inconsistent here. Here is a follow-up issue JDK-8263463 - PR: https://git.openjdk.java.net/jdk/pull/2403
Integrated: 8236847: CDS archive with 4K alignment unusable on machines with 64k pages
On Fri, 19 Feb 2021 18:15:45 GMT, Yumin Qi wrote: > Hi, Please review > Usually most OSes are configured with page size of 4K, but some others are > configured with 64K. If jdk binary is built on 4K platform and run on > different configured platforms, CDS fails to be loaded due to region > alignment mismatch: > Unable to map CDS archive -- os::vm_allocation_granularity() expected: 4096 > actual: 65536 > This change uses 64K as region alignment if OS page size is less than 64K. > For most of the current OSes, means always use 64K as file map region > alignment. >The archive size will increase about 300K due to the change. >Tests: tier1-4 > Run MacOS/X64 binary on MacOS/aarch64 > >Thanks >Yumin This pull request has now been integrated. Changeset: 3820ab9e Author:Yumin Qi URL: https://git.openjdk.java.net/jdk/commit/3820ab9e Stats: 222 lines in 16 files changed: 163 ins; 16 del; 43 mod 8236847: CDS archive with 4K alignment unusable on machines with 64k pages Reviewed-by: iklam, stuefe, erikj, ihse - PR: https://git.openjdk.java.net/jdk/pull/2651
Re: RFR: 8236847: CDS archive with 4K alignment unusable on machines with 64k pages [v7]
On Wed, 10 Mar 2021 18:33:53 GMT, Thomas Stuefe wrote: >> Yumin Qi has updated the pull request with a new target base due to a merge >> or a rebase. The pull request now contains seven commits: >> >> - Merge branch 'master' into jdk-8236847 >> - Merge master >> - Add --enable-compatible-cds-alignment for linux-aarch64 and macosx-x64 in >> jib job >> - Switch to enble compatible-cds-alignment at configuration >> - Make CDS core region alignment configurable at build time >> - Make 64K core region alignment only for specific platforms, also fixed >> comments as suggestions. >> - 8236847: CDS archive with 4K alignment unusable on machines with 64k pages > > src/hotspot/share/memory/metaspaceShared.cpp line 125: > >> 123: // os::vm_allocation_granularity() is usually 4K for most OSes. >> However, on Linux/aarch64, >> 124: // it can be either 4K or 64K and on Macosx-arm it is 16K. To generate >> archives that are >> 125: // compatible for both settings. An alternative cds core region >> alignment can be enabled > > dot -> comma and lowercase Thanks. Fixed. > src/hotspot/os_cpu/bsd_x86/os_bsd_x86.hpp line 30: > >> 28: #if defined(COMPATIBLE_CDS_ALIGNMENT) >> 29: #define CDS_CORE_REGION_ALIGNMENT (16*K) >> 30: #endif > > Could you limit this to `_APPLE_`? I don't think this affects the BSDs. > > (We really should separate BSD and MacOS sources at some point.) > > Also maybe a comment like this: "Core region alignment is 16K to be able to > run binaries built on MacOS x64 on MacOS aarch64." ? Fixed. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/2651
Re: RFR: 8252833: Correct "no comment" warnings from javadoc in java.smartcardio module
On Thu, 11 Mar 2021 01:13:12 GMT, Bradford Wetmore wrote: > Disable the "missing" target for java.smartcardio from doclint. Please assign a new bug and title to ignore/suppress the warnings. The original issue 8252833 should be left open. Thanks - Changes requested by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2930
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v23]
On Wed, 3 Mar 2021 15:57:13 GMT, Gerard Ziemski wrote: >> src/hotspot/os_cpu/bsd_aarch64/os_bsd_aarch64.cpp line 207: >> >>> 205: // Enable WXWrite: this function is called by the signal handler at >>> arbitrary >>> 206: // point of execution. >>> 207: ThreadWXEnable wx(WXWrite, thread); >> >> Note that `thread` can be NULL here if the signal handler is running in a >> non-attached thread. If we then perform: >> `ThreadWXEnable(WXMode new_mode, Thread* thread = NULL) : >> _thread(thread ? thread : Thread::current()),` >> we call Thread::current() on a non-attached thread and that will >> assert/crash if we get NULL. Either avoid using WX when the thread is NULL, >> or else change to use Thread::current_or_null_safe() and ensure all uses >> have a NULL check. > >> Note that `thread` can be NULL here if the signal handler is running in a >> non-attached thread. If we then perform: >> `ThreadWXEnable(WXMode new_mode, Thread* thread = NULL) : _thread(thread ? >> thread : Thread::current()),` >> we call Thread::current() on a non-attached thread and that will >> assert/crash if we get NULL. Either avoid using WX when the thread is NULL, >> or else change to use Thread::current_or_null_safe() and ensure all uses >> have a NULL check. > > https://bugs.openjdk.java.net/browse/JDK-8262903 tracks this issue. Thanks for report and analysis! I fixed this in https://github.com/openjdk/jdk/pull/2200/commits/f6fb01b24f525e578692a1c6f2ff0a55b8233576 - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v25]
> 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: 8262903: [macos_aarch64] Thread::current() called on detached thread - Changes: - all: https://git.openjdk.java.net/jdk/pull/2200/files - new: https://git.openjdk.java.net/jdk/pull/2200/files/a72f6834..f6fb01b2 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2200=24 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2200=23-24 Stats: 13 lines in 5 files changed: 3 ins; 0 del; 10 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: RFR: 8252833: Correct "no comment" warnings from javadoc in java.smartcardio module
On Thu, 11 Mar 2021 01:13:12 GMT, Bradford Wetmore wrote: > Disable the "missing" target for java.smartcardio from doclint. Revoking approval. - Changes requested by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2930
Re: RFR: 8252833: Correct "no comment" warnings from javadoc in java.smartcardio module
On Thu, 11 Mar 2021 09:25:34 GMT, Magnus Ihse Bursie wrote: >> Disable the "missing" target for java.smartcardio from doclint. > > Looks good to me. ... however, this fix does not achieve what the bug report is asking for. The patch here will modify flags to javac. The JBS issue talks about flags for javadoc. - PR: https://git.openjdk.java.net/jdk/pull/2930
Re: RFR: 8252833: Correct "no comment" warnings from javadoc in java.smartcardio module
On Thu, 11 Mar 2021 01:13:12 GMT, Bradford Wetmore wrote: > Disable the "missing" target for java.smartcardio from doclint. Looks good to me. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2930
