Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v8]
On Mon, 15 Feb 2021 20:52:09 GMT, Gerard Ziemski 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 20 additional >> commits since the last revision: >> >> - Lanai PR#191 - 8261705 - jdv >> - Lanai PR#190 - 8261706 - jdv >> - Lanai PR#189 - 8261712 - avu >> - Lanai PR#187 - 8261704 - jdv >> - Lanai PR#186 - 8261638 - avu >> - Lanai PR#185 - 8261632 - jdv >> - Lanai PR#184 - 8261620 - aghaisas >> - Lanai PR#182 - 8261547 - psadhukhan >> - Merge branch 'master' into 8260931_lanai_JEP_branch >> - Lanai PR#181 - 8261143 - aghaisas >> - ... and 10 more: >> https://git.openjdk.java.net/jdk/compare/c8554bef...7b0b0dc4 > > 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. - 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 06:24:13 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 20 additional > commits since the last revision: > > - Lanai PR#191 - 8261705 - jdv > - Lanai PR#190 - 8261706 - jdv > - Lanai PR#189 - 8261712 - avu > - Lanai PR#187 - 8261704 - jdv > - Lanai PR#186 - 8261638 - avu > - Lanai PR#185 - 8261632 - jdv > - Lanai PR#184 - 8261620 - aghaisas > - Lanai PR#182 - 8261547 - psadhukhan > - Merge branch 'master' into 8260931_lanai_JEP_branch > - Lanai PR#181 - 8261143 - aghaisas > - ... and 10 more: > https://git.openjdk.java.net/jdk/compare/827ab648...7b0b0dc4 Marked as reviewed by gziemski (Committer). - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v7]
On Mon, 15 Feb 2021 18:30:51 GMT, Gerard Ziemski wrote: >> Changes requested by gziemski (Committer). > > I took a look at https://bugs.openjdk.java.net/browse/JDK-8261408 and noticed > a startup time regression with the Metal rendering pipeline, so I dug a bit > and here is what I found using Xcode startup profiler: > > Here is the OpenGL pipeline: > src="https://user-images.githubusercontent.com/63425797/107981859-8efb4780-6f88-11eb-849a-385c29f3ff6f.png;> > > > Here is the Metal pipeline: > src="https://user-images.githubusercontent.com/63425797/107981882-9ae70980-6f88-11eb-9b73-d1bd19dfa07f.png;> > > If I read the results correctly, there is a weird 330ms time gap in the case > of the Metal pipeline where nothing is done and it looks like the culprit is > `Java_sun_java2d_metal_MTLGraphicsConfig_isMetalFrameworkAvailable` > > The OpenGL pipeline gets blocked only for 83ms in > `Java_sun_java2d_opengl_CGLGraphicsConfig_getCGLConfigInfo` > > I would advice that we take a closer look at why > 'Java_sun_java2d_metal_MTLGraphicsConfig_isMetalFrameworkAvailable' takes so > long, and optimize it, or cache the results, so the next VM instance can skip > it if needed (assuming the call doesn't end up initializing the native Metal > or something like that), still worth taking a look. > > I would also recommend that you adopt Xcode and its Instruments profiler > tools for future work. Please let me know if you need help in this area. > > I only scratched the surface here and I think that there is plenty of > profiling that can be done to investigate the startup time regression further. > > 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. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v7]
On Mon, 15 Feb 2021 18:30:51 GMT, Gerard Ziemski wrote: >> Changes requested by gziemski (Committer). > > I took a look at https://bugs.openjdk.java.net/browse/JDK-8261408 and noticed > a startup time regression with the Metal rendering pipeline, so I dug a bit > and here is what I found using Xcode startup profiler: > > Here is the OpenGL pipeline: > src="https://user-images.githubusercontent.com/63425797/107981859-8efb4780-6f88-11eb-849a-385c29f3ff6f.png;> > > > Here is the Metal pipeline: > src="https://user-images.githubusercontent.com/63425797/107981882-9ae70980-6f88-11eb-9b73-d1bd19dfa07f.png;> > > If I read the results correctly, there is a weird 330ms time gap in the case > of the Metal pipeline where nothing is done and it looks like the culprit is > `Java_sun_java2d_metal_MTLGraphicsConfig_isMetalFrameworkAvailable` > > The OpenGL pipeline gets blocked only for 83ms in > `Java_sun_java2d_opengl_CGLGraphicsConfig_getCGLConfigInfo` > > I would advice that we take a closer look at why > 'Java_sun_java2d_metal_MTLGraphicsConfig_isMetalFrameworkAvailable' takes so > long, and optimize it, or cache the results, so the next VM instance can skip > it if needed (assuming the call doesn't end up initializing the native Metal > or something like that), still worth taking a look. > > I would also recommend that you adopt Xcode and its Instruments profiler > tools for future work. Please let me know if you need help in this area. > > I only scratched the surface here and I think that there is plenty of > profiling that can be done to investigate the startup time regression further. > 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. - PR: https://git.openjdk.java.net/jdk/pull/2403
Integrated: 8261351: Create implementation for NSAccessibilityRadioButton protocol
On Sat, 13 Feb 2021 05:01:17 GMT, Alexander Zuev wrote: > 8261351: Create implementation for NSAccessibilityRadioButton protocol This pull request has now been integrated. Changeset: 6badd22e Author:Alexander Zuev URL: https://git.openjdk.java.net/jdk/commit/6badd22e Stats: 77 lines in 3 files changed: 76 ins; 0 del; 1 mod 8261351: Create implementation for NSAccessibilityRadioButton protocol Reviewed-by: pbansal - PR: https://git.openjdk.java.net/jdk/pull/2561
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v7]
On Mon, 15 Feb 2021 18:30:51 GMT, Gerard Ziemski wrote: >> Changes requested by gziemski (Committer). > > I took a look at https://bugs.openjdk.java.net/browse/JDK-8261408 and noticed > a startup time regression with the Metal rendering pipeline, so I dug a bit > and here is what I found using Xcode startup profiler: > > Here is the OpenGL pipeline: > src="https://user-images.githubusercontent.com/63425797/107981859-8efb4780-6f88-11eb-849a-385c29f3ff6f.png;> > > > Here is the Metal pipeline: > src="https://user-images.githubusercontent.com/63425797/107981882-9ae70980-6f88-11eb-9b73-d1bd19dfa07f.png;> > > If I read the results correctly, there is a weird 330ms time gap in the case > of the Metal pipeline where nothing is done and it looks like the culprit is > `Java_sun_java2d_metal_MTLGraphicsConfig_isMetalFrameworkAvailable` > > The OpenGL pipeline gets blocked only for 83ms in > `Java_sun_java2d_opengl_CGLGraphicsConfig_getCGLConfigInfo` > > I would advice that we take a closer look at why > 'Java_sun_java2d_metal_MTLGraphicsConfig_isMetalFrameworkAvailable' takes so > long, and optimize it, or cache the results, so the next VM instance can skip > it if needed (assuming the call doesn't end up initializing the native Metal > or something like that), still worth taking a look. > > I would also recommend that you adopt Xcode and its Instruments profiler > tools for future work. Please let me know if you need help in this area. > > I only scratched the surface here and I think that there is plenty of > profiling that can be done to investigate the startup time regression further. 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). - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v7]
On Thu, 11 Feb 2021 20:55:47 GMT, Gerard Ziemski wrote: >> Ajit Ghaisas has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Lanai PR#181 - 8261143 - aghaisas >> - Lanai PR#180 - 8261546 - jdv > > Changes requested by gziemski (Committer). I took a look at https://bugs.openjdk.java.net/browse/JDK-8261408 and noticed a startup time regression with the Metal rendering pipeline, so I dug a bit and here is what I found using Xcode startup profiler: Here is the OpenGL pipeline: https://user-images.githubusercontent.com/63425797/107981859-8efb4780-6f88-11eb-849a-385c29f3ff6f.png;> Here is the Metal pipeline: https://user-images.githubusercontent.com/63425797/107981882-9ae70980-6f88-11eb-9b73-d1bd19dfa07f.png;> If I read the results correctly, there is a weird 330ms time gap in the case of the Metal pipeline where nothing is done and it looks like the culprit is `Java_sun_java2d_metal_MTLGraphicsConfig_isMetalFrameworkAvailable` The OpenGL pipeline gets blocked only for 83ms in `Java_sun_java2d_opengl_CGLGraphicsConfig_getCGLConfigInfo` I would advice that we take a closer look at why 'Java_sun_java2d_metal_MTLGraphicsConfig_isMetalFrameworkAvailable' takes so long, and optimize it, or cache the results, so the next VM instance can skip it if needed (assuming the call doesn't end up initializing the native Metal or something like that), still worth taking a look. I would also recommend that you adopt Xcode and its Instruments profiler tools for future work. Please let me know if you need help in this area. I only scratched the surface here and I think that there is plenty of profiling that can be done to investigate the startup time regression further. - PR: https://git.openjdk.java.net/jdk/pull/2403
