Re: [OpenJDK 2D-Dev] RFR: 8272805: Avoid looking up standard charsets [v2]
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/aaa7beaf...e7127644
Marked as reviewed by turban...@github.com (no known OpenJDK username).
-
PR: https://git.openjdk.java.net/jdk/pull/5210
[OpenJDK 2D-Dev] Integrated: 8269223: -Xcheck:jni WARNINGs working with fonts on Linux
On Wed, 23 Jun 2021 09:19:16 GMT, Maxim Kartashev wrote: > Added an `ExceptionCheck()` followed by `ExceptionDescribe()` and > `ExceptionClear()` immediately after the Java calls made from the callback > function `ReadTTFontFileFunc()` in `freetypeScaler.c`. > > The exception(s) need to be cleared because we're not returning immediately > to Java that would've been able to handle them gracefully. And in order not > to loose the exception entirely (even though the return value would also > indicate an error condition), print out the exception with > `ExceptionDescribe()` to aid in debugging. This pull request has now been integrated. Changeset: 9bc02322 Author:Maxim Kartashev Committer: Sergey Bylokhov URL: https://git.openjdk.java.net/jdk/commit/9bc023220fbbb0b6ea1ed1a0ca2aa3848764e8cd Stats: 105 lines in 5 files changed: 98 ins; 0 del; 7 mod 8269223: -Xcheck:jni WARNINGs working with fonts on Linux Reviewed-by: prr, serb - PR: https://git.openjdk.java.net/jdk/pull/4572
[OpenJDK 2D-Dev] RFR: 8262751: RenderPipelineState assertion error in J2DDemo
Provide correct pipeline state for MTLPaint after reset - Commit messages: - 8262751: RenderPipelineState assertion error in J2DDemo Changes: https://git.openjdk.java.net/jdk/pull/5227/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5227=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8262751 Stats: 61 lines in 2 files changed: 51 ins; 2 del; 8 mod Patch: https://git.openjdk.java.net/jdk/pull/5227.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/5227/head:pull/5227 PR: https://git.openjdk.java.net/jdk/pull/5227
Re: [OpenJDK 2D-Dev] RFR: 8272805: Avoid looking up standard charsets
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: [OpenJDK 2D-Dev] RFR: 8272805: Avoid looking up standard charsets [v2]
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: [OpenJDK 2D-Dev] RFR: 8272756: Remove unnecessary explicit initialization of volatile variables in java.desktop
On Mon, 23 Aug 2021 17:57:52 GMT, Sergey Bylokhov wrote: >> Maybe it's kind of a protection from a race. Yet it's possible either way: >> another thread could see `essentialTags == null` before this one initialises >> the field. > > I think we can drop it completely. Agree. - PR: https://git.openjdk.java.net/jdk/pull/5197
Re: [OpenJDK 2D-Dev] RFR: 8272756: Remove unnecessary explicit initialization of volatile variables in java.desktop
On Mon, 23 Aug 2021 13:08:02 GMT, Alexey Ivanov wrote: >> Looks like there's no purpose, `tags` is not used after assignment > > Maybe it's kind of a protection from a race. Yet it's possible either way: > another thread could see `essentialTags == null` before this one initialises > the field. I think we can drop it completely. - PR: https://git.openjdk.java.net/jdk/pull/5197
Re: [OpenJDK 2D-Dev] RFR: 8272756: Remove unnecessary explicit initialization of volatile variables in java.desktop
On Mon, 23 Aug 2021 06:35:34 GMT, Сергей Цыпанов
wrote:
>> src/java.desktop/share/classes/com/sun/imageio/plugins/tiff/TIFFIFD.java
>> line 64:
>>
>>> 62: Set tags = essentialTags;
>>> 63: if (tags == null) {
>>> 64: essentialTags = Set.of(
>>
>> What is the purpose of the local tags here?
>
> Looks like there's no purpose, `tags` is not used after assignment
Maybe it's kind of a protection from a race. Yet it's possible either way:
another thread could see `essentialTags == null` before this one initialises
the field.
-
PR: https://git.openjdk.java.net/jdk/pull/5197
Re: [OpenJDK 2D-Dev] RFR: 8272805: Avoid looking up standard charsets [v2]
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/db47f6e1...e7127644
Changes to http server look good to me.
-
Marked as reviewed by dfuchs (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/5210
Re: [OpenJDK 2D-Dev] RFR: 8272756: Remove unnecessary explicit initialization of volatile variables in java.desktop
On Fri, 20 Aug 2021 21:26:41 GMT, Sergey Bylokhov wrote:
>> This is a continuation of
>>
>> - https://bugs.openjdk.java.net/browse/JDK-6736490
>> - https://bugs.openjdk.java.net/browse/JDK-8035284
>> - https://bugs.openjdk.java.net/browse/JDK-8145680
>> - https://bugs.openjdk.java.net/browse/JDK-8251548
>>
>> As mentioned in JDK-6736490:
>>
>> _An explicit initialization of a volatile class instance variable, such as
>> private volatile Object = null; or private volatile int scale = 0; is
>> unnecessary since the Java spec automatically initializes objects to null
>> and primitive type short, int, long, float and double to 0 and boolean to
>> false. Explicit initialization of volatile variable to a value the same as
>> the default implicit initialized value results in an unnecessary store and
>> membar operation._
>
> src/java.desktop/share/classes/com/sun/imageio/plugins/tiff/TIFFIFD.java line
> 64:
>
>> 62: Set tags = essentialTags;
>> 63: if (tags == null) {
>> 64: essentialTags = Set.of(
>
> What is the purpose of the local tags here?
Looks like there's no purpose, `tags` is not used after assignment
-
PR: https://git.openjdk.java.net/jdk/pull/5197
