Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v27]
On Fri, 12 Mar 2021 16:44:38 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:
>
> Fix most of issues in java/foreign/ tests
>
> Failures related to va_args are tracked in JDK-8263512.
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: Thread* thread = Thread::current_or_null_safe();
Sorry but this should be MACOS_AARCH64 only. All three lines need to be ifdef'd
if you are going to include the assertion.
Thanks,
David
-
PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8189198: Add "forRemoval = true" to Applet API deprecations
On Wed, 10 Mar 2021 18:33:37 GMT, Andy Herrick wrote: > implementation of > JDK-8256145: JEP 398: Deprecate the Applet API for Removal Marked as reviewed by almatvee (Committer). - PR: https://git.openjdk.java.net/jdk/pull/2920
Re: [OpenJDK 2D-Dev] RFR: 8255790: GTKL: Java 16 crashes on initialising GTKL on Manjaro Linux
On Sat, 13 Mar 2021 00:15:16 GMT, Phil Race wrote: > From a build perspective this partially reverts > https://bugs.openjdk.java.net/browse/JDK-8249821 except that it keeps > the harfbuzz sources separate and still supports building and running against > a system harfbuzz which is only of interest or relevance on Linux. > > I ended up having to go this way because its is the least unsatisfactory > solution. > I did not want us to build a devkit to link against a system linux only to > find we couldn't use it at runtime > because too many systems have to old a version of harfbuzz. > > This solves the Manjaro Linux problem and I've manually verified building > against a system hardbuxz on Ubuntu 20.10 > > There are couple of incidental fixes in here too > - "libharfbuzz" should not have been in the EXTRA_HEADERS var when building > against a system version > - harfbuzz/hb-ucdn is gone and should not be listed as a header directory > needed to build the bundled copy > - I expect it also resolves https://bugs.openjdk.java.net/browse/JDK-8262502 Marked as reviewed by serb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2982
[OpenJDK 2D-Dev] RFR: 8255790: GTKL: Java 16 crashes on initialising GTKL on Manjaro Linux
>From a build perspective this partially reverts >https://bugs.openjdk.java.net/browse/JDK-8249821 except that it keeps the harfbuzz sources separate and still supports building and running against a system harfbuzz which is only of interest or relevance on Linux. I ended up having to go this way because its is the least unsatisfactory solution. I did not want us to build a devkit to link against a system linux only to find we couldn't use it at runtime because too many systems have to old a version of harfbuzz. This solves the Manjaro Linux problem and I've manually verified building against a system hardbuxz on Ubuntu 20.10 There are couple of incidental fixes in here too - "libharfbuzz" should not have been in the EXTRA_HEADERS var when building against a system version - harfbuzz/hb-ucdn is gone and should not be listed as a header directory needed to build the bundled copy - I expect it also resolves https://bugs.openjdk.java.net/browse/JDK-8262502 - Commit messages: - 8255790: GTKL: Java 16 crashes on initialising GTKL on Manjaro Linux Changes: https://git.openjdk.java.net/jdk/pull/2982/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2982=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8255790 Stats: 96 lines in 2 files changed: 7 ins; 57 del; 32 mod Patch: https://git.openjdk.java.net/jdk/pull/2982.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2982/head:pull/2982 PR: https://git.openjdk.java.net/jdk/pull/2982
Re: [OpenJDK 2D-Dev] RFR: 8263311: Watch registry changes for remote printers update instead of polling
On Wed, 10 Mar 2021 15:38:27 GMT, Alexey Ivanov wrote: > [JDK-8153732](https://bugs.openjdk.java.net/browse/JDK-8153732) implemented > polling for remote printers. > That bug description also mentions watching the registry for changes and > links to the article which describes the method yet it does so in terms of > WMI. Using WMI is not necessary to watch for the registry updates. > > It is possible to replace polling mechanism with registry change > notifications. If the registry at `HKCU\Printers\Connections` is updated, > refresh the list of print services. > > It works perfectly well in my own testing with sharing a Generic / Text Only > printer from another laptop. The notification comes as soon as the printer is > installed, it results in a new key created under `Connections`. If a remote > printer is removed, the notification is also triggered as the key > corresponding to that printer is removed from the registry. > > I updated the steps in the manual test: `RemotePrinterStatusRefresh.java`. @aivanov-jdk can you please check what that old comment was about: https://bugs.openjdk.java.net/browse/JDK-8153732?focusedCommentId=14188974=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-14188974 Any idea why RegistryValueChange was rejected as a solution? - PR: https://git.openjdk.java.net/jdk/pull/2915
Re: [OpenJDK 2D-Dev] RFR: 8189198: Add "forRemoval = true" to Applet API deprecations
On Wed, 10 Mar 2021 18:33:37 GMT, Andy Herrick wrote: > implementation of > JDK-8256145: JEP 398: Deprecate the Applet API for Removal Marked as reviewed by iris (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2920
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v27]
> 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: Fix most of issues in java/foreign/ tests Failures related to va_args are tracked in JDK-8263512. - Changes: - all: https://git.openjdk.java.net/jdk/pull/2200/files - new: https://git.openjdk.java.net/jdk/pull/2200/files/29991c92..5bfe0f08 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2200=26 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2200=25-26 Stats: 5 lines in 2 files changed: 4 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/2200.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2200/head:pull/2200 PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v21]
On Tue, 9 Mar 2021 18:01:11 GMT, Anton Kozlov wrote: >> 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? Robustness, clarity, maintainability, convention. Why not? - PR: https://git.openjdk.java.net/jdk/pull/2200
[OpenJDK 2D-Dev] RFR: 8189198: Add "forRemoval = true" to Applet API deprecations
implementation of JDK-8256145: JEP 398: Deprecate the Applet API for Removal - Commit messages: - 8189198: Add "forRemoval = true" to Applet API deprecations Changes: https://git.openjdk.java.net/jdk/pull/2920/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2920=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8189198 Stats: 74 lines in 22 files changed: 14 ins; 0 del; 60 mod Patch: https://git.openjdk.java.net/jdk/pull/2920.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2920/head:pull/2920 PR: https://git.openjdk.java.net/jdk/pull/2920
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v24]
On Tue, 9 Mar 2021 16:12:36 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 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 > @theRealAph, could you elaborate on what is need to be done for [#2200 > (review)](https://github.com/openjdk/jdk/pull/2200#pullrequestreview-600597066). I think that what you've got now is fine. - Marked as reviewed by aph (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v25]
On Thu, 11 Mar 2021 20:28:46 GMT, Stefan Karlsson 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 > > Marked as reviewed by stefank (Reviewer). > you still need to use Thread::current_or_null_safe() [for SafeFetch]. OK :) I fixed this in https://github.com/openjdk/jdk/pull/2200/commits/fd4812e585e0528010a8863df50956a3b64a6744 @dcubed-ojdk, I also updated copyrights, this concludes fixes for the review https://github.com/openjdk/jdk/pull/2200#pullrequestreview-581784107. @theRealAph, could you elaborate on what is need to be done for https://github.com/openjdk/jdk/pull/2200#pullrequestreview-600597066. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v9]
On Tue, 2 Mar 2021 08:12:10 GMT, Anton Kozlov wrote: >> I wasn't able to replicate JDK-8020753 and JDK-8186286. So will remove these >> workaround >> @gerard-ziemski, 8020753 was originally your fix, do you know if it still >> needed on intel-mac ? > > The x86_bsd still carries the workaround > https://github.com/openjdk/jdk/blob/master/src/hotspot/os_cpu/bsd_x86/os_bsd_x86.cpp#L745. > It's worth having macos ports to be aligned by features. I would propose to > have this workaround for now, and decide on it later for macos/x86 and > macos/aarch64 at once. Sorry for chiming in so late. Hello Anton. Please keep JDK-8020753 away from this port, as JDK-8020753 was very intel-specific workaround and macos11.0 on aarch64 doesn't show any signs of original bug. - PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8263311: Watch registry changes for remote printers update instead of polling
On Wed, 10 Mar 2021 15:38:27 GMT, Alexey Ivanov wrote: > [JDK-8153732](https://bugs.openjdk.java.net/browse/JDK-8153732) implemented > polling for remote printers. > That bug description also mentions watching the registry for changes and > links to the article which describes the method yet it does so in terms of > WMI. Using WMI is not necessary to watch for the registry updates. > > It is possible to replace polling mechanism with registry change > notifications. If the registry at `HKCU\Printers\Connections` is updated, > refresh the list of print services. > > It works perfectly well in my own testing with sharing a Generic / Text Only > printer from another laptop. The notification comes as soon as the printer is > installed, it results in a new key created under `Connections`. If a remote > printer is removed, the notification is also triggered as the key > corresponding to that printer is removed from the registry. > > I updated the steps in the manual test: `RemotePrinterStatusRefresh.java`. Marked as reviewed by psadhukhan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2915
Re: [OpenJDK 2D-Dev] RFR: 8263311: Watch registry changes for remote printers update instead of polling
On Fri, 12 Mar 2021 12:30:13 GMT, Alexey Ivanov wrote: >> I can understand that as a user, you cannot /shouldn't change the name of a >> remote network printer but a network admin can change the name, so shouldn't >> we get notification on that name change when this method gets called after >> the name change? or would it be notified as a new printer in that case? Then >> what will happen to the old stale printer in the registry? > > You can't. Try it yourself. The printer connection is per-user, after all > it's stored under HKCU. > > Windows displays the name of remote printers as “Generic / Text Only on > 192.168.1.18”: _the name of the printer_ and _the remote host_. > > If you open the *Printer Properties* dialog of a remote printer and edit its > name, you change the name of the printer on the remote host which shares it. > It changes *absolutely nothing* on the local system. > >> shouldn't we get notification on that name change when this method gets >> called after the name change? > > Yes, we should. But we cannot. > > For Windows, nothing has changed on the local system, it's the remote system > that has been changed. > >> or would it be notified as a new printer in that case? > > No, it wouldn't _because nothing has changed on the local system_. > >> Then what will happen to the old stale printer in the registry? > > Nothing, again. It just stays there but Windows reports an error if you try > to open its Printer Properties, Windows reports an error if you try to print > to it. I tried printing with Java and Notepad: the behaviour is the same. The > difference is that Notepad displays the error message whereas Java throws an > exception (it depends on the Java app, I guess). > > The behaviour aligns to the one seen when the printer is unreachable because > of, let's say, network issues. OK. - PR: https://git.openjdk.java.net/jdk/pull/2915
Re: [OpenJDK 2D-Dev] RFR: 8263311: Watch registry changes for remote printers update instead of polling
On Fri, 12 Mar 2021 11:38:08 GMT, Prasanta Sadhukhan wrote: >>> Is this only about addition/removal? What about printer name change? >> >> You cannot change the name of a remote printer. >> >> Opening *Printer Properties* dialog from the printer context menu on the >> local host and editing its name changes the name of the printer on the >> remote host which shares it. Windows warns, “This is a shared printer. If >> you rename a shared printer, existing connections to this printer from other >> computers will break and will have to be created again.” >> >> Nothing has been updated on the local system after renaming the printer. As >> the warning said, the printer does not work any more because it refers to a >> printer that does not exist. >> >> To fix this, one has to add a new remote printer which refers to the new >> name of the printer on the remote host. >> >>> Shouldn't we get notified in that case as trying to print on printer with >>> old name will not find the printer!! >> >> I'm afraid there's nothing Java can do to mitigate renaming the printer. >> >>> If yes, in that regard I guess `REG_NOTIFY_CHANGE_LAST_SET` is the one to >>> go for as it states >>> _"Notify the caller of changes to a value of the key. This can include >>> adding or deleting a value, or changing an existing value. "_ >> >> Since no values are created, removed, or changed when a remote printer is >> added or removed under `Connections` key, adding >> `REG_NOTIFY_CHANGE_LAST_SET` to `dwNotifyFilter` is not required. There are >> some values stored in the printer key but Java does not use them directly; >> if any value is changed there, getting notifications about such an event >> only creates noise. >> >> So, as far as Java is concerned, getting notifications about new keys being >> created or removed under `Connections` is enough. This is what >> `REG_NOTIFY_CHANGE_NAME` filter does. > > I can understand that as a user, you cannot /shouldn't change the name of a > remote network printer but a network admin can change the name, so shouldn't > we get notification on that name change when this method gets called after > the name change? or would it be notified as a new printer in that case? Then > what will happen to the old stale printer in the registry? You can't. Try it yourself. The printer connection is per-user, after all it's stored under HKCU. Windows displays the name of remote printers as “Generic / Text Only on 192.168.1.18”: _the name of the printer_ and _the remote host_. If you open the *Printer Properties* dialog of a remote printer and edit its name, you change the name of the printer on the remote host which shares it. It changes *absolutely nothing* on the local system. > shouldn't we get notification on that name change when this method gets > called after the name change? Yes, we should. But we cannot. For Windows, nothing has changed on the local system, it's the remote system that has been changed. > or would it be notified as a new printer in that case? No, it wouldn't _because nothing has changed on the local system_. > Then what will happen to the old stale printer in the registry? Nothing, again. It just stays there but Windows reports an error if you try to open its Printer Properties, Windows reports an error if you try to print to it. I tried printing with Java and Notepad: the behaviour is the same. The difference is that Notepad displays the error message whereas Java throws an exception (it depends on the Java app, I guess). The behaviour aligns to the one seen when the printer is unreachable because of, let's say, network issues. - PR: https://git.openjdk.java.net/jdk/pull/2915
Re: [OpenJDK 2D-Dev] RFR: 8253795: Implementation of JEP 391: macOS/AArch64 Port [v26]
> 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 three additional commits since the last revision: - Add Azul copyright - Update Oracle copyright years - Use Thread::current_or_null_safe in SafeFetch - Changes: - all: https://git.openjdk.java.net/jdk/pull/2200/files - new: https://git.openjdk.java.net/jdk/pull/2200/files/f6fb01b2..29991c92 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=2200=25 - incr: https://webrevs.openjdk.java.net/?repo=jdk=2200=24-25 Stats: 83 lines in 53 files changed: 41 ins; 0 del; 42 mod Patch: https://git.openjdk.java.net/jdk/pull/2200.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2200/head:pull/2200 PR: https://git.openjdk.java.net/jdk/pull/2200
Re: [OpenJDK 2D-Dev] RFR: 8263311: Watch registry changes for remote printers update instead of polling
On Fri, 12 Mar 2021 11:29:17 GMT, Alexey Ivanov wrote: >> Is this only about addition/removal? What about printer name change? >> Shouldn't we get notified in that case as trying to print on printer with >> old name will not find the printer!! >> If yes, in that regard I guess REG_NOTIFY_CHANGE_LAST_SET is the one to go >> for as it states >> `"Notify the caller of changes to a value of the key. This can include >> adding or deleting a value, or changing an existing value. "` > >> Is this only about addition/removal? What about printer name change? > > You cannot change the name of a remote printer. > > Opening *Printer Properties* dialog from the printer context menu on the > local host and editing its name changes the name of the printer on the remote > host which shares it. Windows warns, “This is a shared printer. If you rename > a shared printer, existing connections to this printer from other computers > will break and will have to be created again.” > > Nothing has been updated on the local system after renaming the printer. As > the warning said, the printer does not work any more because it refers to a > printer that does not exist. > > To fix this, one has to add a new remote printer which refers to the new name > of the printer on the remote host. > >> Shouldn't we get notified in that case as trying to print on printer with >> old name will not find the printer!! > > I'm afraid there's nothing Java can do to mitigate renaming the printer. > >> If yes, in that regard I guess `REG_NOTIFY_CHANGE_LAST_SET` is the one to go >> for as it states >> _"Notify the caller of changes to a value of the key. This can include >> adding or deleting a value, or changing an existing value. "_ > > Since no values are created, removed, or changed when a remote printer is > added or removed under `Connections` key, adding `REG_NOTIFY_CHANGE_LAST_SET` > to `dwNotifyFilter` is not required. There are some values stored in the > printer key but Java does not use them directly; if any value is changed > there, getting notifications about such an event only creates noise. > > So, as far as Java is concerned, getting notifications about new keys being > created or removed under `Connections` is enough. This is what > `REG_NOTIFY_CHANGE_NAME` filter does. I can understand that as a user, you cannot /shouldn't change the name of a remote network printer but a network admin can change the name, so shouldn't we get notification on that name change when this method gets called after the name change? or would it be notified as a new printer in that case? Then what will happen to the old stale printer in the registry? - PR: https://git.openjdk.java.net/jdk/pull/2915
Re: [OpenJDK 2D-Dev] RFR: 8263311: Watch registry changes for remote printers update instead of polling
On Fri, 12 Mar 2021 04:55:45 GMT, Prasanta Sadhukhan wrote: > Is this only about addition/removal? What about printer name change? You cannot change the name of a remote printer. Opening *Printer Properties* dialog from the printer context menu on the local host and editing its name changes the name of the printer on the remote host which shares it. Windows warns, “This is a shared printer. If you rename a shared printer, existing connections to this printer from other computers will break and will have to be created again.” Nothing has been updated on the local system after renaming the printer. As the warning said, the printer does not work any more because it refers to a printer that does not exist. To fix this, one has to add a new remote printer which refers to the new name of the printer on the remote host. > Shouldn't we get notified in that case as trying to print on printer with old > name will not find the printer!! I'm afraid there's nothing Java can do to mitigate renaming the printer. > If yes, in that regard I guess `REG_NOTIFY_CHANGE_LAST_SET` is the one to go > for as it states > _"Notify the caller of changes to a value of the key. This can include adding > or deleting a value, or changing an existing value. "_ Since no values are created, removed, or changed when a remote printer is added or removed under `Connections` key, adding `REG_NOTIFY_CHANGE_LAST_SET` to `dwNotifyFilter` is not required. There are some values stored in the printer key but Java does not use them directly; if any value is changed there, getting notifications about such an event only creates noise. So, as far as Java is concerned, getting notifications about new keys being created or removed under `Connections` is enough. This is what `REG_NOTIFY_CHANGE_NAME` filter does. - PR: https://git.openjdk.java.net/jdk/pull/2915
Re: [OpenJDK 2D-Dev] RFR: 8260931: Implement JEP 382: New macOS Rendering Pipeline [v13]
On Fri, 12 Mar 2021 10:11:27 GMT, Prasanta Sadhukhan 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/f5d78efb...369c3d21 > > src/java.desktop/macosx/classes/sun/awt/CGraphicsConfig.java line 82: > >> 80: * Creates a new SurfaceData that will be associated with the given >> 81: * CGLLayer. >> 82: */ > > I guess we need to specify CGLLayer/MTLLayer as now we are implementing this > method in both pipeline. I have updated JDK-8263363 to include this. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: [OpenJDK 2D-Dev] 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/d4addb43...369c3d21 Marked as reviewed by psadhukhan (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: [OpenJDK 2D-Dev] 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/b163fb99...369c3d21 src/java.desktop/macosx/classes/sun/awt/CGraphicsConfig.java line 82: > 80: * Creates a new SurfaceData that will be associated with the given > 81: * CGLLayer. > 82: */ I guess we need to specify CGLLayer/MTLLayer as now we are implementing this method in both pipeline. - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: [OpenJDK 2D-Dev] 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/46a8379b...369c3d21 Marked as reviewed by jdv (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2403
Re: [OpenJDK 2D-Dev] RFR: 8263482: Make access to the ICC color profiles data multithread-friendly
On Fri, 12 Mar 2021 05:10:25 GMT, Sergey Bylokhov wrote:
> FYI: probably is better/simpler to review it via webrev.
>
> After migration to the lcms from the kcms the performance of some operations
> was regressed. One possible workaround was to split the operation into
> multiple threads. But unfortunately, we have too many bottlenecks which
> prevent using multithreading. This is the request to remove/minimize such
> bottlenecks(at least some of them), but it does not affect the
> single-threaded performance it should be handled separately.
>
> The main code pattern optimized here is this:
> activate();
> byte[] theHeader = getData(cmmProfile, icSigHead);
> > CMSManager.getModule().getTagData(p, tagSignature);
> Notes about the code above:
>
> 1. Before the change "activate()" method checked that the "cmmProfile" field
> was not null. After that we usually used the "cmmProfile" as the parameter to
> some other method. This included two volatile reads, and also required to
> check when we need to call the "activate()" method before usage of the
> "cmmProfile" field.
> Solution: "activate()" renamed to the "cmmProfile()" which became an accessor
> for the field, so we will get one volatile read and can easily monitor the
> usage of the field itself(it is used directly only in this method).
>
> 2. The synchronized static method "CMSManager.getModule()" reimplemented to
> have only one volatile read.
>
> 3. The usage of locking in the "getTagData()" is changed. Instead of the
> synchronized instance methods, we now use the mix of "ConcurrentHashMap" and
> StampedLock.
>
> See some comments inline.
>
> Some numbers(small numbers are better):
>
> 1. Performance of ((ICC_ProfileRGB)
> ICC_Profile.getInstance(ColorSpace.CS_sRGB)).getMatrix();
>
> jdk 15.0.2
> Benchmark Mode CntScore Error Units
> CMMPerf.ThreadsMAX.testGetMatrix avgt5 19,624 ±0,059 us/op
> CMMPerf.testGetMatrix avgt50,154 ±0,001 us/op
>
> jdk - before the fix
> Benchmark Mode CntScore Error Units
> CMMPerf.ThreadsMAX.testGetMatrix avgt5 12,935 ±0,042 us/op
> CMMPerf.testGetMatrix avgt50,127 ±0,007 us/op
>
> jdk - after the fix
> Benchmark Mode CntScore Error Units
> CMMPerf.ThreadsMAX.testGetMatrix avgt50,561 ±0,005 us/op
> CMMPerf.testGetMatrix avgt50,092 ±0,001 us/op
>
> 2. Part of performance gain in jdk17 is from some other fixes, for example
> Performance of ICC_Profile.getInstance(ColorSpace.CS_sRGB); and
> ColorSpace.getInstance(ColorSpace.CS_sRGB);
>
> jdk 15.0.2
> Benchmark Mode CntScore Error Units
> CMMPerf.ThreadsMAX.testGetSRGBProfile avgt52,299 ±0,032 us/op
> CMMPerf.ThreadsMAX.testGetSRGBSpaceavgt52,210 ±0,051 us/op
> CMMPerf.testGetSRGBProfile avgt50,019 ±0,001 us/op
> CMMPerf.testGetSRGBSpace avgt50,018 ±0,001 us/op
>
> jdk - same before/ after the fix
> Benchmark Mode CntScore Error Units
> CMMPerf.ThreadsMAX.testGetSRGBProfile avgt50,005 ±0,001 us/op
> CMMPerf.ThreadsMAX.testGetSRGBSpaceavgt50,005 ±0,001 us/op
> CMMPerf.testGetSRGBProfile avgt50,005 ±0,001 us/op
> CMMPerf.testGetSRGBSpace avgt50,005 ±0,001 us/op
>
> note "ThreadsMAX" is 32 threads.
src/java.desktop/share/classes/sun/java2d/cmm/lcms/LCMSProfile.java line 70:
> 68: } finally {
> 69: lock.unlockRead(stamp);
> 70: }
The comments about the change in this class and the changes in the "getTag()"
method.
1. I have removed the TagData and TagCache classes and move the tag cache to
the profile class directly.
2. I have moved synchronization logic from the LCMS.java to this class, so now
we can use more "granular" synchronizations.
The logic behind this change:
1. The StampedLock guards the access to the native lcms code, so when we change
the tags via "LCMS.setTagDataNative" nobody will call "LCMS.getTagNative" and
"LCMS.getProfileDataNative". I tried the "ReentrantReadWriteLock", but it is a
little bit slower.
2. The cache itself is maintained by the ConcurrentHashMap, and the usage of
"byte[] t = tags.get(sig);" w/o synchronization is a key for the performance.
Note that it is possible to use "tags.computeIfAbsent" instead of
"tags.get(sig)" and take care of "native access" synchronization in the
"LCMS.getTagNative", but unfortuntly for some workload the "get()" is x10 times
faster than computeIfAbsent() if the key is already exists in the map(common
situation for the cache).
src/java.desktop/share/classes/java/awt/color/ICC_Profile.java line 1103:
> 1101: public
[OpenJDK 2D-Dev] RFR: 8263482: Make access to the ICC color profiles data multithread-friendly
FYI: probably is better/simpler to review it via webrev. After migration to the lcms from the kcms the performance of some operations was regressed. One possible workaround was to split the operation into multiple threads. But unfortunately, we have too many bottlenecks which prevent using multithreading. This is the request to remove/minimize such bottlenecks(at least some of them), but it does not affect the single-threaded performance it should be handled separately. The main code pattern optimized here is this: activate(); byte[] theHeader = getData(cmmProfile, icSigHead); > CMSManager.getModule().getTagData(p, tagSignature); Notes about the code above: 1. Before the change "activate()" method checked that the "cmmProfile" field was not null. After that we usually used the "cmmProfile" as the parameter to some other method. This included two volatile reads, and also required to check when we need to call the "activate()" method before usage of the "cmmProfile" field. Solution: "activate()" renamed to the "cmmProfile()" which became an accessor for the field, so we will get one volatile read and can easily monitor the usage of the field itself(it is used directly only in this method). 2. The synchronized static method "CMSManager.getModule()" reimplemented to have only one volatile read. 3. The usage of locking in the "getTagData()" is changed. Instead of the synchronized instance methods, we now use the mix of "ConcurrentHashMap" and StampedLock. See some comments inline. Some numbers(small numbers are better): 1. Performance of ((ICC_ProfileRGB) ICC_Profile.getInstance(ColorSpace.CS_sRGB)).getMatrix(); jdk 15.0.2 Benchmark Mode CntScore Error Units CMMPerf.ThreadsMAX.testGetMatrix avgt5 19,624 ±0,059 us/op CMMPerf.testGetMatrix avgt50,154 ±0,001 us/op jdk - before the fix Benchmark Mode CntScore Error Units CMMPerf.ThreadsMAX.testGetMatrix avgt5 12,935 ±0,042 us/op CMMPerf.testGetMatrix avgt50,127 ±0,007 us/op jdk - after the fix Benchmark Mode CntScore Error Units CMMPerf.ThreadsMAX.testGetMatrix avgt50,561 ±0,005 us/op CMMPerf.testGetMatrix avgt50,092 ±0,001 us/op 2. Part of performance gain in jdk17 is from some other fixes, for example Performance of ICC_Profile.getInstance(ColorSpace.CS_sRGB); and ColorSpace.getInstance(ColorSpace.CS_sRGB); jdk 15.0.2 Benchmark Mode CntScore Error Units CMMPerf.ThreadsMAX.testGetSRGBProfile avgt52,299 ±0,032 us/op CMMPerf.ThreadsMAX.testGetSRGBSpaceavgt52,210 ±0,051 us/op CMMPerf.testGetSRGBProfile avgt50,019 ±0,001 us/op CMMPerf.testGetSRGBSpace avgt50,018 ±0,001 us/op jdk - same before/ after the fix Benchmark Mode CntScore Error Units CMMPerf.ThreadsMAX.testGetSRGBProfile avgt50,005 ±0,001 us/op CMMPerf.ThreadsMAX.testGetSRGBSpaceavgt50,005 ±0,001 us/op CMMPerf.testGetSRGBProfile avgt50,005 ±0,001 us/op CMMPerf.testGetSRGBSpace avgt50,005 ±0,001 us/op note "ThreadsMAX" is 32 threads. - Commit messages: - Update ICC_Profile.java - Merge branch 'master' into threads - Update LCMS.c - Initial fix Changes: https://git.openjdk.java.net/jdk/pull/2957/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=2957=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8263482 Stats: 212 lines in 5 files changed: 52 ins; 83 del; 77 mod Patch: https://git.openjdk.java.net/jdk/pull/2957.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/2957/head:pull/2957 PR: https://git.openjdk.java.net/jdk/pull/2957
Re: [OpenJDK 2D-Dev] 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/392a1086...369c3d21 Marked as reviewed by kizune (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/2403
