Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Tue, 9 Mar 2021 22:11:20 GMT, Alexey Ushakov wrote: >> src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLSurfaceDataBase.h >> line 109: >> >>> 107: #define MTLSD_TEXTURE sun_java2d_pipe_hw_AccelSurface_TEXTURE >>> 108: #define MTLSD_FLIP_BACKBUFFER >>> sun_java2d_pipe_hw_AccelSurface_FLIP_BACKBUFFER >>> 109: #define MTLSD_RT_TEXTURE >>> sun_java2d_pipe_hw_AccelSurface_RT_TEXTURE >> >> Only two of them are supported? MTLSD_TEXTURE and MTLSD_RT_TEXTURE? > > 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. - 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:04:17 GMT, Alexey Ushakov wrote: >> Probably it's enough to set the context in SET_SURFACES, I'll double-check >> it. > > Looks like it works even without SET_SCRATCH_SURFACE (I did some limited > testing) but I don't think we should do such a huge change at this moment. JDK-8263309 - 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:13:03 GMT, Sergey Bylokhov wrote: >> src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLBlitLoops.m line >> 676: >> >>> 674: height = srcInfo.bounds.y2 - srcInfo.bounds.y1; >>> 675: >>> 676: pDst = PtrAddBytes(pDst, dstx * dstInfo.pixelStride); >> >> Here and in other places use the PtrPixelsRow instead, do not use >> multiplication, see the history of the OGLBlitLoops_SurfaceToSwBlit method. > > I think this at least should be investigated before integration. JDK-8263324 - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v11]
On Tue, 9 Mar 2021 19:52:35 GMT, Phil Race 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 41 additional >> commits since the last revision: >> >> - 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 >> - Lanai PR#205 - 8262496 - avu >> - Lanai PR#203 - 8262313 - jdv >> - Lanai PR#202 - 8262293 - avu >> - Lanai PR#201 - 8261891 - avu >> - ... and 31 more: >> https://git.openjdk.java.net/jdk/compare/1bd8aea0...de456939 > > test/jdk/performance/client/RenderPerfTest/build.xml line 72: > >> 70: >> 71: >> 72: > > This was presumably borrowed from J2DBench so this comment should be fixed ! JDK-8263325 - 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 17:59:26 GMT, Alexey Ushakov wrote: >> In fact, we don't have any scratch surfaces. SET_SCRATCH_SURFACE effectively >> sets the new context. For better readability, we should add the new op >> SET_CONTEXT in BufferedOpCodes > > Probably it's enough to set the context in SET_SURFACES, I'll double-check it. Looks like it works even without SET_SCRATCH_SURFACE (I did some limited testing) but I don't think we should do such a huge change at this moment. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Sun, 7 Mar 2021 22:31:16 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 36 additional >> commits since the last revision: >> >> - Lanai PR#206 - 8262729 - aghaisas >> - Lanai PR#205 - 8262496 - avu >> - Lanai PR#203 - 8262313 - jdv >> - Lanai PR#202 - 8262293 - avu >> - Lanai PR#201 - 8261891 - avu >> - Lanai PR#200 - 8262115 - aghaisas >> - Merge branch 'master' into 8260931_lanai_JEP_branch >> - Lanai PR#199 - 8262091 - aghaisas >> - Lanai PR#198 - 8261646 - avu >> - Lanai PR#197 - 8261960 - jdv >> - ... and 26 more: >> https://git.openjdk.java.net/jdk/compare/2f09d382...5cb1fd91 > > src/java.desktop/macosx/classes/sun/java2d/metal/MTLLayer.java line 44: > >> 42: >> 43: // Pass the insets to native code to make adjustments in blitTexture >> 44: private static native void nativeSetInsets(long layerPtr, int top, >> int left); > > Probably I have asked this already, but why we need to use insets? Why insets > is not necessary for ogl? We have different coordinate systems for OGL and Metal. Metal (0,0) for textures is the top-left corner whereas OGL (0,0) is the bottom-left one, so we need to have insets to perform blitting to drawable correctly. > 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. > src/java.desktop/macosx/classes/sun/java2d/metal/MTLGraphicsConfig.java line > 168: > >> 166: } >> 167: >> 168: ContextCapabilities caps = new MTLContext.MTLContextCaps( > > CAPS_DOUBLEBUFFERED is not included? JDK-8263306 > 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 > src/java.desktop/macosx/classes/sun/java2d/metal/MTLSurfaceData.java line 221: > >> 219: protected native boolean initRTexture(long pData, boolean isOpaque, >> int width, int height); >> 220: >> 221: protected native boolean initFlipBackbuffer(long pData); > > flip back buffer is supported? No, it should be removed (JDK-8263312) > src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLSurfaceData.m > line 284: > >> 282: */ >> 283: jboolean >> 284: MTLSD_InitMTLWindow(JNIEnv *env, BMTLSDOps *bmtlsdo) > > When the MTLWindow is used? MTLContext setSurfacesEnv > src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLSurfaceDataBase.h > line 109: > >> 107: #define MTLSD_TEXTURE sun_java2d_pipe_hw_AccelSurface_TEXTURE >> 108: #define MTLSD_FLIP_BACKBUFFER >> sun_java2d_pipe_hw_AccelSurface_FLIP_BACKBUFFER >> 109: #define MTLSD_RT_TEXTURE >> sun_java2d_pipe_hw_AccelSurface_RT_TEXTURE > > Only two of them are supported? MTLSD_TEXTURE and MTLSD_RT_TEXTURE? Also, MTLSD_WINDOW is supported - 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 17:26:24 GMT, Alexey Ushakov wrote:
>> src/java.desktop/macosx/classes/sun/java2d/metal/MTLContext.java line 68:
>>
>>> 66: * when disposing a texture-based surface).
>>> 67: */
>>> 68: public static void setScratchSurface(long pConfigInfo) {
>>
>> How the scratch surface is used in the metal pipeline? Why it is not enough
>> to set the "context current"?
>
> In fact, we don't have any scratch surfaces. SET_SCRATCH_SURFACE effectively
> sets the new context. For better readability, we should add the new op
> SET_CONTEXT in BufferedOpCodes
Probably it's enough to set the context in SET_SURFACES, I'll double-check it.
>> src/java.desktop/macosx/classes/sun/java2d/metal/MTLContext.java line 140:
>>
>>> 138: StringBuilder sb = new StringBuilder(super.toString());
>>> 139: if ((caps & CAPS_DOUBLEBUFFERED) != 0) {
>>> 140: sb.append("CAPS_DOUBLEBUFFERED|");
>>
>> Related to other questions, we do not include CAPS_DOUBLEBUFFERED, but
>> anyway, report the surface as double buffered.
>
> We can remove all the unused constants
JDK-8263306
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Mon, 1 Mar 2021 11:17:39 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 36 additional > commits since the last revision: > > - Lanai PR#206 - 8262729 - aghaisas > - Lanai PR#205 - 8262496 - avu > - Lanai PR#203 - 8262313 - jdv > - Lanai PR#202 - 8262293 - avu > - Lanai PR#201 - 8261891 - avu > - Lanai PR#200 - 8262115 - aghaisas > - Merge branch 'master' into 8260931_lanai_JEP_branch > - Lanai PR#199 - 8262091 - aghaisas > - Lanai PR#198 - 8261646 - avu > - Lanai PR#197 - 8261960 - jdv > - ... and 26 more: > https://git.openjdk.java.net/jdk/compare/8f9013c3...5cb1fd91 Looks good, but a couple of things should be fixed (JDK-8263325, JDK-8263324) - Changes requested by avu (no project role). PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Sun, 7 Mar 2021 22:48:47 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 36 additional >> commits since the last revision: >> >> - Lanai PR#206 - 8262729 - aghaisas >> - Lanai PR#205 - 8262496 - avu >> - Lanai PR#203 - 8262313 - jdv >> - Lanai PR#202 - 8262293 - avu >> - Lanai PR#201 - 8261891 - avu >> - Lanai PR#200 - 8262115 - aghaisas >> - Merge branch 'master' into 8260931_lanai_JEP_branch >> - Lanai PR#199 - 8262091 - aghaisas >> - Lanai PR#198 - 8261646 - avu >> - Lanai PR#197 - 8261960 - jdv >> - ... and 26 more: >> https://git.openjdk.java.net/jdk/compare/af4701dd...5cb1fd91 > > src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLBlitLoops.m line > 676: > >> 674: height = srcInfo.bounds.y2 - srcInfo.bounds.y1; >> 675: >> 676: pDst = PtrAddBytes(pDst, dstx * dstInfo.pixelStride); > > Here and in other places use the PtrPixelsRow instead, do not use > multiplication, see the history of the OGLBlitLoops_SurfaceToSwBlit method. I think this at least should be investigated before integration. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v11]
On Mon, 8 Mar 2021 08:06:03 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 41 additional > commits since the last revision: > > - 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 > - Lanai PR#205 - 8262496 - avu > - Lanai PR#203 - 8262313 - jdv > - Lanai PR#202 - 8262293 - avu > - Lanai PR#201 - 8261891 - avu > - ... and 31 more: > https://git.openjdk.java.net/jdk/compare/851d3def...de456939 src/java.desktop/macosx/native/libawt_lwawt/java2d/metal/MTLGraphicsConfig.m line 185: > 183: > 184: NSRect contentRect = NSMakeRect(0, 0, 64, 64); > 185: NSWindow *window = Some remnant from the scratch surface initialization? - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v8]
On Mon, 15 Feb 2021 20:55:13 GMT, Phil Race wrote: >> Marked as reviewed by gziemski (Committer). > >> > > Thanks @gerard-ziemski for taking a detailed look at this. >> > > We do have an open bug to address this. Please refer >> > > [JDK-8259825](https://bugs.openjdk.java.net/browse/JDK-8259825). >> > >> > >> > Hi Gerard, expecting a process and parsing its output is definitely not >> > ideal and that's why there's this open bug. >> > One thing that is under consideration is to equate >= 10.14 with Metal is >> > available since as of 10.14 macOS won't install if a system does not >> > support metal. We have no compelling reason to support metal on earlier >> > releases anyway .. those are either already unsupported or barely >> > supported and OGL will always be available there. >> >> I did not know that there already was a bug covering this issue. >> >> Your idea seems reasonable. >> >> More than just focusing on this immediate issue, however, I was hoping to >> raise the point of us starting adopting profiling tools to analyze our code >> (memory utilization, leaks, cpu/gpu profiling etc). A new feature that >> brings 10% benefit, but uses 50% more resources for example would probably >> not be a good investment. And to figure that we need to relay on some good >> tools and Xcode provides some. > > There were actually tasks to do profiling of the memory footprint and look > for leaks. We did not think it possible to be able to assert "Metal must use > less memory than OpenGL" or dig into tiny differences but it was intended to > find the big issues. See https://bugs.openjdk.java.net/browse/JDK-8237856 > @prsadhuk maybe able to say how much of it was doing using Xcode. Regarding this comment from @mrserb > Probably place it near J2Dbench? I can't find the context but if it is suggesting moving RenderPerfTest to be co-located with J2Bench let's NOT do that. It is much more likely that J2DBench will be moved out of demo into test ... - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v11]
On Mon, 8 Mar 2021 08:06:03 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 41 additional > commits since the last revision: > > - 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 > - Lanai PR#205 - 8262496 - avu > - Lanai PR#203 - 8262313 - jdv > - Lanai PR#202 - 8262293 - avu > - Lanai PR#201 - 8261891 - avu > - ... and 31 more: > https://git.openjdk.java.net/jdk/compare/32b236e4...de456939 test/jdk/performance/client/RenderPerfTest/build.xml line 72: > 70: > 71: > 72: This was presumably borrowed from J2DBench so this comment should be fixed ! - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v21]
On Mon, 1 Mar 2021 10:31:19 GMT, Andrew Haley wrote: >> Anton Kozlov has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Merge remote-tracking branch 'origin/jdk/jdk-macos' into jdk-macos >> - Minor fixes > > src/hotspot/cpu/aarch64/globalDefinitions_aarch64.hpp line 62: > >> 60: >> 61: #if defined(__APPLE__) || defined(_WIN64) >> 62: #define R18_RESERVED > > #define R18_RESERVED true``` We always check for `R18_RESERVED` with `#if(n)def`, is there any reason to define the value for the macro? - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v12]
On Tue, 9 Feb 2021 09:23:50 GMT, Stefan Karlsson wrote:
>> Anton Kozlov has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Update signal handler part for debugger
>
> 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.
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8236847: CDS archive with 4K alignment unusable on machines with 64k pages [v6]
On Mon, 1 Mar 2021 17:08:02 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 > > Yumin Qi has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains six commits: > > - 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 Build changes look good. - Marked as reviewed by ihse (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2651
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Sun, 7 Mar 2021 22:29:50 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 36 additional
>> commits since the last revision:
>>
>> - Lanai PR#206 - 8262729 - aghaisas
>> - Lanai PR#205 - 8262496 - avu
>> - Lanai PR#203 - 8262313 - jdv
>> - Lanai PR#202 - 8262293 - avu
>> - Lanai PR#201 - 8261891 - avu
>> - Lanai PR#200 - 8262115 - aghaisas
>> - Merge branch 'master' into 8260931_lanai_JEP_branch
>> - Lanai PR#199 - 8262091 - aghaisas
>> - Lanai PR#198 - 8261646 - avu
>> - Lanai PR#197 - 8261960 - jdv
>> - ... and 26 more:
>> https://git.openjdk.java.net/jdk/compare/69ed64f1...5cb1fd91
>
> src/java.desktop/macosx/classes/sun/java2d/metal/MTLContext.java line 140:
>
>> 138: StringBuilder sb = new StringBuilder(super.toString());
>> 139: if ((caps & CAPS_DOUBLEBUFFERED) != 0) {
>> 140: sb.append("CAPS_DOUBLEBUFFERED|");
>
> Related to other questions, we do not include CAPS_DOUBLEBUFFERED, but
> anyway, report the surface as double buffered.
We can remove all the unused constants
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v10]
On Sun, 7 Mar 2021 22:24:54 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 36 additional
>> commits since the last revision:
>>
>> - Lanai PR#206 - 8262729 - aghaisas
>> - Lanai PR#205 - 8262496 - avu
>> - Lanai PR#203 - 8262313 - jdv
>> - Lanai PR#202 - 8262293 - avu
>> - Lanai PR#201 - 8261891 - avu
>> - Lanai PR#200 - 8262115 - aghaisas
>> - Merge branch 'master' into 8260931_lanai_JEP_branch
>> - Lanai PR#199 - 8262091 - aghaisas
>> - Lanai PR#198 - 8261646 - avu
>> - Lanai PR#197 - 8261960 - jdv
>> - ... and 26 more:
>> https://git.openjdk.java.net/jdk/compare/ffb3d48c...5cb1fd91
>
> src/java.desktop/macosx/classes/sun/java2d/metal/MTLContext.java line 68:
>
>> 66: * when disposing a texture-based surface).
>> 67: */
>> 68: public static void setScratchSurface(long pConfigInfo) {
>
> How the scratch surface is used in the metal pipeline? Why it is not enough
> to set the "context current"?
In fact, we don't have any scratch surfaces. SET_SCRATCH_SURFACE effectively
sets the new context. For better readability, we should add the new op
SET_CONTEXT in BufferedOpCodes
-
PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v12]
On Tue, 9 Feb 2021 09:12:13 GMT, Stefan Karlsson wrote: >> Anton Kozlov has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Update signal handler part for debugger > > 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. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v12]
On Tue, 9 Feb 2021 09:06:26 GMT, Stefan Karlsson wrote: >> Anton Kozlov has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Update signal handler part for debugger > > src/hotspot/share/runtime/threadWXSetters.hpp line 28: > >> 26: #define SHARE_RUNTIME_THREADWXSETTERS_HPP >> 27: >> 28: #include "runtime/thread.inline.hpp" > > This breaks against our convention to forbid inline.hpp files from being > included from being included from .hpp files. You need to rework this by > either moving the implementation to a .cpp file, or convert this file into an > .inline.hpp > > See the Source Files section in: > https://htmlpreview.github.io/?https://github.com/openjdk/jdk/blob/master/doc/hotspot-style.html Thanks, I renamed the file to threadWXSetters.inline.hpp - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v24]
> 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 with a new target base due to a merge or a rebase. The pull request now contains 105 commits: - Merge commit 'refs/pull/11/head' of https://github.com/AntonKozlov/jdk into jdk-macos - workaround JDK-8262895 by disabling subtest - Fix typo - Rename threadWXSetters.hpp -> threadWXSetters.inline.hpp - JDK-8259937: bsd_aarch64 part - Merge remote-tracking branch 'upstream/jdk/master' into jdk-macos - Fix after JDK-8259539, partially revert preconditions - JDK-8260471: bsd_aarch64 part - JDK-8259539: bsd_aarch64 part - JDK-8257828: bsd_aarch64 part - ... and 95 more: https://git.openjdk.java.net/jdk/compare/a6e34b3d...a72f6834 - Changes: https://git.openjdk.java.net/jdk/pull/2200/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2200=23 Stats: 2873 lines in 73 files changed: 2787 ins; 27 del; 59 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: 8257913: Add more known library locations to simplify Linux cross-compilation [v3]
On Mon, 8 Mar 2021 23:29:25 GMT, Magnus Ihse Bursie wrote: >> I thought we need to declare these as `AC_SUBST` as we define the variables >> outside this file? I can revert these additions if you think these are >> incorrect. > > No, they are only needed to be able to populate the @FOO@ tags in > spec.gmk.in. All the *.m4 files are concatenated into a single shell script > by autoconf, so referring to variables in a different m4 file does not really > matter. > > I don't think it's worth a separate bug to remove them, but if you touch the > area again you can get rid of them while you're at it :) Noted. Thanks! - PR: https://git.openjdk.java.net/jdk/pull/2833
