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 [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 [v8]
> **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/89c20e04...7b0b0dc4 - Changes: - all: https://git.openjdk.java.net/jdk/pull/2403/files - new: https://git.openjdk.java.net/jdk/pull/2403/files/7340d067..7b0b0dc4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2403=07 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2403=06-07 Stats: 6664 lines in 263 files changed: 3414 ins; 1659 del; 1591 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
