Re: RFR: 8279796: Fix typo: Constucts -> Constructs

[Sergey Bylokhov]

On Wed, 19 Jan 2022 22:18:32 GMT, Weijun Wang  wrote:

> Two edits.

src/java.desktop/share/classes/javax/swing/plaf/basic/BasicFileChooserUI.java 
line 727:

> 725: Handler handler;
> 726: /**
> 727:  * Constructs a {@code DoubleClickListener}.

This change is under review here:
https://github.com/openjdk/jdk/pull/7030

-

PR: https://git.openjdk.java.net/jdk/pull/7147

Re: RFR: 8280010: Remove double buffering of InputStream for Properties.load

[Sergey Bylokhov]

On Mon, 10 Jan 2022 20:46:36 GMT, Andrey Turbanov  wrote:

> `Properties.load` uses `java.util.Properties.LineReader`. LineReader already 
> buffers input stream. Hence wrapping InputStream in BufferedInputStream is 
> redundant.

Marked as reviewed by serb (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/7021

Re: RFR: 8280010: Remove double buffering of InputStream for Properties.load

[Sergey Bylokhov]

On Mon, 10 Jan 2022 20:46:36 GMT, Andrey Turbanov  wrote:

> `Properties.load` uses `java.util.Properties.LineReader`. LineReader already 
> buffers input stream. Hence wrapping InputStream in BufferedInputStream is 
> redundant.

The code change looks fine, but can you please check the actual performance 
impact, will the perf be the same after the change?

-

PR: https://git.openjdk.java.net/jdk/pull/7021

Integrated: 8279134: Fix Amazon copyright in various files

[Sergey Bylokhov]

On Wed, 22 Dec 2021 09:07:24 GMT, Sergey Bylokhov  wrote:

> This bug is similar to https://bugs.openjdk.java.net/browse/JDK-8244094
> 
> Currently, some of the files in the OpenJDK repo have Amazon copyright 
> notices which are all slightly different and do not conform to Amazons 
> preferred copyright notice which is simply (intentionally without copyright 
> year):
> 
> "Copyright Amazon.com Inc. or its affiliates. All Rights Reserved." 
> 
> @simonis @phohensee

This pull request has now been integrated.

Changeset: 7fea1032
Author:Sergey Bylokhov 
URL:   
https://git.openjdk.java.net/jdk/commit/7fea10327ed27fcf8eae474ca5b15c3b4bafff2a
Stats: 15 lines in 14 files changed: 0 ins; 1 del; 14 mod

8279134: Fix Amazon copyright in various files

Reviewed-by: xliu, phh

-

PR: https://git.openjdk.java.net/jdk/pull/6915

Re: RFR: 8279134: Fix Amazon copyright in various files [v2]

[Sergey Bylokhov]

> This bug is similar to https://bugs.openjdk.java.net/browse/JDK-8244094
> 
> Currently, some of the files in the OpenJDK repo have Amazon copyright 
> notices which are all slightly different and do not conform to Amazons 
> preferred copyright notice which is simply (intentionally without copyright 
> year):
> 
> "Copyright Amazon.com Inc. or its affiliates. All Rights Reserved." 
> 
> @simonis @phohensee

Sergey Bylokhov 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 two additional 
commits since the last revision:

 - Merge branch 'master' into JDK-8279134
 - Initial fix JDK-8279134

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/6915/files
  - new: https://git.openjdk.java.net/jdk/pull/6915/files/cb05f5bb..52c5d9c3

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=6915=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6915=00-01

  Stats: 619 lines in 42 files changed: 446 ins; 100 del; 73 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6915.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6915/head:pull/6915

PR: https://git.openjdk.java.net/jdk/pull/6915

RFR: 8279134: Fix Amazon copyright in various files

[Sergey Bylokhov]

This bug is similar to https://bugs.openjdk.java.net/browse/JDK-8244094

Currently, some of the files in the OpenJDK repo have Amazon copyright notices 
which are all slightly different and do not conform to Amazons preferred 
copyright notice which is simply (intentionally without copyright year):

"Copyright Amazon.com Inc. or its affiliates. All Rights Reserved." 

@simonis @phohensee

-

Commit messages:
 - Initial fix JDK-8279134

Changes: https://git.openjdk.java.net/jdk/pull/6915/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6915=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8279134
  Stats: 15 lines in 14 files changed: 0 ins; 1 del; 14 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6915.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6915/head:pull/6915

PR: https://git.openjdk.java.net/jdk/pull/6915

Re: RFR: 8273101: Eliminate the usage of threadgroup sandboxing in the java.util.logging [v2]

[Sergey Bylokhov]

> At the time Java supported applets and webstart, a special mechanism for 
> launching various applications in one JVM was used to reduce memory usage and 
> each application was isolated from each other.
> 
> This isolation was implemented via ThreadGroups where each application 
> created its own thread group and all data for the application was stored in 
> the context of its own group.
> 
> Some parts of the JDK used ThreadGroups directly, others used special classes 
> like sun.awt.AppContext.
> 
> Such sandboxing became useless since the plugin and webstart are no longer 
> supported and were removed in jdk11.
> 
> Note that this is just a first step for the overall cleanup, here is my plan:
> 1. Cleanup the usage of AppContext in the “java.util.logging.LogManager" 
> class in the java.base module.
> 2. Cleanup the usage of TheadGroup in the JavaSound
> 3. Cleanup the usage of TheadGroup in the JavaBeans
> 4. Cleanup the usage of AppContext in the Swing
> 5. Cleanup the usage of AppContext in the AWT
> 6. Delete the AppContext class.
> 
> The current PR is for the first step. So this is the request to delete the 
> usage of AppContext in the LogManager and related tests.
> 
> Tested by tier1/tier2/tier3 and jdk_desktop tests.

Sergey Bylokhov has updated the pull request with a new target base due to a 
merge or a rebase. The pull request now contains four commits:

 - Merge branch 'master' into JDK-8273101
 - Some tests deleted
 - Update the RootLevelInConfigFile test
 - Initial version of JDK-8273101

-

Changes: https://git.openjdk.java.net/jdk/pull/5326/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5326=01
  Stats: 1423 lines in 10 files changed: 0 ins; 1418 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5326.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5326/head:pull/5326

PR: https://git.openjdk.java.net/jdk/pull/5326

Re: RFR: 8278080: Add --with-cacerts-src='user cacerts folder' to enable deterministic cacerts generation

[Sergey Bylokhov]

On Thu, 2 Dec 2021 10:55:57 GMT, Andrew Leonard  wrote:

> This is the case at Adoptium for example, which uses the Mozilla trusted CA 
> certs.

But they didn't think skipping this test was too strong a step? For example 
validation of the certs expiration is quite useful. I tried to update the test 
to take into account additional certs, but it caused a merge conflict each time 
the certs in OpenJDK are updated. Probably we can add a config file that can 
inject/override some info in the test(at least skip the checksum validation)? 
By default this config file will be empty and will not be modified in the 
OpenJDK, but the vendors will be able to modify it. @wangweij @rhalade what do 
you think?

-

PR: https://git.openjdk.java.net/jdk/pull/6647

Re: RFR: JDK-8276447 Deprecate finalization-related methods for removal

[Sergey Bylokhov]

On Thu, 18 Nov 2021 21:51:30 GMT, Brent Christian  wrote:

> Here are the code changes for the "Deprecate finalizers in the standard Java 
> API" portion of JEP 421 ("Deprecate Finalization for Removal") for code 
> review.
> 
> This change makes the indicated deprecations, and updates the API spec for 
> JEP 421. It also updates the relevant @SuppressWarning annotations.
> 
> The CSR has been approved.
> An automated test build+test run passes cleanly (FWIW :D ).

Marked as reviewed by serb (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/6465

Re: RFR: 8273101: Eliminate the usage of threadgroup sandboxing in the java.util.logging

[Sergey Bylokhov]

On Fri, 29 Oct 2021 16:32:43 GMT, Mandy Chung  wrote:

> The change looks okay. My suggestion is to get 1-6 all ready to push around 
> the same time. It's okay to have separate JBS issues and PRs.

ok, I'll continue to work using the plan from the description.

-

PR: https://git.openjdk.java.net/jdk/pull/5326

Re: RFR: 8273101: Eliminate the usage of threadgroup sandboxing in the java.util.logging

[Sergey Bylokhov]

On Wed, 1 Sep 2021 06:31:16 GMT, Sergey Bylokhov  wrote:

> At the time Java supported applets and webstart, a special mechanism for 
> launching various applications in one JVM was used to reduce memory usage and 
> each application was isolated from each other.
> 
> This isolation was implemented via ThreadGroups where each application 
> created its own thread group and all data for the application was stored in 
> the context of its own group.
> 
> Some parts of the JDK used ThreadGroups directly, others used special classes 
> like sun.awt.AppContext.
> 
> Such sandboxing became useless since the plugin and webstart are no longer 
> supported and were removed in jdk11.
> 
> Note that this is just a first step for the overall cleanup, here is my plan:
> 1. Cleanup the usage of AppContext in the “java.util.logging.LogManager" 
> class in the java.base module.
> 2. Cleanup the usage of TheadGroup in the JavaSound
> 3. Cleanup the usage of TheadGroup in the JavaBeans
> 4. Cleanup the usage of AppContext in the Swing
> 5. Cleanup the usage of AppContext in the AWT
> 6. Delete the AppContext class.
> 
> The current PR is for the first step. So this is the request to delete the 
> usage of AppContext in the LogManager and related tests.
> 
> Tested by tier1/tier2/tier3 and jdk_desktop tests.

Does anybody have any other suggestions?

-

PR: https://git.openjdk.java.net/jdk/pull/5326

Re: RFR: 8268764: Use Long.hashCode() instead of int-cast where applicable [v4]

[Sergey Bylokhov]

On Thu, 1 Jul 2021 12:19:53 GMT, Сергей Цыпанов  wrote:

>> In some JDK classes there's still the following hashCode() implementation:
>> 
>> long objNum;
>> 
>> public int hashCode() {
>> return (int) objNum;
>> }
>> 
>> This outdated expression should be replaced with Long.hashCode(long) as it
>> 
>> - uses all bits of the original value, does not discard any information 
>> upfront. For example, depending on how you are generating the IDs, the upper 
>> bits could change more frequently (or the opposite).
>> 
>> - does not introduce any bias towards values with more ones (zeros), as it 
>> would be the case if the two halves were combined with an OR (AND) operation.
>> 
>> See https://stackoverflow.com/a/4045083
>> 
>> This is related to https://github.com/openjdk/jdk/pull/4309
>
> Сергей Цыпанов has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8268764: Update copy-right year

Marked as reviewed by serb (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/4491

Re: RFR: 8273101: Eliminate the usage of threadgroup sandboxing in the java.util.logging

[Sergey Bylokhov]

On Wed, 1 Sep 2021 06:31:16 GMT, Sergey Bylokhov  wrote:

> At the time Java supported applets and webstart, a special mechanism for 
> launching various applications in one JVM was used to reduce memory usage and 
> each application was isolated from each other.
> 
> This isolation was implemented via ThreadGroups where each application 
> created its own thread group and all data for the application was stored in 
> the context of its own group.
> 
> Some parts of the JDK used ThreadGroups directly, others used special classes 
> like sun.awt.AppContext.
> 
> Such sandboxing became useless since the plugin and webstart are no longer 
> supported and were removed in jdk11.
> 
> Note that this is just a first step for the overall cleanup, here is my plan:
> 1. Cleanup the usage of AppContext in the “java.util.logging.LogManager" 
> class in the java.base module.
> 2. Cleanup the usage of TheadGroup in the JavaSound
> 3. Cleanup the usage of TheadGroup in the JavaBeans
> 4. Cleanup the usage of AppContext in the Swing
> 5. Cleanup the usage of AppContext in the AWT
> 6. Delete the AppContext class.
> 
> The current PR is for the first step. So this is the request to delete the 
> usage of AppContext in the LogManager and related tests.
> 
> Tested by tier1/tier2/tier3 and jdk_desktop tests.

Does anybody have any other suggestions?

-

PR: https://git.openjdk.java.net/jdk/pull/5326

Re: RFR: 8273101: Eliminate the usage of threadgroup sandboxing in the java.util.logging

[Sergey Bylokhov]

On Mon, 6 Sep 2021 09:39:58 GMT, Alan Bateman  wrote:

> No objection to removing this legacy/unused code but I think it would be 
> useful to useful to have the JBS issue or the PR summary provide a bit more 
> context. As I see it, this is just one piece of the overall cleanup and I 
> assume there are more substantive changes to the java.desktop module coming, 
> is that right?

I have updated the PR and JBS

-

PR: https://git.openjdk.java.net/jdk/pull/5326

Re: RFR: 8273101: Eliminate the usage of threadgroup sandboxing in the java.util.logging

[Sergey Bylokhov]

On Wed, 1 Sep 2021 06:31:16 GMT, Sergey Bylokhov  wrote:

> The "java.util.logging.LogManager" class uses the "threadgroup sandboxing" 
> via an AppContext to support "applet logging isolation". The AppContext class 
> became useless since the plugin and webstart are no longer supported and 
> removed in jdk11.
> 
> This is the request to delete the usage of AppContext in the LogManager and 
> related tests.
> 
> Tested by tier1/tier2/tier3 and jdk_desktop tests.

I'll remove its used one by one from the external usage like in "java.base" 
here, then in Swing, then in 2D like fonts, then last in AWT.

That change should not break something other than the tests which intentionally 
create different appcontexts. And it's easy to review.

-

PR: https://git.openjdk.java.net/jdk/pull/5326

Re: RFR: 8273528: Avoid ByteArrayOutputStream.toByteArray when converting stream to String

[Sergey Bylokhov]

On Thu, 2 Sep 2021 20:19:40 GMT, Andrey Turbanov 
 wrote:

> Using `ByteArrayOutputStream.toString` to convert it's content to a String is 
> cleaner than `new String(out.toByteArray())`. Also it's a bit faster because 
> of one less array copy.

BTW If it is not urgent I suggest to wait 24 hours from the approval before the 
/integrate, people have different timezones and may not be able to add a 
comment if any in time.

-

PR: https://git.openjdk.java.net/jdk/pull/5355

Re: RFR: 8273528: Avoid ByteArrayOutputStream.toByteArray when converting stream to String

[Sergey Bylokhov]

On Thu, 2 Sep 2021 20:19:40 GMT, Andrey Turbanov 
 wrote:

> Using `ByteArrayOutputStream.toString` to convert it's content to a String is 
> cleaner than `new String(out.toByteArray())`. Also it's a bit faster because 
> of one less array copy.

Marked as reviewed by serb (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/5355

Re: RFR: 8273528: Avoid ByteArrayOutputStream.toByteArray when converting stream to String

[Sergey Bylokhov]

On Thu, 2 Sep 2021 20:19:40 GMT, Andrey Turbanov 
 wrote:

> Using `ByteArrayOutputStream.toString` to convert it's content to a String is 
> cleaner than `new String(out.toByteArray())`. Also it's a bit faster because 
> of one less array copy.

Looks fine

-

PR: https://git.openjdk.java.net/jdk/pull/5355

Re: RFR: 8273101: Eliminate the usage of threadgroup sandboxing in the java.util.logging

[Sergey Bylokhov]

On Mon, 6 Sep 2021 09:39:58 GMT, Alan Bateman  wrote:

> As I see it, this is just one piece of the overall cleanup and I assume there 
> are more substantive changes to the java.desktop module coming, is that right?

Yes, you are right.

-

PR: https://git.openjdk.java.net/jdk/pull/5326

Re: RFR: 8273101: Eliminate the usage of threadgroup sandboxing in the java.util.logging

[Sergey Bylokhov]

On Thu, 2 Sep 2021 09:59:51 GMT, Daniel Fuchs  wrote:

>> The "java.util.logging.LogManager" class uses the "threadgroup sandboxing" 
>> via an AppContext to support "applet logging isolation". The AppContext 
>> class became useless since the plugin and webstart are no longer supported 
>> and removed in jdk11.
>> 
>> This is the request to delete the usage of AppContext in the LogManager and 
>> related tests.
>> 
>> Tested by tier1/tier2/tier3 and jdk_desktop tests.
>
> test/jdk/java/util/logging/TestLoggingWithMainAppContext.java line 1:
> 
>> 1: /*
> 
> Humm... There's no direct reference to AppContext here. Maybe this test 
> should be preserved?

It uses it indirectly, the next line under security manager trigger creation of 
the appcontext:
`ImageIO.read(is); // triggers calls to system loggers & creation of main 
AppContext`

but I can preserve/rename it for sure.

-

PR: https://git.openjdk.java.net/jdk/pull/5326

Re: RFR: 8273101: Eliminate the usage of threadgroup sandboxing in the java.util.logging

[Sergey Bylokhov]

On Fri, 3 Sep 2021 17:19:05 GMT, Phil Race  wrote:

> Perhaps this isn't the change that requires the CSR but it then leaves an 
> inconsistent state where desktop supports AppContext still but other modules 
> don't ...

Even java.desktop does not support it fully, since for a couple of years nobody 
care about appcontext when write a new code.
Note that I mentioned the "threadgroup sandboxing" in the subject, which is not 
necessary implemented via Appcontext. The JavaBeans and JavaSound use the 
thread group directly, I plan to remove them as well.

-

PR: https://git.openjdk.java.net/jdk/pull/5326

Integrated: 8272805: Avoid looking up standard charsets

[Sergey Bylokhov]

On Sun, 22 Aug 2021 02:53:44 GMT, Sergey Bylokhov  wrote:

> This is the continuation of JDK-8233884, JDK-8271456, and JDK-8272120.
> 
> In many places standard charsets are looked up via their names, for example:
> absolutePath.getBytes("UTF-8");
> 
> This could be done more efficiently(up to x20 time faster) with use of 
> java.nio.charset.StandardCharsets:
> absolutePath.getBytes(StandardCharsets.UTF_8);
> 
> The later variant also makes the code cleaner, as it is known not to throw 
> UnsupportedEncodingException in contrary to the former variant.
> 
> This change includes:
>  * demo/utils
>  * jdk.xx packages
>  * Some places were missed in the previous changes. I have found it by 
> tracing the calls to the Charset.forName() by executing tier1,2,3 and desktop 
> tests.
> 
> Some performance discussion: https://github.com/openjdk/jdk/pull/5063
> 
> Code excluded in this fix: the Xerces library(should be fixed upstream), 
> J2DBench(should be compatible to 1.4), some code in the network(the change 
> there are not straightforward, will do it later).
> 
> Tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS.

This pull request has now been integrated.

Changeset: 7fff22af
Author:Sergey Bylokhov 
URL:   
https://git.openjdk.java.net/jdk/commit/7fff22afe711c8c04dbf4cf5b4938d40632e4987
Stats: 364 lines in 53 files changed: 98 ins; 128 del; 138 mod

8272805: Avoid looking up standard charsets

Reviewed-by: weijun, naoto, dfuchs, azvegint, erikj

-

PR: https://git.openjdk.java.net/jdk/pull/5210

RFR: 8273101: Eliminate the usage of threadgroup sandboxing in the java.util.logging

[Sergey Bylokhov]

The "java.util.logging.LogManager" class uses the "threadgroup sandboxing" via 
an AppContext to support "applet logging isolation". The AppContext class 
became useless since the plugin and webstart are no longer supported and 
removed in jdk11.

This is the request to delete the usage of AppContext in the LogManager and 
related tests.

Tested by tier1/tier2/tier3 and jdk_desktop tests.

-

Commit messages:
 - Some tests deleted
 - Update the RootLevelInConfigFile test
 - Initial version of JDK-8273101

Changes: https://git.openjdk.java.net/jdk/pull/5326/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5326=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8273101
  Stats: 1423 lines in 10 files changed: 0 ins; 1418 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5326.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5326/head:pull/5326

PR: https://git.openjdk.java.net/jdk/pull/5326

Re: RFR: 8272805: Avoid looking up standard charsets [v4]

[Sergey Bylokhov]

> This is the continuation of JDK-8233884, JDK-8271456, and JDK-8272120.
> 
> In many places standard charsets are looked up via their names, for example:
> absolutePath.getBytes("UTF-8");
> 
> This could be done more efficiently(up to x20 time faster) with use of 
> java.nio.charset.StandardCharsets:
> absolutePath.getBytes(StandardCharsets.UTF_8);
> 
> The later variant also makes the code cleaner, as it is known not to throw 
> UnsupportedEncodingException in contrary to the former variant.
> 
> This change includes:
>  * demo/utils
>  * jdk.xx packages
>  * Some places were missed in the previous changes. I have found it by 
> tracing the calls to the Charset.forName() by executing tier1,2,3 and desktop 
> tests.
> 
> Some performance discussion: https://github.com/openjdk/jdk/pull/5063
> 
> Code excluded in this fix: the Xerces library(should be fixed upstream), 
> J2DBench(should be compatible to 1.4), some code in the network(the change 
> there are not straightforward, will do it later).
> 
> Tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS.

Sergey Bylokhov 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 16 additional commits 
since the last revision:

 - Merge branch 'master' into standard-encodings-in-non-public-modules
 - Merge branch 'master' into standard-encodings-in-non-public-modules
 - Update the usage of Files.readAllLines()
 - Update datatransfer
 - Merge branch 'master' into standard-encodings-in-non-public-modules
 - Merge branch 'master' into standard-encodings-in-non-public-modules
 - Fix related imports
 - Merge branch 'master' into standard-encodings-in-non-public-modules
 - Cleanup UnsupportedEncodingException
 - Update PacketStream.java
 - ... and 6 more: https://git.openjdk.java.net/jdk/compare/7289121a...7fe0327e

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5210/files
  - new: https://git.openjdk.java.net/jdk/pull/5210/files/79829ec8..7fe0327e

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=5210=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5210=02-03

  Stats: 949 lines in 30 files changed: 678 ins; 166 del; 105 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5210.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5210/head:pull/5210

PR: https://git.openjdk.java.net/jdk/pull/5210

Re: RFR: 8272805: Avoid looking up standard charsets [v3]

[Sergey Bylokhov]

> This is the continuation of JDK-8233884, JDK-8271456, and JDK-8272120.
> 
> In many places standard charsets are looked up via their names, for example:
> absolutePath.getBytes("UTF-8");
> 
> This could be done more efficiently(up to x20 time faster) with use of 
> java.nio.charset.StandardCharsets:
> absolutePath.getBytes(StandardCharsets.UTF_8);
> 
> The later variant also makes the code cleaner, as it is known not to throw 
> UnsupportedEncodingException in contrary to the former variant.
> 
> This change includes:
>  * demo/utils
>  * jdk.xx packages
>  * Some places were missed in the previous changes. I have found it by 
> tracing the calls to the Charset.forName() by executing tier1,2,3 and desktop 
> tests.
> 
> Some performance discussion: https://github.com/openjdk/jdk/pull/5063
> 
> Code excluded in this fix: the Xerces library(should be fixed upstream), 
> J2DBench(should be compatible to 1.4), some code in the network(the change 
> there are not straightforward, will do it later).
> 
> Tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS.

Sergey Bylokhov 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 15 additional commits 
since the last revision:

 - Merge branch 'master' into standard-encodings-in-non-public-modules
 - Update the usage of Files.readAllLines()
 - Update datatransfer
 - Merge branch 'master' into standard-encodings-in-non-public-modules
 - Merge branch 'master' into standard-encodings-in-non-public-modules
 - Fix related imports
 - Merge branch 'master' into standard-encodings-in-non-public-modules
 - Cleanup UnsupportedEncodingException
 - Update PacketStream.java
 - Rollback TextTests, should be compatible with jdk1.4
 - ... and 5 more: https://git.openjdk.java.net/jdk/compare/22865adf...79829ec8

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5210/files
  - new: https://git.openjdk.java.net/jdk/pull/5210/files/e7127644..79829ec8

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=5210=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5210=01-02

  Stats: 9423 lines in 339 files changed: 5247 ins; 1917 del; 2259 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5210.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5210/head:pull/5210

PR: https://git.openjdk.java.net/jdk/pull/5210

Re: RFR: 8272805: Avoid looking up standard charsets [v2]

[Sergey Bylokhov]

On Thu, 26 Aug 2021 17:39:36 GMT, Sergey Bylokhov  wrote:

>> Sergey Bylokhov 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 14 additional 
>> commits since the last revision:
>> 
>>  - Update the usage of Files.readAllLines()
>>  - Update datatransfer
>>  - Merge branch 'master' into standard-encodings-in-non-public-modules
>>  - Merge branch 'master' into standard-encodings-in-non-public-modules
>>  - Fix related imports
>>  - Merge branch 'master' into standard-encodings-in-non-public-modules
>>  - Cleanup UnsupportedEncodingException
>>  - Update PacketStream.java
>>  - Rollback TextTests, should be compatible with jdk1.4
>>  - Rollback TextRenderTests, should be compatible with jdk1.4
>>  - ... and 4 more: 
>> https://git.openjdk.java.net/jdk/compare/e5c71cd0...e7127644
>
> Can somebody take a look too the changes in the "jdk.attach", 
> "jdk.hotspot.agent" and IdealGraphVisualizer?

> @mrserb Not sure if it applies but there are couple of classes in `java.xml` 
> that use charset names instead of standard charsets. 
> would it make sense to go through them as well?

Most of the cases in the XML module are related to the Xerces library, I have 
skipped it to make the future merges from upstream of that library simpler.

-

PR: https://git.openjdk.java.net/jdk/pull/5210

Re: RFR: 8272805: Avoid looking up standard charsets [v2]

[Sergey Bylokhov]

On Sun, 22 Aug 2021 23:02:06 GMT, Sergey Bylokhov  wrote:

>> This is the continuation of JDK-8233884, JDK-8271456, and JDK-8272120.
>> 
>> In many places standard charsets are looked up via their names, for example:
>> absolutePath.getBytes("UTF-8");
>> 
>> This could be done more efficiently(up to x20 time faster) with use of 
>> java.nio.charset.StandardCharsets:
>> absolutePath.getBytes(StandardCharsets.UTF_8);
>> 
>> The later variant also makes the code cleaner, as it is known not to throw 
>> UnsupportedEncodingException in contrary to the former variant.
>> 
>> This change includes:
>>  * demo/utils
>>  * jdk.xx packages
>>  * Some places were missed in the previous changes. I have found it by 
>> tracing the calls to the Charset.forName() by executing tier1,2,3 and 
>> desktop tests.
>> 
>> Some performance discussion: https://github.com/openjdk/jdk/pull/5063
>> 
>> Code excluded in this fix: the Xerces library(should be fixed upstream), 
>> J2DBench(should be compatible to 1.4), some code in the network(the change 
>> there are not straightforward, will do it later).
>> 
>> Tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS.
>
> Sergey Bylokhov 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 14 additional 
> commits since the last revision:
> 
>  - Update the usage of Files.readAllLines()
>  - Update datatransfer
>  - Merge branch 'master' into standard-encodings-in-non-public-modules
>  - Merge branch 'master' into standard-encodings-in-non-public-modules
>  - Fix related imports
>  - Merge branch 'master' into standard-encodings-in-non-public-modules
>  - Cleanup UnsupportedEncodingException
>  - Update PacketStream.java
>  - Rollback TextTests, should be compatible with jdk1.4
>  - Rollback TextRenderTests, should be compatible with jdk1.4
>  - ... and 4 more: 
> https://git.openjdk.java.net/jdk/compare/be695a9b...e7127644

Can somebody take a look too the changes in the "jdk.attach", 
"jdk.hotspot.agent" and IdealGraphVisualizer?

-

PR: https://git.openjdk.java.net/jdk/pull/5210

Re: RFR: 8272863: Replace usages of Collections.sort with List.sort call in public java modules

[Sergey Bylokhov]

On Mon, 23 Aug 2021 21:01:48 GMT, Andrey Turbanov 
 wrote:

> Collections.sort is just a wrapper, so it is better to use an instance method 
> directly.

The changes in the src/java.desktop/ looks fine.

Filed: https://bugs.openjdk.java.net/browse/JDK-8272863

-

Marked as reviewed by serb (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5229

Re: RFR: 8272805: Avoid looking up standard charsets [v2]

[Sergey Bylokhov]

On Sun, 22 Aug 2021 15:09:26 GMT, Alan Bateman  wrote:

>> Sergey Bylokhov 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 14 additional 
>> commits since the last revision:
>> 
>>  - Update the usage of Files.readAllLines()
>>  - Update datatransfer
>>  - Merge branch 'master' into standard-encodings-in-non-public-modules
>>  - Merge branch 'master' into standard-encodings-in-non-public-modules
>>  - Fix related imports
>>  - Merge branch 'master' into standard-encodings-in-non-public-modules
>>  - Cleanup UnsupportedEncodingException
>>  - Update PacketStream.java
>>  - Rollback TextTests, should be compatible with jdk1.4
>>  - Rollback TextRenderTests, should be compatible with jdk1.4
>>  - ... and 4 more: 
>> https://git.openjdk.java.net/jdk/compare/465eb90c...e7127644
>
> src/jdk.attach/linux/classes/sun/tools/attach/VirtualMachineImpl.java line 
> 342:
> 
>> 340: 
>> 341: try {
>> 342: for (String line : Files.readAllLines(statusPath, UTF_8)) {
> 
> The 1-arg readAllLines is specified to use UTF-8 so you can drop the second 
> parameter here if you want.

Thank you for the suggestion, I have fixed this and a few other places as well.

-

PR: https://git.openjdk.java.net/jdk/pull/5210

Re: RFR: 8272805: Avoid looking up standard charsets

[Sergey Bylokhov]

On Sun, 22 Aug 2021 18:31:02 GMT, Andrey Turbanov 
 wrote:

> I think it's worth to update _static_ initializer in 
> `sun.datatransfer.DataFlavorUtil.CharsetComparator` too.

Updated as suggested.

-

PR: https://git.openjdk.java.net/jdk/pull/5210

Re: RFR: 8272805: Avoid looking up standard charsets [v2]

[Sergey Bylokhov]

> This is the continuation of JDK-8233884, JDK-8271456, and JDK-8272120.
> 
> In many places standard charsets are looked up via their names, for example:
> absolutePath.getBytes("UTF-8");
> 
> This could be done more efficiently(up to x20 time faster) with use of 
> java.nio.charset.StandardCharsets:
> absolutePath.getBytes(StandardCharsets.UTF_8);
> 
> The later variant also makes the code cleaner, as it is known not to throw 
> UnsupportedEncodingException in contrary to the former variant.
> 
> This change includes:
>  * demo/utils
>  * jdk.xx packages
>  * Some places were missed in the previous changes. I have found it by 
> tracing the calls to the Charset.forName() by executing tier1,2,3 and desktop 
> tests.
> 
> Some performance discussion: https://github.com/openjdk/jdk/pull/5063
> 
> Code excluded in this fix: the Xerces library(should be fixed upstream), 
> J2DBench(should be compatible to 1.4), some code in the network(the change 
> there are not straightforward, will do it later).
> 
> Tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS.

Sergey Bylokhov 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 14 additional commits 
since the last revision:

 - Update the usage of Files.readAllLines()
 - Update datatransfer
 - Merge branch 'master' into standard-encodings-in-non-public-modules
 - Merge branch 'master' into standard-encodings-in-non-public-modules
 - Fix related imports
 - Merge branch 'master' into standard-encodings-in-non-public-modules
 - Cleanup UnsupportedEncodingException
 - Update PacketStream.java
 - Rollback TextTests, should be compatible with jdk1.4
 - Rollback TextRenderTests, should be compatible with jdk1.4
 - ... and 4 more: https://git.openjdk.java.net/jdk/compare/37357100...e7127644

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/5210/files
  - new: https://git.openjdk.java.net/jdk/pull/5210/files/2d9c80b8..e7127644

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=5210=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=5210=00-01

  Stats: 3598 lines in 210 files changed: 2055 ins; 1115 del; 428 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5210.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5210/head:pull/5210

PR: https://git.openjdk.java.net/jdk/pull/5210

RFR: 8272805: Avoid looking up standard charsets

[Sergey Bylokhov]

This is the continuation of JDK-8233884, JDK-8271456, and JDK-8272120.
See https://github.com/openjdk/jdk/pull/5063 and 
https://github.com/openjdk/jdk/pull/4951

In many places standard charsets are looked up via their names, for example:
absolutePath.getBytes("UTF-8");

This could be done more efficiently(up to x20 time faster) with use of 
java.nio.charset.StandardCharsets:
absolutePath.getBytes(StandardCharsets.UTF_8);

The later variant also makes the code cleaner, as it is known not to throw 
UnsupportedEncodingException in contrary to the former variant.

This change includes:
 * demo/utils
 * jdk.xx packages
 * Some places were missed in the previous changes. I have found it by tracing 
the calls to the Charset.forName() by executing tier1,2,3 and desktop tests. 
Also checked for "aliases" usage.

Some performance discussion: https://github.com/openjdk/jdk/pull/5063

Code excluded in this fix: the Xerces library(should be fixed upstream), 
J2DBench(should be compatible to 1.4), some code in the "java.naming"(the 
change there is not straightforward, will do it later).

Tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS.

-

Commit messages:
 - Fix related imports
 - Merge branch 'master' into standard-encodings-in-non-public-modules
 - Cleanup UnsupportedEncodingException
 - Update PacketStream.java
 - Rollback TextTests, should be compatible with jdk1.4
 - Rollback TextRenderTests, should be compatible with jdk1.4
 - Cleanup the UnsupportedEncodingException
 - aliases for ISO_8859_1
 - Update imageio
 - Initial fix

Changes: https://git.openjdk.java.net/jdk/pull/5210/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5210=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8272805
  Stats: 333 lines in 48 files changed: 91 ins; 123 del; 119 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5210.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5210/head:pull/5210

PR: https://git.openjdk.java.net/jdk/pull/5210

Integrated: 8272120: Avoid looking for standard encodings in "java." modules

[Sergey Bylokhov]

On Tue, 10 Aug 2021 05:08:54 GMT, Sergey Bylokhov  wrote:

> This is the continuation of JDK-8233884 and JDK-8271456. This change affects 
> fewer cases so I fix all "java." modules at once.
> 
> In many places standard charsets are looked up via their names, for example:
> absolutePath.getBytes("UTF-8");
> 
> This could be done more efficiently(up to x20 time faster) with use of 
> java.nio.charset.StandardCharsets:
> absolutePath.getBytes(StandardCharsets.UTF_8);
> 
> The later variant also makes the code cleaner, as it is known not to throw 
> UnsupportedEncodingException in contrary to the former variant.
> 
> tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS.

This pull request has now been integrated.

Changeset: ec2fc384
Author:Sergey Bylokhov 
URL:   
https://git.openjdk.java.net/jdk/commit/ec2fc384e50668b667335f973ffeb5a19bbcfb9b
Stats: 127 lines in 15 files changed: 24 ins; 53 del; 50 mod

8272120: Avoid looking for standard encodings in "java." modules

Reviewed-by: alanb, dfuchs, naoto

-

PR: https://git.openjdk.java.net/jdk/pull/5063

Re: RFR: 8272120: Avoid looking for standard encodings in "java." modules

[Sergey Bylokhov]

On Tue, 10 Aug 2021 09:18:39 GMT, Alan Bateman  wrote:

> It would be useful to get up to date performance data on using Charset vs. 
> charset name. At least historically, the charset name versions were faster 
> (see [JDK-6764325](https://bugs.openjdk.java.net/browse/JDK-6764325)).

The code in question was changed a few times since then, the last change was 
done by the https://github.com/openjdk/jdk/pull/2102. So currently the code for 
string.getBytes String/Charset uses the same code paths, except that the string 
version has an additional call to lookup the charset.
The string version:
https://github.com/openjdk/jdk/blob/66d1faa7847b645f20ab2e966adf0a523e3ffeb2/src/java.base/share/classes/java/lang/String.java#L1753
The charset version:
https://github.com/openjdk/jdk/blob/66d1faa7847b645f20ab2e966adf0a523e3ffeb2/src/java.base/share/classes/java/lang/String.java#L1777

I checked the performance and the charset is always faster, depending on the 
string size, up to x20.

@cl4es please confirm my observation since you did it already for 
https://github.com/openjdk/jdk/pull/2102

-

PR: https://git.openjdk.java.net/jdk/pull/5063

RFR: 8272120: Avoid looking for standard encodings in "java." modules

[Sergey Bylokhov]

This is the continuation of JDK-8233884 and JDK-8271456. This change affects 
fewer cases so I fix all "java." modules at once.

In many places standard charsets are looked up via their names, for example:
absolutePath.getBytes("UTF-8");

This could be done more efficiently(up to x20 time faster) with use of 
java.nio.charset.StandardCharsets:
absolutePath.getBytes(StandardCharsets.UTF_8);

The later variant also makes the code cleaner, as it is known not to throw 
UnsupportedEncodingException in contrary to the former variant.

tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS.

-

Commit messages:
 - Initial fix of JDK-8272120

Changes: https://git.openjdk.java.net/jdk/pull/5063/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5063=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8272120
  Stats: 127 lines in 15 files changed: 24 ins; 53 del; 50 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5063.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5063/head:pull/5063

PR: https://git.openjdk.java.net/jdk/pull/5063

Re: RFR: 8269130: Replace usages of Collection.toArray() with Collection.toArray(T[]) to avoid redundant array copying

[Sergey Bylokhov]

On Mon, 14 Jun 2021 17:00:29 GMT, Andrey Turbanov 
 wrote:

> I found few places, where code initially perform `Object[] 
> Colleciton.toArray()` call and then manually copy array into another array 
> with required type.
> This PR cleanups such places to more shorter call `T[] 
> Collection.toArray(T[])`.

Changes in the desktop module looks fine.

-

Marked as reviewed by serb (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/4487

Re: RFR: 8269130: Replace usages of Collection.toArray() with Collection.toArray(T[]) to avoid redundant array copying

[Sergey Bylokhov]

On Mon, 14 Jun 2021 17:00:29 GMT, Andrey Turbanov 
 wrote:

> I found few places, where code initially perform `Object[] 
> Colleciton.toArray()` call and then manually copy array into another array 
> with required type.
> This PR cleanups such places to more shorter call `T[] 
> Collection.toArray(T[])`.

src/java.desktop/share/classes/sun/java2d/SunGraphicsEnvironment.java line 191:

> 189: installed[i]);
> 190: }
> 191: String[] retval = map.keySet().toArray(new String[0]);

Looks like previously the code returns values, and now it will return keys, 
please recheck.

-

PR: https://git.openjdk.java.net/jdk/pull/4487

Re: RFR: 8080272 Refactor I/O stream copying to use java.io.InputStream.transferTo

[Sergey Bylokhov]

On Sun, 20 Dec 2020 20:33:43 GMT, Phil Race  wrote:

>>> One or two of the sources changes should probably uses Files.copy, e.g. 
>>> ZipPath, sjavac/CopyFile.java.
>> 
>> Good idea! Replaced in few places. But not in ZipPath: it's actually 
>> implementation of underlying call from Files.copy: 
>> `jdk.nio.zipfs.ZipFileSystemProvider#copy`. So, `Files.copy` call will be 
>> recursive.
>
> So these changes are all over the place which means no one person knows how 
> to test all of it.
> Have you run the sound, swing tests, and the  printing tests on unix and 
> windows ?
> For printing for sure you'll need to do some manual testing.

I already suggested this, but anyway please always extract the changes to the 
java.desktop module to the separate PR.

-

PR: https://git.openjdk.java.net/jdk/pull/1853

Re: RFR: 8254162: Implementation of Foreign-Memory Access API (Third Incubator) [v20]

[Sergey Bylokhov]

On Mon, 2 Nov 2020 11:59:09 GMT, Maurizio Cimadamore  
wrote:

>> This patch contains the changes associated with the third incubation round 
>> of the foreign memory access API incubation  (see JEP 393 [1]). This 
>> iteration focus on improving the usability of the API in 3 main ways:
>> 
>> * first, by providing a way to obtain truly *shared* segments, which can be 
>> accessed and closed concurrently from multiple threads
>> * second, by providing a way to register a memory segment against a 
>> `Cleaner`, so as to have some (optional) guarantee that the memory will be 
>> deallocated, eventually
>> * third, by not requiring users to dive deep into var handles when they 
>> first pick up the API; a new `MemoryAccess` class has been added, which 
>> defines several useful dereference routines; these are really just thin 
>> wrappers around memory access var handles, but they make the barrier of 
>> entry for using this API somewhat lower.
>> 
>> A big conceptual shift that comes with this API refresh is that the role of 
>> `MemorySegment` and `MemoryAddress` is not the same as it used to be; it 
>> used to be the case that a memory address could (sometimes, not always) have 
>> a back link to the memory segment which originated it; additionally, memory 
>> access var handles used `MemoryAddress` as a basic unit of dereference.
>> 
>> This has all changed as per this API refresh;  now a `MemoryAddress` is just 
>> a dumb carrier which wraps a pair of object/long addressing coordinates; 
>> `MemorySegment` has become the star of the show, as far as dereferencing 
>> memory is concerned. You cannot dereference memory if you don't have a 
>> segment. This improves usability in a number of ways - first, it is a lot 
>> easier to wrap native addresses (`long`, essentially) into a 
>> `MemoryAddress`; secondly, it is crystal clear what a client has to do in 
>> order to dereference memory: if a client has a segment, it can use that; 
>> otherwise, if the client only has an address, it will have to create a 
>> segment *unsafely* (this can be done by calling 
>> `MemoryAddress::asSegmentRestricted`).
>> 
>> A list of the API, implementation and test changes is provided below. If  
>> you have any questions, or need more detailed explanations, I (and the  rest 
>> of the Panama team) will be happy to point at existing discussions,  and/or 
>> to provide the feedback required. 
>> 
>> A big thank to Erik Osterlund, Vladimir Ivanov and David Holmes, without 
>> whom the work on shared memory segment would not have been possible; also 
>> I'd like to thank Paul Sandoz, whose insights on API design have been very 
>> helpful in this journey.
>> 
>> Thanks 
>> Maurizio 
>> 
>> Javadoc: 
>> 
>> http://cr.openjdk.java.net/~mcimadamore/8254162_v1/javadoc/jdk/incubator/foreign/package-summary.html
>> 
>> Specdiff: 
>> 
>> http://cr.openjdk.java.net/~mcimadamore/8254162_v1/specdiff/jdk/incubator/foreign/package-summary.html
>> 
>> CSR: 
>> 
>> https://bugs.openjdk.java.net/browse/JDK-8254163
>> 
>> 
>> 
>> ### API Changes
>> 
>> * `MemorySegment`
>>   * drop factory for restricted segment (this has been moved to 
>> `MemoryAddress`, see below)
>>   * added a no-arg factory for a native restricted segment representing 
>> entire native heap
>>   * rename `withOwnerThread` to `handoff`
>>   * add new `share` method, to create shared segments
>>   * add new `registerCleaner` method, to register a segment against a cleaner
>>   * add more helpers to create arrays from a segment e.g. `toIntArray`
>>   * add some `asSlice` overloads (to make up for the fact that now segments 
>> are more frequently used as cursors)
>>   * rename `baseAddress` to `address` (so that `MemorySegment` can implement 
>> `Addressable`)
>> * `MemoryAddress`
>>   * drop `segment` accessor
>>   * drop `rebase` method and replace it with `segmentOffset` which returns 
>> the offset (a `long`) of this address relative to a given segment
>> * `MemoryAccess`
>>   * New class supporting several static dereference helpers; the helpers are 
>> organized by carrier and access mode, where a carrier is one of the usual 
>> suspect (a Java primitive, minus `boolean`); the access mode can be simple 
>> (e.g. access base address of given segment), or indexed, in which case the 
>> accessor takes a segment and either a low-level byte offset,or a high level 
>> logical index. The classification is reflected in the naming scheme (e.g. 
>> `getByte` vs. `getByteAtOffset` vs `getByteAtIndex`).
>> * `MemoryHandles`
>>   * drop `withOffset` combinator
>>   * drop `withStride` combinator
>>   * the basic memory access handle factory now returns a var handle which 
>> takes a `MemorySegment` and a `long` - from which it is easy to derive all 
>> the other handles using plain var handle combinators.
>> * `Addressable`
>>   * This is a new interface which is attached to entities which can be 
>> projected to a `MemoryAddress`. For now, both `MemoryAddress` and 
>> 

Re: RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects [v2]

[Sergey Bylokhov]

On Wed, 28 Oct 2020 08:40:02 GMT, Сергей Цыпанов 
 wrote:

>> client changes are fine
>
> Rebased onto master to have the fix introduced in 
> https://github.com/openjdk/jdk/pull/778

FYI it is better to use merge, instead of rebase+force push. Rebase breaks 
history and all existed code comments.

-

PR: https://git.openjdk.java.net/jdk/pull/818

Re: RFR: 8255299: Drop explicit zeroing at instantiation of Atomic* objects

[Sergey Bylokhov]

On Thu, 22 Oct 2020 20:46:15 GMT, Сергей Цыпанов 
 wrote:

> As discussed in https://github.com/openjdk/jdk/pull/510 there is never a 
> reason to explicitly instantiate any instance of `Atomic*` class with its 
> default value, i.e. `new AtomicInteger(0)` could be replaced with `new 
> AtomicInteger()` which is faster:
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> @BenchmarkMode(value = Mode.AverageTime)
> public class AtomicBenchmark {
>   @Benchmark
>   public Object defaultValue() {
> return new AtomicInteger();
>   }
>   @Benchmark
>   public Object explicitValue() {
> return new AtomicInteger(0);
>   }
> }
> THis benchmark demonstrates that `explicitValue()` is much slower:
> Benchmark  Mode  Cnt   Score   Error  Units
> AtomicBenchmark.defaultValue   avgt   30   4.778 ± 0.403  ns/op
> AtomicBenchmark.explicitValue  avgt   30  11.846 ± 0.273  ns/op
> So meanwhile https://bugs.openjdk.java.net/browse/JDK-8145948 is still in 
> progress we could trivially replace explicit zeroing with default 
> constructors gaining some performance benefit with no risk.
> 
> I've tested the changes locally, both tier1 and tier 2 are ok. 
> 
> Could one create an issue for tracking this?

The changes in src/java.desktop looks fine.

-

Marked as reviewed by serb (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/818

Re: RFR: 8252999: replace all String.equals("") usages with String.isEmpty()

[Sergey Bylokhov]

On Sun, 6 Sep 2020 13:57:19 GMT, Dmitriy Dumanskiy 
 wrote:

> **isEmpty** is faster + has less byte code + easier to read. Benchmarks could 
> be found
>   
> [here](https://medium.com/javarevisited/micro-optimizations-in-java-string-equals-22be19fd8416).

src/demo/share/java2d/J2DBench/src/j2dbench/tests/iio/InputImageTests.java line 
187:

> 185: String format = spi.getFormatNames()[0].toLowerCase();
> 186: String suffix = spi.getFileSuffixes()[0].toLowerCase();
> 187: if (suffix == null || suffix.equals("")) {

This file intentionally maintains compatibility to jdk1.4, so equals("") should 
be used.

src/demo/share/java2d/J2DBench/src/j2dbench/tests/iio/OutputImageTests.java 
line 146:

> 144: String format = spi.getFormatNames()[0].toLowerCase();
> 145: String suffix = spi.getFileSuffixes()[0].toLowerCase();
> 146: if (suffix == null || suffix.equals("")) {

This file intentionally maintains compatibility to jdk1.4, so equals("") should 
be used.

-

PR: https://git.openjdk.java.net/jdk/pull/29

Re: Fwd: Re: RFR (12): 8191053: Provide a mechanism to make system's security manager immutable

[Sergey Bylokhov]

Hi, Sean.
One question related to SecurityManager and performance, is it possible 
to provide a special version of AccessController.doPrivileged which will 
be noop if SecurityManager is not present?


On 03/10/2018 13:12, Sean Mullan wrote:
For those of you that are not also subscribed to security-dev, this is 
mostly FYI, as the review is winding down, but if you have any comments, 
let me know.


This change will add new token options ("allow" and "disallow") to the 
java.security.manager system property. The "disallow" option is intended 
to provide a potential performance boost for applications that don't 
enable a SecurityManager, as there is a cost associated with allowing a 
SecurityManager to enabled at runtime, even if it is never enabled. The 
CSR provides a good summary of the issue and spec changes: 
https://bugs.openjdk.java.net/browse/JDK-8203316


Thanks,
Sean

 Forwarded Message 
Subject: Re: RFR (12): 8191053: Provide a mechanism to make system's 
security manager immutable

Date: Tue, 2 Oct 2018 11:34:09 -0400
From: Sean Mullan 
Organization: Oracle Corporation
To: security Dev OpenJDK 

Hello,

Thanks for all the comments so far, and the interesting discussions 
about the future of the SecurityManager. We will definitely return to 
those discussions in the near future, but for now I have a second webrev 
ready for review for this enhancement:


http://cr.openjdk.java.net/~mullan/webrevs/8191053/webrev.01/

The main changes since the initial revision are:

1. System.setSecurityManager is no longer deprecated. This type of 
change clearly needs more discussion and is not an essential part of 
this RFE.


2. After further thought, I took Bernd's suggestion [1] and instead of 
adding a new property to disallow the setting of a SecurityManager at 
runtime, have added new tokens to the existing "java.security.manager" 
system property, named "=disallow", and "=allow" to toggle this 
behavior. The "=" is to avoid any potential clashes with custom SM 
classes with those names. I think this is a cleaner approach. There are 
a couple of new paragraphs in the SecurityManager class description 
describing the "java.security.manager" property and how the new tokens 
work.


3. I also added a comment that Bernd had requested [2] on why 
System.setSecurityManager calls checkPackageAccess("java.lang").


Also, the CSR has been updated: 
https://bugs.openjdk.java.net/browse/JDK-8203316


Thanks,
Sean

[1] 
http://mail.openjdk.java.net/pipermail/security-dev/2018-September/018217.html 

[2] 
http://mail.openjdk.java.net/pipermail/security-dev/2018-September/018193.html 



On 9/13/18 4:02 PM, Sean Mullan wrote:
This is a SecurityManager related change which warrants some 
additional details for its motivation.


The current System.setSecurityManager() API allows a SecurityManager 
to be set at run-time. However, because of this mutability, it incurs 
a performance overhead even for applications that never call it and do 
not enable a SecurityManager dynamically, which is probably the 
majority of applications.


For example, there are lots of "SecurityManager sm = 
System.getSecurityManager(); if (sm != null) ..." checks in the JDK. 
If it was known that a SecurityManager could never be set at run-time, 
these checks could be optimized using constant-folding.


There are essentially two main parts to this change:

1. Deprecation of System.securityManager()

Going forward, we want to discourage applications from calling 
System.setSecurityManager(). Instead they should enable a 
SecurityManager using the java.security.manager system property on the 
command-line.


2. A new JDK-specific system property to disallow the setting of the 
security manager at run-time: jdk.allowSecurityManager


If set to false, it allows the run-time to optimize the code and 
improve performance when it is known that an application will never 
run with a SecurityManager. To support this behavior, the 
System.setSecurityManager() API has been updated such that it can 
throw an UnsupportedOperationException if it does not allow a security 
manager to be set dynamically.


webrev: http://cr.openjdk.java.net/~mullan/webrevs/8191053/webrev.00/
CSR: https://bugs.openjdk.java.net/browse/JDK-8203316
JBS: https://bugs.openjdk.java.net/browse/JDK-8191053

(I will likely also send this to core-libs for additional review later)

--Sean



--
Best regards, Sergey.

Re: JDK 12 RFR of JDK-8209024: Use SuppressWarnings on serialVersionUID fields in interfaces

[Sergey Bylokhov]

Hi, Joe.

On 06/08/2018 14:30, joe darcy wrote:
Even if currently less commonly used, I think "ineffectual" better 
captures the intention of what I want to convey in the comment than 
"ineffective."


Can you please clarify this: what does it mean "ineffectual" in this 
context? why we need to "suppress" them and why these fields cannot be 
dropped?




--
Best regards, Sergey.

Re: RFR(M): 8170525: Fix minor issues in awt coding

[Sergey Bylokhov]

cc 2d-dev.

On 30.11.16 18:41, Lindenmaier, Goetz wrote:

Hi Vincent,

thanks for the quit review!
Good catch that I lost the change to p11_mutex.c ... I had to change
it and it fell out of my patches.
I edited the Last Modified Date, and also updated the copyright messages.
New webrev:
http://cr.openjdk.java.net/~goetz/wr16/8170525-awt-dev/

Best regards,
  Goetz.

(Am I correct that your openJdk name is Vinnie?)


-Original Message-
From: Vincent Ryan [mailto:vincent.x.r...@oracle.com]
Sent: Mittwoch, 30. November 2016 14:53
To: Lindenmaier, Goetz 
Cc: awt-...@openjdk.java.net; security-dev@openjdk.java.net
Subject: Re: RFR(M): 8170525: Fix minor issues in awt coding

Hello Goetz,

Please modify the bug summary to reference ECC too.
Your ECC changes look fine but the ‘Last Modified Date’ line in the 4 source
code headers will need to be updated/added.

BTW p11_mutex.c is listed below but appears to be missing from the webrev.

Thanks.




On 30 Nov 2016, at 13:12, Lindenmaier, Goetz
 > wrote:

Hi,

I’d like to propose a row of smaller fixes where code is noted down a
bit questionable.
SAP’s quality process requires that we fix these in our internal 
delivery,
and I
Would like to share my fixes with openJdk.  Some of these fixes are of
more
theoretical nature as how I understand the code paths never allow the
problematic situation, but fixing it nevertheless assures that nothing 
is
overseen if the code changes.  Most changes are in  libawt_xawt, some
are in libsunec.

I’d appreciate a review:
http://cr.openjdk.java.net/~goetz/wr16/8170525-awt/webrev.01/

Changes in detail:

awt_InputMethod.c:

One might overrun the 100 byte fixed-size string statusWindow->status
by copying text->string.multi_byte without checking the length.

gtk3_interface.c:

This less-than-zero comparison of an unsigned value is never true.

Using uninitialized value color. Field color.alpha is uninitialized.
E.g. used at gtk3_interface.c:2287.

XToolkit.c

Using uninitialized value ret_timeout.
E.g. in XToolkit.c:6809.

XWindow.c

Argument is incompatible with corresponding format string conversion.

splashscreen_sys.c

Overflowed or truncated value (or a value computed from an
overflowed or truncated value) (gdk_scale > 0) ? native_scale *
(double)gdk_scale : native_scale used as return value.

ec.c

Using uninitialized value k.dp when calling mp_clear.

ecdecode.c

You might overrun the 291 byte fixed-size string genenc by copying
curveParams->geny without checking the length.
Added sanity check before doing the string concatenation.

ecl_mult.c

Using uninitialized value kt.flag when calling *group->point_mul. (The
function pointer resolves to ec_GF2m_pt_mul_mont.)

mpi.c

Using uninitialized value s. Field s.flag is uninitialized when calling
s_mp_exch.
Using uninitialized value tmp. Field tmp.flag is uninitialized when
calling s_mp_exch
Using uninitialized value t.dp when calling mp_clear.

p11_mutex.c

Using uninitialized value *ckpInitArgs. Field ckpInitArgs->flags is
uninitialized when calling memcpy.


DataBufferNative.c

Using uninitialized value lockInfo.rasBase when calling
BN_GetPixelPointer.

fontpath.c

You might overrun the 512 byte fixed-size string fontDirPath by copying
DirP->name[index] without checking the length.






--
Best regards, Sergey.

Re: JDK 9 RFR of JDK-8151763; Use more informative format for problem list

[Sergey Bylokhov]

The fix looks fine to me. can you please clarify what "enabling better 
reporting" from the bug description means? Where this information will 
be reported?


On 31.03.16 2:48, Joseph D. Darcy wrote:

Hi Mandy,

Hopefully the third time will be the charm for this changeset after your
correction to the commented-out test:

 http://cr.openjdk.java.net/~darcy/8151763.2

I aligned the bug number in column 64 unless the test name took more
characters. (This isn't as evident in the webrev since the tab expansion
is different than in a text editor.)

Thanks,

-Joe

On 3/29/2016 12:31 PM, Mandy Chung wrote:

On Mar 29, 2016, at 12:15 PM, joe darcy  wrote:

Hi Mandy,

On 3/28/2016 8:48 PM, Mandy Chung wrote:

On Mar 28, 2016, at 5:03 PM, Joseph D. Darcy 
wrote:

Hello,

New iteration of the webrev updated after the Jigsaw integration
and incorporating the earlier feedback.

http://cr.openjdk.java.net/~darcy/8151763.1


# tools/jimage/JImageTest.java
linux-i586,windows-i586

Is this test accidentally removed?  Other than this, looks okay.

The "#" lines are comments so I was removing a commented out line. (I
assumed, but did not verify, the line for this test was a leftover
artifact of the recent Jigsaw merge.)

I missed “#” since this test should be excluded (some error might have
been creeped in before the integration)

This test needs to be added back in the problem list.  I’ll create a
changeset.


Nit: it’d be good to have most of bug ids aligned in the same column
start.
Here are a few ones:

  210 sun/security/krb5/auto/Unreachable.java 7164518 macosx-all
no PortUnreachableException on Mac
  212 java/security/KeyPairGenerator/SolarisShortDSA.java 7041639
solaris-all Solaris DSA keypair generation bug
  213 sun/security/tools/keytool/standard.sh  7041639
solaris-all Solaris DSA keypair generation bug
  346 java/util/Arrays/ParallelPrefix.java 8080165,8085982 generic-all
  348 java/util/BitSet/BitSetStreamTest.java 8079538 generic-all
  360 sun/tools/jmap/heapconfig/JMapHeapConfigTest.java
8072131,8132452 generic-all
  370 sun/tools/jinfo/JInfoSanityTest.java
8059035 generic-all



I was trying to avoid introducing lots of spacing changes in an
attempt to make the patch easier to review, but I can look over these
cases again.

That’d be good.  Thanks
Mandy





--
Best regards, Sergey.

Re: JDK 9 RFR of JDK-8151763; Use more informative format for problem list

[Sergey Bylokhov]

Looks fine.

On 12.03.16 6:28, joe darcy wrote:

Hello,

As Jon Gibbons has noted off-list, the problem list entries can directly
include the bug number associated with the test in question, enabling
better reporting. This format should be used rather than the current
convention of putting the bug number in a comment.

Please review the webrev to adopt the revised format for the problem list:

 http://cr.openjdk.java.net/~darcy/8151763.0/

I've verified jtreg produces the same test list with the old and new
versions of the problem list.

Thanks,

-Joe



--
Best regards, Sergey.

Re: AWT Dev RfR JDK-8051626 Rework security restrictions of Java Access Bridge and related Utilities

[Sergey Bylokhov]

The fix looks fine.

On 16.07.15 0:42, Pete Brunet wrote:

An update is available and mostly changes only the test case,
Bug8151626.java.  The other change is to remove jtreg.security.policy.

http://cr.openjdk.java.net/~ptbrunet/JDK-8051626/webrev.01/

Changes:

 From Sean
- The jtreg @run statement was removed; don't specify security manager
or security policy.
- Remove jtreg.security.policy
- Add System.setSecurityManager(new SecurityManager()); to the beginning
of the code.

 From Sergey
- Wrap test in invokeAndWait
- Add frame.dispose in finally
- Remove System.exit

I also removed the Thread.sleep.  It doesn't appear to be needed.

Pete

On 7/14/15 5:00 AM, Sergey Bylokhov wrote:

Hi, Pete.
The fix looks fine, but you should tweak the test a little bit.
  - You access the swing components on non-EDT thread.
  - You should not use System.exit in the test.
  - The JFrame should be disposed before the end of the test.

On 14.07.15 1:34, Pete Brunet wrote:

Please review the webrev for
https://bugs.openjdk.java.net/browse/JDK-8051626

http://cr.openjdk.java.net/~ptbrunet/JDK-8051626/webrev.00/

This is so the the Java Accessibility Utilities package,
com.sun.java.accessibility.util, can be run with the security manager
active but the non-public accessibility packages won't, i.e.
com.sun.java.accessibility.internal and
com.sun.java.accessibility.util.internal.

Running the regression test proves that there will be a security
exception when using a method of com.sun.java.accessibility.util before
the fix but not after.

Pete





--
Best regards, Sergey.

Re: AWT Dev RfR JDK-8051626 Rework security restrictions of Java Access Bridge and related Utilities

[Sergey Bylokhov]

Hi, Pete.
The fix looks fine, but you should tweak the test a little bit.
 - You access the swing components on non-EDT thread.
 - You should not use System.exit in the test.
 - The JFrame should be disposed before the end of the test.

On 14.07.15 1:34, Pete Brunet wrote:

Please review the webrev for
https://bugs.openjdk.java.net/browse/JDK-8051626

http://cr.openjdk.java.net/~ptbrunet/JDK-8051626/webrev.00/

This is so the the Java Accessibility Utilities package,
com.sun.java.accessibility.util, can be run with the security manager
active but the non-public accessibility packages won't, i.e.
com.sun.java.accessibility.internal and
com.sun.java.accessibility.util.internal.

Running the regression test proves that there will be a security
exception when using a method of com.sun.java.accessibility.util before
the fix but not after.

Pete



--
Best regards, Sergey.