Re: Wrong behavior of standard IO library when interacting with Samba (very serious)
Regarding the issue with isWritable, I came across this post after investigating: https://devblogs.microsoft.com/oldnewthing/20060202-00/?p=32413 This does seem to be a Windows `AccessCheck` problem, and there is almost no solution. For this reason, isWritable is not very useful. While you may not be able to fix it, I request an update to isWritable's Javadoc describing the limitations of this method. Its actual function is easily misunderstood, and I feel that users should be told in the documentation that it is recommended to try writing directly instead of checking with this method first. In addition to the problems mentioned here, I've also encountered some rarer problems, such as the bizarre AccessDeniedException. These issues don't seem to be reproducible on all machines, I'll investigate them further. If anyone is interested in these issues, I can provide a reproduction environment to help you test, thank you. On Mon, Mar 7, 2022 at 4:28 PM Alan Bateman wrote: > On 07/03/2022 05:33, Glavo wrote: > > I am a Java application developer. I noticed that when my program runs on > > Windows in a samba shared folder (mounted as a drive, or accessed via a > UNC > > path), the Java standard IO library has some unusual behavior. > > Note that these issues only occur when accessing a folder shared by > > *Samba*, but not for the folder shared via SMB by another Windows host. > > > > One of the bugs was reported years ago (JDK-8154915): `Files.isWritable` > > always returns false for files shared by samba. It's worth noting for > this > > question that `File::canWrite()`'s behavior is normal. > > (So in my program I pass `!Files.isWritable(p) && p.toFile().canWrite()` > to > > detect if it's shared by samba and give the user a warning) > > This problem keeps showing up on several of my devices, so it should be > > fine to reproduce. The reason it wasn't resolved seems to be that the > > OpenJDK maintainers didn't understand that it came up when interacting > with > > Samba (not just SMB). > Testing if a file is writable, without side effects, can be complicated > on Windows. File.canXXX only looks at the DOS attribute so can't give an > accurate result. Files.isWritable examines the DACL, the legacy DOS > attribute, and whether the volume is read-only, so there is more to go > wrong. We've looked at many Windows <--> SAMBA interop issues over the > years and it's always been that the Windows calls were failing or > returning results that suggested the file had a DACL with 0 entries. I > can't tell from your mail where the issue is but some of the behavior > you report in your mail is very unusual. The JDK does not cache results > so if you are saying that it requires restarting the JDK then it > suggests an issue at a lower level. > > -Alan >
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v7]
On Mon, 7 Mar 2022 20:36:36 GMT, Jim Laskey wrote:
>> Several attempts have been made to improve Formatter's numeric performance
>> by caching the current Locale zero. Such fixes, however, ignore the real
>> issue, which is the slowness of fetching DecimalFormatSymbols. By directly
>> caching DecimalFormatSymbols in the Formatter, this enhancement streamlines
>> the process of accessing Locale DecimalFormatSymbols and specifically
>> getZeroDigit(). The result is a general improvement in the performance of
>> numeric formatting.
>>
>>
>> @Benchmark
>> public void bigDecimalDefaultLocale() {
>> result = String.format("%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f", X);
>> }
>>
>> @Benchmark
>> public void bigDecimalLocaleAlternate() {
>> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> other = String.format(DEFAULT, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> }
>>
>> @Benchmark
>> public void bigDecimalThaiLocale() {
>> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> }
>>
>> @Benchmark
>> public void integerDefaultLocale() {
>> result = String.format("%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d", x);
>> }
>>
>> @Benchmark
>> public void integerLocaleAlternate() {
>> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> other = String.format(DEFAULT, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> }
>>
>> @Benchmark
>> public void integerThaiLocale() {
>> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> }
>>
>>
>> Before:
>> Benchmark Mode Cnt Score Error Units
>> MyBenchmark.bigDecimalDefaultLocale thrpt 25 75498.923 ± 3686.966 ops/s
>> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 39068.721 ± 162.983 ops/s
>> MyBenchmark.bigDecimalThaiLocale thrpt 25 77256.530 ± 294.743 ops/s
>> MyBenchmark.integerDefaultLocale thrpt 25 344093.071 ± 6189.002 ops/s
>> MyBenchmark.integerLocaleAlternate thrpt 25 165685.488 ± 440.857 ops/s
>> MyBenchmark.integerThaiLocale thrpt 25 327461.302 ± 1168.243 ops/s
>>
>> After:
>> Benchmark Mode Cnt Score Error Units
>> MyBenchmark.bigDecimalDefaultLocale thrpt 25 94735.293 ± 674.587 ops/s
>> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 44215.547 ± 291.664 ops/s
>> MyBenchmark.bigDecimalThaiLocale thrpt 25 91390.997 ± 658.677 ops/s
>> MyBenchmark.integerDefaultLocale thrpt 25 363722.977 ± 2864.554 ops/s
>> MyBenchmark.integerLocaleAlternate thrpt 25 165789.514 ± 779.656 ops/s
>> MyBenchmark.integerThaiLocale thrpt 25 351400.818 ± 1030.246 ops/s
>
> Jim Laskey has updated the pull request incrementally with four additional
> commits since the last revision:
>
> - Add comment about DecimalFormatSymbols being mutable objects.
> - Revert "Cache DecimalFormatSymbols in DecimalFormatSymbols instead of
> Formatter. No significant performance degradation."
>
>This reverts commit fcbf66a2fe9641d3c3349f12cc7b32d8b84c6f72.
> - Revert "Drop DecimalFormatSymbols.getLocale change"
>
>This reverts commit b93cdb03ec68f24f4b8851c0966bb144c30b5110.
> - Revert "Correct caching test"
>
>This reverts commit bf7975396aaad4ed58d053bde8f54984215eeba5.
src/java.base/share/classes/java/util/Formatter.java line 2019:
> 2017: return dfs;
> 2018: }
> 2019: // Fetch a new local instance of DecimalFormatSymbols. Note
> that DFS are mutatble
Thank you Jim for these comments about multi-threaded access. Looks good to me.
One minor typo on this line - "mutatble" should have been "mutable".
-
PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: 8281560: Matcher.hitEnd returns unexpected results in presence of CANON_EQ flag. [v2]
On Tue, 8 Mar 2022 04:20:17 GMT, Ian Graves wrote: >> test/jdk/java/util/regex/RegExTest.java line 4568: >> >>> 4566: >>> 4567: var matcher1 = Pattern.compile(pat1).matcher(testInput); >>> 4568: var matcher2 = Pattern.compile(pat2).matcher(testInput); >> >> Hello Ian, >> The JBS issue notes that the bug is only noticed when the `Pattern.CANON_EQ` >> is used. The javadoc of this `CANON_EQ` states that this isn't enabled by >> default. Do you think this test should also include matchers which use >> Pattern(s) with this `CANON_EQ` explicitly enabled? > > Oh wow yes. Thanks for this catch. I was rewriting the tests to fit this mold > and left the flags out. Added them back in. Thanks again! Thank you Ian, the changed version of the test looks good. - PR: https://git.openjdk.java.net/jdk/pull/7466
Re: RFR: 8281560: Matcher.hitEnd returns unexpected results in presence of CANON_EQ flag. [v4]
> Fixing a bug in `NFCCharProperty` where code falling through an if-statement > can prematurely set the matcher's `hitEnd` field to true. Ian Graves has updated the pull request incrementally with one additional commit since the last revision: Adding back some missing CANON_EQ flags - Changes: - all: https://git.openjdk.java.net/jdk/pull/7466/files - new: https://git.openjdk.java.net/jdk/pull/7466/files/0d23365f..98c3d6fd Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7466=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7466=02-03 Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod Patch: https://git.openjdk.java.net/jdk/pull/7466.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7466/head:pull/7466 PR: https://git.openjdk.java.net/jdk/pull/7466
Re: RFR: 8281560: Matcher.hitEnd returns unexpected results in presence of CANON_EQ flag. [v2]
On Tue, 8 Mar 2022 03:49:43 GMT, Jaikiran Pai wrote: >> Ian Graves has updated the pull request with a new target base due to a >> merge or a rebase. The pull request now contains two commits: >> >> - merging master >> - Catching erronious setting of matcher.hitEnd > > test/jdk/java/util/regex/RegExTest.java line 4568: > >> 4566: >> 4567: var matcher1 = Pattern.compile(pat1).matcher(testInput); >> 4568: var matcher2 = Pattern.compile(pat2).matcher(testInput); > > Hello Ian, > The JBS issue notes that the bug is only noticed when the `Pattern.CANON_EQ` > is used. The javadoc of this `CANON_EQ` states that this isn't enabled by > default. Do you think this test should also include matchers which use > Pattern(s) with this `CANON_EQ` explicitly enabled? Oh wow yes. Thanks for this catch. I was rewriting the tests to fit this mold and left the flags out. Added them back in. Thanks again! - PR: https://git.openjdk.java.net/jdk/pull/7466
Re: RFR: 8281560: Matcher.hitEnd returns unexpected results in presence of CANON_EQ flag. [v3]
> Fixing a bug in `NFCCharProperty` where code falling through an if-statement > can prematurely set the matcher's `hitEnd` field to true. Ian Graves has updated the pull request incrementally with one additional commit since the last revision: Removing errant newline - Changes: - all: https://git.openjdk.java.net/jdk/pull/7466/files - new: https://git.openjdk.java.net/jdk/pull/7466/files/cf8ffd6c..0d23365f Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7466=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7466=01-02 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7466.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7466/head:pull/7466 PR: https://git.openjdk.java.net/jdk/pull/7466
Re: RFR: 8281560: Matcher.hitEnd returns unexpected results in presence of CANON_EQ flag. [v3]
On Mon, 7 Mar 2022 23:05:55 GMT, Lance Andersen wrote:
>> Ian Graves has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Removing errant newline
>
> src/java.base/share/classes/java/util/regex/Pattern.java line 4009:
>
>> 4007: return false;
>> 4008: }
>> 4009: else {
>
> Is there a reason the "else" starts on its own line?
I must love Enter too much. Thanks for the catch!
-
PR: https://git.openjdk.java.net/jdk/pull/7466
RFR: 8264160: Regex \b is not consistent with \w without UNICODE_CHARACTER_CLASS
Proposed change in behavior to correct inconsistencies between `\w` and `\b` metacharacters - Commit messages: - Fixing bad javadoc - Merge remote-tracking branch 'upstream/master' into bug-8264160 - Updating spec - Proposed change for \b metacharacter Changes: https://git.openjdk.java.net/jdk/pull/7539/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7539=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8264160 Stats: 78 lines in 2 files changed: 60 ins; 6 del; 12 mod Patch: https://git.openjdk.java.net/jdk/pull/7539.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7539/head:pull/7539 PR: https://git.openjdk.java.net/jdk/pull/7539
Re: RFR: 8281560: Matcher.hitEnd returns unexpected results in presence of CANON_EQ flag. [v2]
On Mon, 7 Mar 2022 22:29:29 GMT, Ian Graves wrote: >> Fixing a bug in `NFCCharProperty` where code falling through an if-statement >> can prematurely set the matcher's `hitEnd` field to true. > > Ian Graves has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains two commits: > > - merging master > - Catching erronious setting of matcher.hitEnd test/jdk/java/util/regex/RegExTest.java line 4568: > 4566: > 4567: var matcher1 = Pattern.compile(pat1).matcher(testInput); > 4568: var matcher2 = Pattern.compile(pat2).matcher(testInput); Hello Ian, The JBS issue notes that the bug is only noticed when the `Pattern.CANON_EQ` is used. The javadoc of this `CANON_EQ` states that this isn't enabled by default. Do you think this test should also include matchers which use Pattern(s) with this `CANON_EQ` explicitly enabled? - PR: https://git.openjdk.java.net/jdk/pull/7466
Please help backport the fix with the XSLT compiler in JDK18 to JDK11 & JDK17
Hi there, I notice the issue with the XSLT compiler (https://github.com/openjdk/jdk/blob/master/src/java.xml/share/classes/com/sun/org/apache/xalan/internal/xsltc/compiler/XSLTC.java) I raised later last year at https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-November/083135.html has been fixed in JDK18 via https://bugs.openjdk.java.net/browse/JDK-8276657. Could you help backport the fix to JDK11 and JDK17 give both of them share the same issue in code? Many thanks. Best Regards Cheng Jin
Re: RFR: 8281560: Matcher.hitEnd returns unexpected results in presence of CANON_EQ flag. [v2]
On Mon, 7 Mar 2022 22:29:29 GMT, Ian Graves wrote: >> Fixing a bug in `NFCCharProperty` where code falling through an if-statement >> can prematurely set the matcher's `hitEnd` field to true. > > Ian Graves has updated the pull request with a new target base due to a merge > or a rebase. The pull request now contains two commits: > > - merging master > - Catching erronious setting of matcher.hitEnd LGTM - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/7466
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v17]
On Sat, 5 Mar 2022 19:54:44 GMT, Joe Darcy wrote: >> This is an early review of changes to better model JVM access flags, that is >> "modifiers" like public, protected, etc. but explicitly at a VM level. >> >> Language level modifiers and JVM level access flags are closely related, but >> distinct. There are concepts that overlap in the two domains (public, >> private, etc.), others that only have a language-level modifier (sealed), >> and still others that only have an access flag (synthetic). >> >> The existing java.lang.reflect.Modifier class is inadequate to model these >> subtleties. For example, the bit positions used by access flags on different >> kinds of elements overlap (such as "volatile" for fields and "bridge" for >> methods. Just having a raw integer does not provide sufficient context to >> decode the corresponding language-level string. Methods like >> Modifier.methodModifiers() were introduced to cope with this situation. >> >> With additional modifiers and flags on the horizon with projects like >> Valhalla, addressing the existent modeling deficiency now ahead of time is >> reasonable before further strain is introduced. >> >> This PR in its current form is meant to give the overall shape of the API. >> It is missing implementations to map from, say, method modifiers to access >> flags, taking into account overlaps in bit positions. >> >> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in >> once the API is further along. > > Joe Darcy 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 26 additional commits since > the last revision: > > - Respond to review feedback. > - Merge branch 'master' into JDK-8266670 > - Make workding changes suggested in review feedback. > - Merge branch 'master' into JDK-8266670 > - Typo fix; add implSpec to Executable. > - Appease jcheck. > - Fix some bugs found by inspection, docs cleanup. > - Merge branch 'master' into JDK-8266670 > - Initial support for accessFlags methods > - Add mask to access flag functionality. > - ... and 16 more: > https://git.openjdk.java.net/jdk/compare/012ab186...14980605 > The mapping from Location to AccessFlag(s) could be implemented event without > using a Map. You just have to be careful not to use EnumSet for that (i.e. > before AccessFlag enum constants are fully initialized) - an ArrayList is > better for this case anyway. Also, conversion from ModuleDescriptor > modifier(s) to AccessFlag(s) could be done without first creating a bitmask > and then re-constructing a set of AccessFlag(s) from it. Reflective object > modifiers can only be translated via bitmask, but various ModuleDescriptor > Modifier(s) may have a direct mapping to corresponding AccessFlag(s). For > example: > > [plevart@5a3cd8f](https://github.com/plevart/jdk/commit/5a3cd8f6851a0deae7e3e5028bba9a51d7863929) Yes, I made the same observation for the module-related modifiers when coding this up; I'll look to add some API support to avoid the need to detour through bitmasks. To get the public API worked out, I wanted to avoid complications in inter-dependent class initialization. Thanks for suggesting an alternative; I'll consider that when I resume work on this PR (need to start writing some tests...) Thanks. - PR: https://git.openjdk.java.net/jdk/pull/7445
Re: RFR: 8282429: StringBuilder/StringBuffer.toString() skip compressing for UTF16 strings [v2]
On Mon, 7 Mar 2022 12:10:51 GMT, Daniel Jeliński wrote:
>> Xin Liu has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> make sure String(StringBuffer) is still synchronized.
>
> src/java.base/share/classes/java/lang/String.java line 1446:
>
>> 1444: */
>> 1445: public String(StringBuffer buffer) {
>> 1446: this(buffer, null);
>
> This method is no longer synchronized on StringBuffer
you're right. fixed in revision#2.
this could be very tricky to discover. thanks for the head-up!
-
PR: https://git.openjdk.java.net/jdk/pull/7671
Re: RFR: 8282429: StringBuilder/StringBuffer.toString() skip compressing for UTF16 strings [v2]
> If AbstractStringBuilder only grow, the inflated value which has been encoded > in UTF16 can't be compressed. > toString() can skip compression in this case. This can save an > ArrayAllocation in StringUTF16::compress(). > > java.io.BufferedRead::readLine() is a case that StringBuilder grows only. > > In microbench, we expect to see that allocation/op reduces 20%. The initial > capacity of StringBuilder is S in bytes. When it encounters the 1st character > that can't be encoded in LATIN1, it inflates and allocate a new array of 2*S. > `toString()` will try to compress that value so it need to allocate S bytes. > The last step allocates 2*S bytes because it has to copy the string. so it > requires to allocate 5 * S bytes in total. By skipping the failed > compression, it only allocates 4 * S bytes. that's 20%. In real execution, > we observe 16% allocation reduction, tracked by JMH GC profiler > `gc.alloc.rate.norm `. I think it's because HotSpot can't track all > allocations. > > Not only allocation drops, the runtime performance(ns/op) also increases from > 3.34% to 18.91%. > > Before: > > $$make test > TEST="micro:org.openjdk.bench.java.lang.StringBuilders.toStringWithMixedChars" > MICRO="OPTIONS=-prof gc -gc true -o before.log -jvm > $HOME/Devel/jdk_baseline/bin/java" > > Benchmark > (MIXED_SIZE) Mode Cnt Score Error Units > StringBuilders.toStringWithMixedChars >128 avgt 15 649.846 ± 76.291 ns/op > StringBuilders.toStringWithMixedChars:·gc.alloc.rate >128 avgt 15 872.855 ± 128.259 MB/sec > StringBuilders.toStringWithMixedChars:·gc.alloc.rate.norm >128 avgt 15 880.121 ± 0.050B/op > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space >128 avgt 15 707.730 ± 194.421 MB/sec > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space.norm >128 avgt 15 706.602 ± 94.504B/op > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space >128 avgt 15 0.001 ± 0.002 MB/sec > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space.norm >128 avgt 15 0.001 ± 0.001B/op > StringBuilders.toStringWithMixedChars:·gc.count >128 avgt 15 113.000counts > StringBuilders.toStringWithMixedChars:·gc.time >128 avgt 1585.000ms > StringBuilders.toStringWithMixedChars >256 avgt 15 1316.652 ± 112.771 ns/op > StringBuilders.toStringWithMixedChars:·gc.alloc.rate >256 avgt 15 800.864 ± 76.869 MB/sec > StringBuilders.toStringWithMixedChars:·gc.alloc.rate.norm >256 avgt 15 1648.288 ± 0.162B/op > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space >256 avgt 15 599.736 ± 174.001 MB/sec > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space.norm >256 avgt 15 1229.669 ± 318.518B/op > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space >256 avgt 15 0.001 ± 0.001 MB/sec > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space.norm >256 avgt 15 0.001 ± 0.002B/op > StringBuilders.toStringWithMixedChars:·gc.count >256 avgt 15 133.000counts > StringBuilders.toStringWithMixedChars:·gc.time >256 avgt 1592.000ms > StringBuilders.toStringWithMixedChars > 1024 avgt 15 5204.303 ± 418.115 ns/op > StringBuilders.toStringWithMixedChars:·gc.alloc.rate > 1024 avgt 15 768.730 ± 72.945 MB/sec > StringBuilders.toStringWithMixedChars:·gc.alloc.rate.norm > 1024 avgt 15 6256.844 ± 0.358B/op > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space > 1024 avgt 15 655.852 ± 121.602 MB/sec > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space.norm > 1024 avgt 15 5315.265 ± 578.878B/op > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space > 1024 avgt 15 0.002 ± 0.002 MB/sec > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space.norm > 1024 avgt 15 0.014 ± 0.011B/op > StringBuilders.toStringWithMixedChars:·gc.count > 1024 avgt 1596.000counts > StringBuilders.toStringWithMixedChars:·gc.time > 1024 avgt 1586.000ms > > > After > > $make test >
Re: RFR: 8281146: Replace StringCoding.hasNegatives with countPositives [v11]
> I'm requesting comments and, hopefully, some help with this patch to replace > `StringCoding.hasNegatives` with `countPositives`. The new method does a very > similar pass, but alters the intrinsic to return the number of leading bytes > in the `byte[]` range which only has positive bytes. This allows for dealing > much more efficiently with those `byte[]`s that has a ASCII prefix, with no > measurable cost on ASCII-only or latin1/UTF16-mostly input. > > Microbenchmark results: > https://jmh.morethan.io/?gists=428b487e92e3e47ccb7f169501600a88,3c585de7435506d3a3bdb32160fe8904 > > - Only implemented on x86 for now, but I want to verify that implementations > of `countPositives` can be implemented with similar efficiency on all > platforms that today implement a `hasNegatives` intrinsic (aarch64, ppc etc) > before moving ahead. This pretty much means holding up this until it's > implemented on all platforms, which can either contributed to this PR or as > dependent follow-ups. > > - An alternative to holding up until all platforms are on board is to allow > the implementation of `StringCoding.hasNegatives` and `countPositives` to be > implemented so that the non-intrinsified method calls into the intrinsified. > This requires structuring the implementations differently based on which > intrinsic - if any - is actually implemented. One way to do this could be to > mimic how `java.nio` handles unaligned accesses and expose which intrinsic is > available via `Unsafe` into a `static final` field. > > - There are a few minor regressions (~5%) in the x86 implementation on > `encode-/decodeLatin1Short`. Those regressions disappear when mixing inputs, > for example `encode-/decodeShortMixed` even see a minor improvement, which > makes me consider those corner case regressions with little real world > implications (if you have latin1 Strings, you're likely to also have > ASCII-only strings in your mix). Claes Redestad has updated the pull request incrementally with two additional commits since the last revision: - use 32-bit mask to calculate correct remainder value - ary1 not required to have USE_KILL effect - Changes: - all: https://git.openjdk.java.net/jdk/pull/7231/files - new: https://git.openjdk.java.net/jdk/pull/7231/files/81ef04ec..934b5b8a Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7231=10 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7231=09-10 Stats: 3 lines in 2 files changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/7231.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7231/head:pull/7231 PR: https://git.openjdk.java.net/jdk/pull/7231
Re: RFR: 8281560: Matcher.hitEnd returns unexpected results in presence of CANON_EQ flag. [v2]
On Mon, 7 Mar 2022 22:29:29 GMT, Ian Graves wrote:
>> Fixing a bug in `NFCCharProperty` where code falling through an if-statement
>> can prematurely set the matcher's `hitEnd` field to true.
>
> Ian Graves has updated the pull request with a new target base due to a merge
> or a rebase. The pull request now contains two commits:
>
> - merging master
> - Catching erronious setting of matcher.hitEnd
src/java.base/share/classes/java/util/regex/Pattern.java line 4009:
> 4007: return false;
> 4008: }
> 4009: else {
Is there a reason the "else" starts on its own line?
-
PR: https://git.openjdk.java.net/jdk/pull/7466
Re: RFR: 8281146: Replace StringCoding.hasNegatives with countPositives [v10]
> I'm requesting comments and, hopefully, some help with this patch to replace > `StringCoding.hasNegatives` with `countPositives`. The new method does a very > similar pass, but alters the intrinsic to return the number of leading bytes > in the `byte[]` range which only has positive bytes. This allows for dealing > much more efficiently with those `byte[]`s that has a ASCII prefix, with no > measurable cost on ASCII-only or latin1/UTF16-mostly input. > > Microbenchmark results: > https://jmh.morethan.io/?gists=428b487e92e3e47ccb7f169501600a88,3c585de7435506d3a3bdb32160fe8904 > > - Only implemented on x86 for now, but I want to verify that implementations > of `countPositives` can be implemented with similar efficiency on all > platforms that today implement a `hasNegatives` intrinsic (aarch64, ppc etc) > before moving ahead. This pretty much means holding up this until it's > implemented on all platforms, which can either contributed to this PR or as > dependent follow-ups. > > - An alternative to holding up until all platforms are on board is to allow > the implementation of `StringCoding.hasNegatives` and `countPositives` to be > implemented so that the non-intrinsified method calls into the intrinsified. > This requires structuring the implementations differently based on which > intrinsic - if any - is actually implemented. One way to do this could be to > mimic how `java.nio` handles unaligned accesses and expose which intrinsic is > available via `Unsafe` into a `static final` field. > > - There are a few minor regressions (~5%) in the x86 implementation on > `encode-/decodeLatin1Short`. Those regressions disappear when mixing inputs, > for example `encode-/decodeShortMixed` even see a minor improvement, which > makes me consider those corner case regressions with little real world > implications (if you have latin1 Strings, you're likely to also have > ASCII-only strings in your mix). Claes Redestad has updated the pull request incrementally with one additional commit since the last revision: Better implementation for aarch64 returning roughly the count of positive bytes - Changes: - all: https://git.openjdk.java.net/jdk/pull/7231/files - new: https://git.openjdk.java.net/jdk/pull/7231/files/85be36ae..81ef04ec Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7231=09 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7231=08-09 Stats: 34 lines in 3 files changed: 13 ins; 3 del; 18 mod Patch: https://git.openjdk.java.net/jdk/pull/7231.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7231/head:pull/7231 PR: https://git.openjdk.java.net/jdk/pull/7231
Re: RFR: JDK-8282144 RandomSupport.convertSeedBytesToLongs sign extension overwrites previous bytes
On Thu, 24 Feb 2022 14:47:50 GMT, Jim Laskey wrote: > Class: ./java.base/share/classes/jdk/internal/util/random/RandomSupport.java > Method: public static long[] convertSeedBytesToLongs(byte[] seed, int n, int > z) > > The method attempts to create an array of longs by consuming the input bytes > most significant bit first. New bytes are appended to the existing long using > the OR operator on the signed byte. Due to sign extension this will overwrite > all the existing bits from 63 to 8 if the next byte is negative. Marked as reviewed by bpb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7614
Re: RFR: JDK-8282144 RandomSupport.convertSeedBytesToLongs sign extension overwrites previous bytes
On Sun, 27 Feb 2022 22:30:44 GMT, Jim Laskey wrote:
>> test/jdk/java/util/Random/T8282144.java line 39:
>>
>>> 37: public class T8282144 {
>>> 38: public static void main(String[] args) {
>>> 39: RandomGenerator rng =
>>> RandomGeneratorFactory.of("L64X128MixRandom").create(42);
>>
>> Does `rng` always produce the same sequence? If so, then perhaps the seed,
>> `42`, should be a random value that is printed.
>
> 42 was chosen because its is known to produce negative byte values, other
> random values might not.
OK
>> test/jdk/java/util/Random/T8282144.java line 52:
>>
>>> 50: for (int k = 0; k < existing.length; k++) {
>>> 51: if (existing[k] != testing[k]) {
>>> 52: throw new
>>> RuntimeException("convertSeedBytesToLongs incorrect");
>>
>> Should `i`, `j`, and `k` be included in the exception message?
>
> Correctness is binary - either it works or it doesn't. The values of i, j, k
> would not assist in isolating issues. Might add to confusion if displayed.
Understood.
-
PR: https://git.openjdk.java.net/jdk/pull/7614
Re: RFR: 8281560: Matcher.hitEnd returns unexpected results in presence of CANON_EQ flag. [v2]
> Fixing a bug in `NFCCharProperty` where code falling through an if-statement > can prematurely set the matcher's `hitEnd` field to true. Ian Graves has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits: - merging master - Catching erronious setting of matcher.hitEnd - Changes: https://git.openjdk.java.net/jdk/pull/7466/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7466=01 Stats: 29 lines in 2 files changed: 27 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7466.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7466/head:pull/7466 PR: https://git.openjdk.java.net/jdk/pull/7466
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v7]
On Mon, 7 Mar 2022 20:36:36 GMT, Jim Laskey wrote:
>> Several attempts have been made to improve Formatter's numeric performance
>> by caching the current Locale zero. Such fixes, however, ignore the real
>> issue, which is the slowness of fetching DecimalFormatSymbols. By directly
>> caching DecimalFormatSymbols in the Formatter, this enhancement streamlines
>> the process of accessing Locale DecimalFormatSymbols and specifically
>> getZeroDigit(). The result is a general improvement in the performance of
>> numeric formatting.
>>
>>
>> @Benchmark
>> public void bigDecimalDefaultLocale() {
>> result = String.format("%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f", X);
>> }
>>
>> @Benchmark
>> public void bigDecimalLocaleAlternate() {
>> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> other = String.format(DEFAULT, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> }
>>
>> @Benchmark
>> public void bigDecimalThaiLocale() {
>> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> }
>>
>> @Benchmark
>> public void integerDefaultLocale() {
>> result = String.format("%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d", x);
>> }
>>
>> @Benchmark
>> public void integerLocaleAlternate() {
>> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> other = String.format(DEFAULT, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> }
>>
>> @Benchmark
>> public void integerThaiLocale() {
>> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> }
>>
>>
>> Before:
>> Benchmark Mode Cnt Score Error Units
>> MyBenchmark.bigDecimalDefaultLocale thrpt 25 75498.923 ± 3686.966 ops/s
>> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 39068.721 ± 162.983 ops/s
>> MyBenchmark.bigDecimalThaiLocale thrpt 25 77256.530 ± 294.743 ops/s
>> MyBenchmark.integerDefaultLocale thrpt 25 344093.071 ± 6189.002 ops/s
>> MyBenchmark.integerLocaleAlternate thrpt 25 165685.488 ± 440.857 ops/s
>> MyBenchmark.integerThaiLocale thrpt 25 327461.302 ± 1168.243 ops/s
>>
>> After:
>> Benchmark Mode Cnt Score Error Units
>> MyBenchmark.bigDecimalDefaultLocale thrpt 25 94735.293 ± 674.587 ops/s
>> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 44215.547 ± 291.664 ops/s
>> MyBenchmark.bigDecimalThaiLocale thrpt 25 91390.997 ± 658.677 ops/s
>> MyBenchmark.integerDefaultLocale thrpt 25 363722.977 ± 2864.554 ops/s
>> MyBenchmark.integerLocaleAlternate thrpt 25 165789.514 ± 779.656 ops/s
>> MyBenchmark.integerThaiLocale thrpt 25 351400.818 ± 1030.246 ops/s
>
> Jim Laskey has updated the pull request incrementally with four additional
> commits since the last revision:
>
> - Add comment about DecimalFormatSymbols being mutable objects.
> - Revert "Cache DecimalFormatSymbols in DecimalFormatSymbols instead of
> Formatter. No significant performance degradation."
>
>This reverts commit fcbf66a2fe9641d3c3349f12cc7b32d8b84c6f72.
> - Revert "Drop DecimalFormatSymbols.getLocale change"
>
>This reverts commit b93cdb03ec68f24f4b8851c0966bb144c30b5110.
> - Revert "Correct caching test"
>
>This reverts commit bf7975396aaad4ed58d053bde8f54984215eeba5.
Looks good. Since you are adding a new API, I would expect some unit tests for
`DecimalFormatSymbols.getLocale()` method.
-
PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: 8282664: Unroll by hand StringUTF16 and StringLatin1 polynomial hash loops [v2]
On Fri, 4 Mar 2022 17:44:44 GMT, Ludovic Henry wrote: >> Despite the hash value being cached for Strings, computing the hash still >> represents a significant CPU usage for applications handling lots of text. >> >> Even though it would be generally better to do it through an enhancement to >> the autovectorizer, the complexity of doing it by hand is trivial and the >> gain is sizable (2x speedup) even without the Vector API. The algorithm has >> been proposed by Richard Startin and Paul Sandoz [1]. >> >> Speedup are as follows on a `Intel(R) Xeon(R) E-2276G CPU @ 3.80GHz` >> >> >> Benchmark(size) Mode Cnt Score >>Error Units >> StringHashCode.Algorithm.scalarLatin1 0 avgt 25 2.111 >> ± 0.210 ns/op >> StringHashCode.Algorithm.scalarLatin1 1 avgt 25 3.500 >> ± 0.127 ns/op >> StringHashCode.Algorithm.scalarLatin110 avgt 25 7.001 >> ± 0.099 ns/op >> StringHashCode.Algorithm.scalarLatin1 100 avgt 2561.285 >> ± 0.444 ns/op >> StringHashCode.Algorithm.scalarLatin1 1000 avgt 25 628.995 >> ± 0.846 ns/op >> StringHashCode.Algorithm.scalarLatin1 1 avgt 25 6307.990 >> ± 4.071 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled16 0 avgt 25 2.358 >> ± 0.092 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled16 1 avgt 25 3.631 >> ± 0.159 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled16 10 avgt 25 7.049 >> ± 0.019 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled16 100 avgt 2533.626 >> ± 1.218 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled161000 avgt 25 317.811 >> ± 1.225 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled16 1 avgt 25 3212.333 >> ± 14.621 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled80 avgt 25 2.356 >> ± 0.097 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled81 avgt 25 3.630 >> ± 0.158 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled8 10 avgt 25 8.724 >> ± 0.065 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled8 100 avgt 2532.402 >> ± 0.019 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled8 1000 avgt 25 321.949 >> ± 0.251 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled81 avgt 25 3202.083 >> ± 1.667 ns/op >> StringHashCode.Algorithm.scalarUTF16 0 avgt 25 2.135 >> ± 0.191 ns/op >> StringHashCode.Algorithm.scalarUTF16 1 avgt 25 5.202 >> ± 0.362 ns/op >> StringHashCode.Algorithm.scalarUTF16 10 avgt 2511.105 >> ± 0.112 ns/op >> StringHashCode.Algorithm.scalarUTF16100 avgt 2575.974 >> ± 0.702 ns/op >> StringHashCode.Algorithm.scalarUTF16 1000 avgt 25 716.429 >> ± 3.290 ns/op >> StringHashCode.Algorithm.scalarUTF16 1 avgt 25 7095.459 >> ± 43.847 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled160 avgt 25 2.381 >> ± 0.038 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled161 avgt 25 5.268 >> ± 0.422 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled16 10 avgt 2511.248 >> ± 0.178 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled16 100 avgt 2552.966 >> ± 0.089 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled16 1000 avgt 25 450.912 >> ± 1.834 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled161 avgt 25 4403.988 >> ± 2.927 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled8 0 avgt 25 2.401 >> ± 0.032 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled8 1 avgt 25 5.091 >> ± 0.396 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled810 avgt 2512.801 >> ± 0.189 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled8 100 avgt 2552.068 >> ± 0.032 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled8 1000 avgt 25 453.270 >> ± 0.340 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled8 1 avgt 25 4433.112 >> ± 2.699 ns/op >> >> >> At Datadog, we handle a great amount of text (through logs management for >> example), and hashing String represents a large part of our CPU usage. It's >> very unlikely that we are the only one as String.hashCode is such a core >> feature of the JVM-based languages with its use in HashMap for example. >> Having even only a 2x speedup would allow us to save thousands of CPU cores >> per month and improve correspondingly the energy/carbon impact. >> >> [1] >> https://static.rainfocus.com/oracle/oow18/sess/1525822677955001tLqU/PF/codeone18-vector-API-DEV5081_1540354883936001Q3Sv.pdf > > Ludovic Henry has updated the pull request incrementally with one additional > commit since
Integrated: 8282657: Code cleanup: removing double semicolons at the end of lines
On Fri, 28 Jan 2022 14:39:31 GMT, Matteo Baccan wrote: > Hi > > I have reviewed the code for removing double semicolons at the end of lines > > all the best > matteo This pull request has now been integrated. Changeset: ccad3923 Author:Matteo Baccan Committer: Magnus Ihse Bursie URL: https://git.openjdk.java.net/jdk/commit/ccad39237ab860c5c5579537f740177e3f1adcc9 Stats: 93 lines in 82 files changed: 0 ins; 0 del; 93 mod 8282657: Code cleanup: removing double semicolons at the end of lines Reviewed-by: lancea, rriggs, ihse, prr, iris, wetmore, darcy, dholmes - PR: https://git.openjdk.java.net/jdk/pull/7268
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v17]
On Mon, 7 Mar 2022 18:20:23 GMT, Roger Riggs wrote: >> Joe Darcy 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 26 additional commits >> since the last revision: >> >> - Respond to review feedback. >> - Merge branch 'master' into JDK-8266670 >> - Make workding changes suggested in review feedback. >> - Merge branch 'master' into JDK-8266670 >> - Typo fix; add implSpec to Executable. >> - Appease jcheck. >> - Fix some bugs found by inspection, docs cleanup. >> - Merge branch 'master' into JDK-8266670 >> - Initial support for accessFlags methods >> - Add mask to access flag functionality. >> - ... and 16 more: >> https://git.openjdk.java.net/jdk/compare/36b93dbf...14980605 > > src/java.base/share/classes/java/lang/Class.java line 1334: > >> 1332: // allows PRIVATE, PROTECTED, and STATIC, which are not >> 1333: // allowed on Location.CLASS. >> 1334: return AccessFlag.maskToAccessFlags(getModifiers(), > > Computing and creating the Set every time seems like a high overhead > operation (compared to getModifiers()). > Caching either here (in the Member) or in AccessFlag.maskToAccessFlags would > be desirable. > Caching is idempotent so it should not need synchronization. For this phase of the work, I was trying to avoid premature optimization of building caches, etc. Such refinement should certainly be considered later on. - PR: https://git.openjdk.java.net/jdk/pull/7445
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v7]
On Mon, 7 Mar 2022 20:36:36 GMT, Jim Laskey wrote:
>> Several attempts have been made to improve Formatter's numeric performance
>> by caching the current Locale zero. Such fixes, however, ignore the real
>> issue, which is the slowness of fetching DecimalFormatSymbols. By directly
>> caching DecimalFormatSymbols in the Formatter, this enhancement streamlines
>> the process of accessing Locale DecimalFormatSymbols and specifically
>> getZeroDigit(). The result is a general improvement in the performance of
>> numeric formatting.
>>
>>
>> @Benchmark
>> public void bigDecimalDefaultLocale() {
>> result = String.format("%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f", X);
>> }
>>
>> @Benchmark
>> public void bigDecimalLocaleAlternate() {
>> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> other = String.format(DEFAULT, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> }
>>
>> @Benchmark
>> public void bigDecimalThaiLocale() {
>> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> }
>>
>> @Benchmark
>> public void integerDefaultLocale() {
>> result = String.format("%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d", x);
>> }
>>
>> @Benchmark
>> public void integerLocaleAlternate() {
>> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> other = String.format(DEFAULT, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> }
>>
>> @Benchmark
>> public void integerThaiLocale() {
>> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> }
>>
>>
>> Before:
>> Benchmark Mode Cnt Score Error Units
>> MyBenchmark.bigDecimalDefaultLocale thrpt 25 75498.923 ± 3686.966 ops/s
>> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 39068.721 ± 162.983 ops/s
>> MyBenchmark.bigDecimalThaiLocale thrpt 25 77256.530 ± 294.743 ops/s
>> MyBenchmark.integerDefaultLocale thrpt 25 344093.071 ± 6189.002 ops/s
>> MyBenchmark.integerLocaleAlternate thrpt 25 165685.488 ± 440.857 ops/s
>> MyBenchmark.integerThaiLocale thrpt 25 327461.302 ± 1168.243 ops/s
>>
>> After:
>> Benchmark Mode Cnt Score Error Units
>> MyBenchmark.bigDecimalDefaultLocale thrpt 25 94735.293 ± 674.587 ops/s
>> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 44215.547 ± 291.664 ops/s
>> MyBenchmark.bigDecimalThaiLocale thrpt 25 91390.997 ± 658.677 ops/s
>> MyBenchmark.integerDefaultLocale thrpt 25 363722.977 ± 2864.554 ops/s
>> MyBenchmark.integerLocaleAlternate thrpt 25 165789.514 ± 779.656 ops/s
>> MyBenchmark.integerThaiLocale thrpt 25 351400.818 ± 1030.246 ops/s
>
> Jim Laskey has updated the pull request incrementally with four additional
> commits since the last revision:
>
> - Add comment about DecimalFormatSymbols being mutable objects.
> - Revert "Cache DecimalFormatSymbols in DecimalFormatSymbols instead of
> Formatter. No significant performance degradation."
>
>This reverts commit fcbf66a2fe9641d3c3349f12cc7b32d8b84c6f72.
> - Revert "Drop DecimalFormatSymbols.getLocale change"
>
>This reverts commit b93cdb03ec68f24f4b8851c0966bb144c30b5110.
> - Revert "Correct caching test"
>
>This reverts commit bf7975396aaad4ed58d053bde8f54984215eeba5.
LGTM
src/java.base/share/classes/java/util/Formatter.java line 4550:
> 4548:
> 4549: if (l == null || l.equals(Locale.US)) {
> 4550: grpSize = 3;
unintentional indentation
-
Marked as reviewed by rriggs (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: 8279185: Support for IsoFields in JapaneseDate/MinguoDate/ThaiBuddhistDate [v3]
On Mon, 7 Mar 2022 17:04:25 GMT, Joe Wang wrote: >> Is `IsoBased` is fine with me. "isISOLike" is too vague. > > That matches the javadoc as well, that it "supports ISO based fields". Renamed the new method to `isIsoBased()`. Modified the CSR accordingly. - PR: https://git.openjdk.java.net/jdk/pull/7683
Integrated: 8281093: Violating Attribute-Value Normalization in the XML specification 1.0
On Mon, 7 Mar 2022 17:07:20 GMT, Ravi Reddy wrote: > This fix is for violation of XML specification on Attribute-Value > normalization for external entities having character "\r". > > While normalizing entity with '\r', we should be checking if the entity is > external before changing the position and offset. "isExternal()" check is > missed in the new method : > normalizeNewlines(short version, XMLString buffer, boolean append,boolean > storeWS, NameType nt). > . This pull request has now been integrated. Changeset: 3996782c Author:Ravi Reddy Committer: Joe Wang URL: https://git.openjdk.java.net/jdk/commit/3996782c5af7b0396d5133fab457c507758d9340 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod 8281093: Violating Attribute-Value Normalization in the XML specification 1.0 Reviewed-by: joehw - PR: https://git.openjdk.java.net/jdk/pull/7731
Re: RFR: 8279185: Support for IsoFields in JapaneseDate/MinguoDate/ThaiBuddhistDate [v4]
On Mon, 7 Mar 2022 18:20:43 GMT, Naoto Sato wrote:
>> Supporting `IsoFields` temporal fields in chronologies that are similar to
>> ISO chronology. Corresponding CSR has also been drafted.
>
> Naoto Sato 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 five additional commits since
> the last revision:
>
> - Renamed the new method
> - Merge branch 'master' into JDK-8279185
> - Addresses review comments
> - copyright year fix
> - 8279185: Support for IsoFields in JapaneseDate/MinguoDate/ThaiBuddhistDate
src/java.base/share/classes/java/time/chrono/IsoChronology.java line 689:
> 687: */
> 688: @Override
> 689: public boolean isIsoBased() {
Is this meant to be 'default'? The CSR indicated adding default methods.
-
PR: https://git.openjdk.java.net/jdk/pull/7683
Re: RFR: 8282664: Unroll by hand StringUTF16 and StringLatin1 polynomial hash loops [v2]
On Fri, 4 Mar 2022 17:44:44 GMT, Ludovic Henry wrote: >> Despite the hash value being cached for Strings, computing the hash still >> represents a significant CPU usage for applications handling lots of text. >> >> Even though it would be generally better to do it through an enhancement to >> the autovectorizer, the complexity of doing it by hand is trivial and the >> gain is sizable (2x speedup) even without the Vector API. The algorithm has >> been proposed by Richard Startin and Paul Sandoz [1]. >> >> Speedup are as follows on a `Intel(R) Xeon(R) E-2276G CPU @ 3.80GHz` >> >> >> Benchmark(size) Mode Cnt Score >>Error Units >> StringHashCode.Algorithm.scalarLatin1 0 avgt 25 2.111 >> ± 0.210 ns/op >> StringHashCode.Algorithm.scalarLatin1 1 avgt 25 3.500 >> ± 0.127 ns/op >> StringHashCode.Algorithm.scalarLatin110 avgt 25 7.001 >> ± 0.099 ns/op >> StringHashCode.Algorithm.scalarLatin1 100 avgt 2561.285 >> ± 0.444 ns/op >> StringHashCode.Algorithm.scalarLatin1 1000 avgt 25 628.995 >> ± 0.846 ns/op >> StringHashCode.Algorithm.scalarLatin1 1 avgt 25 6307.990 >> ± 4.071 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled16 0 avgt 25 2.358 >> ± 0.092 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled16 1 avgt 25 3.631 >> ± 0.159 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled16 10 avgt 25 7.049 >> ± 0.019 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled16 100 avgt 2533.626 >> ± 1.218 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled161000 avgt 25 317.811 >> ± 1.225 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled16 1 avgt 25 3212.333 >> ± 14.621 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled80 avgt 25 2.356 >> ± 0.097 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled81 avgt 25 3.630 >> ± 0.158 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled8 10 avgt 25 8.724 >> ± 0.065 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled8 100 avgt 2532.402 >> ± 0.019 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled8 1000 avgt 25 321.949 >> ± 0.251 ns/op >> StringHashCode.Algorithm.scalarLatin1Unrolled81 avgt 25 3202.083 >> ± 1.667 ns/op >> StringHashCode.Algorithm.scalarUTF16 0 avgt 25 2.135 >> ± 0.191 ns/op >> StringHashCode.Algorithm.scalarUTF16 1 avgt 25 5.202 >> ± 0.362 ns/op >> StringHashCode.Algorithm.scalarUTF16 10 avgt 2511.105 >> ± 0.112 ns/op >> StringHashCode.Algorithm.scalarUTF16100 avgt 2575.974 >> ± 0.702 ns/op >> StringHashCode.Algorithm.scalarUTF16 1000 avgt 25 716.429 >> ± 3.290 ns/op >> StringHashCode.Algorithm.scalarUTF16 1 avgt 25 7095.459 >> ± 43.847 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled160 avgt 25 2.381 >> ± 0.038 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled161 avgt 25 5.268 >> ± 0.422 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled16 10 avgt 2511.248 >> ± 0.178 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled16 100 avgt 2552.966 >> ± 0.089 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled16 1000 avgt 25 450.912 >> ± 1.834 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled161 avgt 25 4403.988 >> ± 2.927 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled8 0 avgt 25 2.401 >> ± 0.032 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled8 1 avgt 25 5.091 >> ± 0.396 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled810 avgt 2512.801 >> ± 0.189 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled8 100 avgt 2552.068 >> ± 0.032 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled8 1000 avgt 25 453.270 >> ± 0.340 ns/op >> StringHashCode.Algorithm.scalarUTF16Unrolled8 1 avgt 25 4433.112 >> ± 2.699 ns/op >> >> >> At Datadog, we handle a great amount of text (through logs management for >> example), and hashing String represents a large part of our CPU usage. It's >> very unlikely that we are the only one as String.hashCode is such a core >> feature of the JVM-based languages with its use in HashMap for example. >> Having even only a 2x speedup would allow us to save thousands of CPU cores >> per month and improve correspondingly the energy/carbon impact. >> >> [1] >> https://static.rainfocus.com/oracle/oow18/sess/1525822677955001tLqU/PF/codeone18-vector-API-DEV5081_1540354883936001Q3Sv.pdf > > Ludovic Henry has updated the pull request incrementally with one additional > commit since
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Mon, 7 Mar 2022 16:40:15 GMT, Lance Andersen wrote: > What problem are you having editing the PR header? You should be able to do > so as the author of the PR Exactly. You should see an "Edit" button near the right edge of the PR title. See the attached image:  - PR: https://git.openjdk.java.net/jdk/pull/7268
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v7]
> Several attempts have been made to improve Formatter's numeric performance by
> caching the current Locale zero. Such fixes, however, ignore the real issue,
> which is the slowness of fetching DecimalFormatSymbols. By directly caching
> DecimalFormatSymbols in the Formatter, this enhancement streamlines the
> process of accessing Locale DecimalFormatSymbols and specifically
> getZeroDigit(). The result is a general improvement in the performance of
> numeric formatting.
>
>
> @Benchmark
> public void bigDecimalDefaultLocale() {
> result = String.format("%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
> %1$f", X);
> }
>
> @Benchmark
> public void bigDecimalLocaleAlternate() {
> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
> %1$f %1$f", X);
> other = String.format(DEFAULT, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
> %1$f %1$f", X);
> }
>
> @Benchmark
> public void bigDecimalThaiLocale() {
> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
> %1$f %1$f", X);
> }
>
> @Benchmark
> public void integerDefaultLocale() {
> result = String.format("%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
> %1$d", x);
> }
>
> @Benchmark
> public void integerLocaleAlternate() {
> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
> %1$d %1$d", x);
> other = String.format(DEFAULT, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
> %1$d %1$d", x);
> }
>
> @Benchmark
> public void integerThaiLocale() {
> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
> %1$d %1$d", x);
> }
>
>
> Before:
> Benchmark Mode Cnt Score Error Units
> MyBenchmark.bigDecimalDefaultLocale thrpt 25 75498.923 ± 3686.966 ops/s
> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 39068.721 ± 162.983 ops/s
> MyBenchmark.bigDecimalThaiLocale thrpt 25 77256.530 ± 294.743 ops/s
> MyBenchmark.integerDefaultLocale thrpt 25 344093.071 ± 6189.002 ops/s
> MyBenchmark.integerLocaleAlternate thrpt 25 165685.488 ± 440.857 ops/s
> MyBenchmark.integerThaiLocale thrpt 25 327461.302 ± 1168.243 ops/s
>
> After:
> Benchmark Mode Cnt Score Error Units
> MyBenchmark.bigDecimalDefaultLocale thrpt 25 94735.293 ± 674.587 ops/s
> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 44215.547 ± 291.664 ops/s
> MyBenchmark.bigDecimalThaiLocale thrpt 25 91390.997 ± 658.677 ops/s
> MyBenchmark.integerDefaultLocale thrpt 25 363722.977 ± 2864.554 ops/s
> MyBenchmark.integerLocaleAlternate thrpt 25 165789.514 ± 779.656 ops/s
> MyBenchmark.integerThaiLocale thrpt 25 351400.818 ± 1030.246 ops/s
Jim Laskey has updated the pull request incrementally with four additional
commits since the last revision:
- Add comment about DecimalFormatSymbols being mutable objects.
- Revert "Cache DecimalFormatSymbols in DecimalFormatSymbols instead of
Formatter. No significant performance degradation."
This reverts commit fcbf66a2fe9641d3c3349f12cc7b32d8b84c6f72.
- Revert "Drop DecimalFormatSymbols.getLocale change"
This reverts commit b93cdb03ec68f24f4b8851c0966bb144c30b5110.
- Revert "Correct caching test"
This reverts commit bf7975396aaad4ed58d053bde8f54984215eeba5.
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/7703/files
- new: https://git.openjdk.java.net/jdk/pull/7703/files/bf797539..89354a0e
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=7703=06
- incr: https://webrevs.openjdk.java.net/?repo=jdk=7703=05-06
Stats: 34 lines in 2 files changed: 17 ins; 14 del; 3 mod
Patch: https://git.openjdk.java.net/jdk/pull/7703.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/7703/head:pull/7703
PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v6]
On Mon, 7 Mar 2022 17:09:37 GMT, Jim Laskey wrote:
>> Several attempts have been made to improve Formatter's numeric performance
>> by caching the current Locale zero. Such fixes, however, ignore the real
>> issue, which is the slowness of fetching DecimalFormatSymbols. By directly
>> caching DecimalFormatSymbols in the Formatter, this enhancement streamlines
>> the process of accessing Locale DecimalFormatSymbols and specifically
>> getZeroDigit(). The result is a general improvement in the performance of
>> numeric formatting.
>>
>>
>> @Benchmark
>> public void bigDecimalDefaultLocale() {
>> result = String.format("%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f", X);
>> }
>>
>> @Benchmark
>> public void bigDecimalLocaleAlternate() {
>> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> other = String.format(DEFAULT, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> }
>>
>> @Benchmark
>> public void bigDecimalThaiLocale() {
>> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> }
>>
>> @Benchmark
>> public void integerDefaultLocale() {
>> result = String.format("%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d", x);
>> }
>>
>> @Benchmark
>> public void integerLocaleAlternate() {
>> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> other = String.format(DEFAULT, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> }
>>
>> @Benchmark
>> public void integerThaiLocale() {
>> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> }
>>
>>
>> Before:
>> Benchmark Mode Cnt Score Error Units
>> MyBenchmark.bigDecimalDefaultLocale thrpt 25 75498.923 ± 3686.966 ops/s
>> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 39068.721 ± 162.983 ops/s
>> MyBenchmark.bigDecimalThaiLocale thrpt 25 77256.530 ± 294.743 ops/s
>> MyBenchmark.integerDefaultLocale thrpt 25 344093.071 ± 6189.002 ops/s
>> MyBenchmark.integerLocaleAlternate thrpt 25 165685.488 ± 440.857 ops/s
>> MyBenchmark.integerThaiLocale thrpt 25 327461.302 ± 1168.243 ops/s
>>
>> After:
>> Benchmark Mode Cnt Score Error Units
>> MyBenchmark.bigDecimalDefaultLocale thrpt 25 94735.293 ± 674.587 ops/s
>> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 44215.547 ± 291.664 ops/s
>> MyBenchmark.bigDecimalThaiLocale thrpt 25 91390.997 ± 658.677 ops/s
>> MyBenchmark.integerDefaultLocale thrpt 25 363722.977 ± 2864.554 ops/s
>> MyBenchmark.integerLocaleAlternate thrpt 25 165789.514 ± 779.656 ops/s
>> MyBenchmark.integerThaiLocale thrpt 25 351400.818 ± 1030.246 ops/s
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Correct caching test
Interesting fishing trip. DecimalFormatSymbols are mutable objects. As are
NumberFormat/DecimalFormal. Hence, caching in the Formatter is the correct
location.
-
PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: 8281093: Violating Attribute-Value Normalization in the XML specification 1.0
On Mon, 7 Mar 2022 17:07:20 GMT, Ravi Reddy wrote: > This fix is for violation of XML specification on Attribute-Value > normalization for external entities having character "\r". > > While normalizing entity with '\r', we should be checking if the entity is > external before changing the position and offset. "isExternal()" check is > missed in the new method : > normalizeNewlines(short version, XMLString buffer, boolean append,boolean > storeWS, NameType nt). > . Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7731
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v17]
On Sat, 5 Mar 2022 19:54:44 GMT, Joe Darcy wrote: >> This is an early review of changes to better model JVM access flags, that is >> "modifiers" like public, protected, etc. but explicitly at a VM level. >> >> Language level modifiers and JVM level access flags are closely related, but >> distinct. There are concepts that overlap in the two domains (public, >> private, etc.), others that only have a language-level modifier (sealed), >> and still others that only have an access flag (synthetic). >> >> The existing java.lang.reflect.Modifier class is inadequate to model these >> subtleties. For example, the bit positions used by access flags on different >> kinds of elements overlap (such as "volatile" for fields and "bridge" for >> methods. Just having a raw integer does not provide sufficient context to >> decode the corresponding language-level string. Methods like >> Modifier.methodModifiers() were introduced to cope with this situation. >> >> With additional modifiers and flags on the horizon with projects like >> Valhalla, addressing the existent modeling deficiency now ahead of time is >> reasonable before further strain is introduced. >> >> This PR in its current form is meant to give the overall shape of the API. >> It is missing implementations to map from, say, method modifiers to access >> flags, taking into account overlaps in bit positions. >> >> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in >> once the API is further along. > > Joe Darcy 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 26 additional commits since > the last revision: > > - Respond to review feedback. > - Merge branch 'master' into JDK-8266670 > - Make workding changes suggested in review feedback. > - Merge branch 'master' into JDK-8266670 > - Typo fix; add implSpec to Executable. > - Appease jcheck. > - Fix some bugs found by inspection, docs cleanup. > - Merge branch 'master' into JDK-8266670 > - Initial support for accessFlags methods > - Add mask to access flag functionality. > - ... and 16 more: > https://git.openjdk.java.net/jdk/compare/aa899b84...14980605 src/java.base/share/classes/java/lang/Class.java line 1334: > 1332: // allows PRIVATE, PROTECTED, and STATIC, which are not > 1333: // allowed on Location.CLASS. > 1334: return AccessFlag.maskToAccessFlags(getModifiers(), Computing and creating the Set every time seems like a high overhead operation (compared to getModifiers()). Caching either here (in the Member) or in AccessFlag.maskToAccessFlags would be desirable. Caching is idempotent so it should not need synchronization. - PR: https://git.openjdk.java.net/jdk/pull/7445
Re: RFR: 8279185: Support for IsoFields in JapaneseDate/MinguoDate/ThaiBuddhistDate [v4]
On Mon, 7 Mar 2022 18:30:28 GMT, Joe Wang wrote:
>> Naoto Sato 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 five additional
>> commits since the last revision:
>>
>> - Renamed the new method
>> - Merge branch 'master' into JDK-8279185
>> - Addresses review comments
>> - copyright year fix
>> - 8279185: Support for IsoFields in JapaneseDate/MinguoDate/ThaiBuddhistDate
>
> src/java.base/share/classes/java/time/chrono/IsoChronology.java line 689:
>
>> 687: */
>> 688: @Override
>> 689: public boolean isIsoBased() {
>
> Is this meant to be 'default'? The CSR indicated adding default methods.
The `default` method has been added to `java.time.chrono.Chronology` interface.
This is its overridden method implemented in `IsoChronology` concrete class.
-
PR: https://git.openjdk.java.net/jdk/pull/7683
Re: RFR: 8279185: Support for IsoFields in JapaneseDate/MinguoDate/ThaiBuddhistDate [v4]
> Supporting `IsoFields` temporal fields in chronologies that are similar to > ISO chronology. Corresponding CSR has also been drafted. Naoto Sato 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 five additional commits since the last revision: - Renamed the new method - Merge branch 'master' into JDK-8279185 - Addresses review comments - copyright year fix - 8279185: Support for IsoFields in JapaneseDate/MinguoDate/ThaiBuddhistDate - Changes: - all: https://git.openjdk.java.net/jdk/pull/7683/files - new: https://git.openjdk.java.net/jdk/pull/7683/files/e0b329d7..530ed40e Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7683=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7683=02-03 Stats: 12800 lines in 349 files changed: 8488 ins; 2645 del; 1667 mod Patch: https://git.openjdk.java.net/jdk/pull/7683.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7683/head:pull/7683 PR: https://git.openjdk.java.net/jdk/pull/7683
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Mon, 7 Mar 2022 17:12:25 GMT, Magnus Ihse Bursie wrote: > TheShermanTanker is not the author of this PR, he's just assisting the author > by creating the JBS issue. Ah, that explains it then. - PR: https://git.openjdk.java.net/jdk/pull/7268
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Mon, 7 Mar 2022 16:40:15 GMT, Lance Andersen wrote: >> Hi >> >> I have reviewed the code for removing double semicolons at the end of lines >> >> all the best >> matteo > > What problem are you having editing the PR header? You should be able to do > so as the author of the PR @LanceAndersen @kevinrushforth TheShermanTanker is not the author of this PR, he's just assisting the author by creating the JBS issue. - PR: https://git.openjdk.java.net/jdk/pull/7268
RFR: 8281093: Violating Attribute-Value Normalization in the XML specification 1.0
This fix is for violation of XML specification on Attribute-Value normalization for external entities having character "\r". While normalizing entity with '\r', we should be checking if the entity is external before changing the position and offset. "isExternal()" check is missed in the new method : normalizeNewlines(short version, XMLString buffer, boolean append,boolean storeWS, NameType nt). . - Commit messages: - 8281093: Violating Attribute-Value Normalization in the XML specification 1.0 Changes: https://git.openjdk.java.net/jdk/pull/7731/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7731=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8281093 Stats: 3 lines in 1 file changed: 0 ins; 0 del; 3 mod Patch: https://git.openjdk.java.net/jdk/pull/7731.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7731/head:pull/7731 PR: https://git.openjdk.java.net/jdk/pull/7731
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v6]
> Several attempts have been made to improve Formatter's numeric performance by
> caching the current Locale zero. Such fixes, however, ignore the real issue,
> which is the slowness of fetching DecimalFormatSymbols. By directly caching
> DecimalFormatSymbols in the Formatter, this enhancement streamlines the
> process of accessing Locale DecimalFormatSymbols and specifically
> getZeroDigit(). The result is a general improvement in the performance of
> numeric formatting.
>
>
> @Benchmark
> public void bigDecimalDefaultLocale() {
> result = String.format("%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
> %1$f", X);
> }
>
> @Benchmark
> public void bigDecimalLocaleAlternate() {
> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
> %1$f %1$f", X);
> other = String.format(DEFAULT, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
> %1$f %1$f", X);
> }
>
> @Benchmark
> public void bigDecimalThaiLocale() {
> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
> %1$f %1$f", X);
> }
>
> @Benchmark
> public void integerDefaultLocale() {
> result = String.format("%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
> %1$d", x);
> }
>
> @Benchmark
> public void integerLocaleAlternate() {
> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
> %1$d %1$d", x);
> other = String.format(DEFAULT, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
> %1$d %1$d", x);
> }
>
> @Benchmark
> public void integerThaiLocale() {
> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
> %1$d %1$d", x);
> }
>
>
> Before:
> Benchmark Mode Cnt Score Error Units
> MyBenchmark.bigDecimalDefaultLocale thrpt 25 75498.923 ± 3686.966 ops/s
> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 39068.721 ± 162.983 ops/s
> MyBenchmark.bigDecimalThaiLocale thrpt 25 77256.530 ± 294.743 ops/s
> MyBenchmark.integerDefaultLocale thrpt 25 344093.071 ± 6189.002 ops/s
> MyBenchmark.integerLocaleAlternate thrpt 25 165685.488 ± 440.857 ops/s
> MyBenchmark.integerThaiLocale thrpt 25 327461.302 ± 1168.243 ops/s
>
> After:
> Benchmark Mode Cnt Score Error Units
> MyBenchmark.bigDecimalDefaultLocale thrpt 25 94735.293 ± 674.587 ops/s
> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 44215.547 ± 291.664 ops/s
> MyBenchmark.bigDecimalThaiLocale thrpt 25 91390.997 ± 658.677 ops/s
> MyBenchmark.integerDefaultLocale thrpt 25 363722.977 ± 2864.554 ops/s
> MyBenchmark.integerLocaleAlternate thrpt 25 165789.514 ± 779.656 ops/s
> MyBenchmark.integerThaiLocale thrpt 25 351400.818 ± 1030.246 ops/s
Jim Laskey has updated the pull request incrementally with one additional
commit since the last revision:
Correct caching test
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/7703/files
- new: https://git.openjdk.java.net/jdk/pull/7703/files/b93cdb03..bf797539
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=7703=05
- incr: https://webrevs.openjdk.java.net/?repo=jdk=7703=04-05
Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
Patch: https://git.openjdk.java.net/jdk/pull/7703.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/7703/head:pull/7703
PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: 8279185: Support for IsoFields in JapaneseDate/MinguoDate/ThaiBuddhistDate [v3]
On Mon, 7 Mar 2022 03:00:45 GMT, Roger Riggs wrote: >> OK, I propose `isIsoBased()` for the name, which I initially thought of. If >> there is no objection, I will modify the spec/impl. > > Is `IsoBased` is fine with me. "isISOLike" is too vague. That matches the javadoc as well, that it "supports ISO based fields". - PR: https://git.openjdk.java.net/jdk/pull/7683
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Fri, 28 Jan 2022 14:39:31 GMT, Matteo Baccan wrote: > Hi > > I have reviewed the code for removing double semicolons at the end of lines > > all the best > matteo But as the JBS title and PR title now match, this is a moot point. - PR: https://git.openjdk.java.net/jdk/pull/7268
Re: RFR: 8282008: Incorrect handling of quoted arguments in ProcessBuilder [v4]
On Thu, 3 Mar 2022 15:38:44 GMT, Olga Mikhaltsova
wrote:
>> This fix made equal processing of strings such as ""C:\\Program
>> Files\\Git\\"" before and after JDK-8250568.
>>
>> For example, it's needed to execute the following command on Windows:
>> `C:\Windows\SysWOW64\WScript.exe "MyVB.vbs" "C:\Program Files\Git" "Test"`
>> it's equal to:
>> `new ProcessBuilder("C:\\Windows\\SysWOW64\\WScript.exe", "MyVB.vbs",
>> ""C:\\Program Files\\Git\\"", "Test").start();`
>>
>> While processing, the 3rd argument ""C:\\Program Files\\Git\\"" treated as
>> unquoted due to the condition added in JDK-8250568.
>>
>> private static String unQuote(String str) {
>> ..
>>if (str.endsWith("\\"")) {
>> return str;// not properly quoted, treat as unquoted
>> }
>> ..
>> }
>>
>>
>> that leads to the additional surrounding by quotes in
>> ProcessImpl::createCommandLine(..) because needsEscaping(..) returns true
>> due to the space inside the string argument.
>> As a result the native function CreateProcessW
>> (src/java.base/windows/native/libjava/ProcessImpl_md.c) gets the incorrectly
>> quoted argument:
>>
>> pcmd = C:\Windows\SysWOW64\WScript.exe MyVB.vbs ""C:\Program Files\Git"" Test
>> (jdk.lang.Process.allowAmbiguousCommands = true)
>> pcmd = "C:\Windows\SysWOW64\WScript.exe" MyVB.vbs ""C:\Program Files\Git\\""
>> Test
>> (jdk.lang.Process.allowAmbiguousCommands = false)
>>
>>
>> Obviously, a string ending with `"\\""` must not be started with `"""` to
>> treat as unquoted overwise it’s should be treated as properly quoted.
>
> Olga Mikhaltsova has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Reverted addition of the test via echo
As an alternative fix, please take a look at Draft PR:
https://github.com/openjdk/jdk/pull/7709.
In the default handling of arguments, the check for what is quoted is reverted
to prior to 8255068. First and last quotes are sufficient to identify a
"quoted" string. The check for a backslash ("\") is removed.
This original check is sufficient for `jdk.lang.Process.allowAmbiguousCommands
= true`.
For the case where the system property `jdk.lang.Process.allowAmbiguousCommands
= false`
and the argument has first and last quotes, a backslash ("\") before the final
quote must not allow the quote to interpreted as a literal quote and merge the
following argument. The backslashes will doubled to prevent the interpretation
of the quote as a literal. This is the correct encoding if the command uses
the ".exe" encoding, when reparsing the arguments the doubled backslashes are
reduced to the original contents.
When the command is using the simpler parsing that does not support literal
quotes, the backslash before the quote is typically is a trailing backslash on
a file path and in that case the additional backslash is redundant and has no
effect on the interpretation of the argument as a directory path.
The PR includes a test of the 12 combinations of invoking an "java"/.exe
program, a .cmd script, and a Visual Basic script (which uses the .exe rules
but different command line parser); with and without application quotes and
compares the actual results with the expected arguments.
-
PR: https://git.openjdk.java.net/jdk/pull/7504
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Fri, 28 Jan 2022 14:39:31 GMT, Matteo Baccan wrote: > Hi > > I have reviewed the code for removing double semicolons at the end of lines > > all the best > matteo What problem are you having editing the PR header? You should be able to do so as the author of the PR - PR: https://git.openjdk.java.net/jdk/pull/7268
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Mon, 7 Mar 2022 13:40:48 GMT, Erik Joelsson wrote: > > Should I change the JBS issue title to match the PR title, or is it > > preferred for the PR title to change? > > They need to match. You can either do it manually, or change the title to > just the bug number and the bot will change it for you. Alright, I can't change the title of the PR, I guess it'll be easier for me to change the issue instead - PR: https://git.openjdk.java.net/jdk/pull/7268
Integrated: 8280404: Unexpected exception thrown when CEN file entry comment length is not valid
On Thu, 3 Mar 2022 11:19:12 GMT, Lance Andersen wrote: > Hi all, > > This PR addresses an issue where an unexpected exception is thrown when the > CEN file entry comment length is not correct. > > Mach5 tiers 1 - 3 run clean with this change. This pull request has now been integrated. Changeset: f0995abe Author:Lance Andersen URL: https://git.openjdk.java.net/jdk/commit/f0995abe62b81cf9c96cc07caa0ac27d00c96ff1 Stats: 355 lines in 2 files changed: 354 ins; 0 del; 1 mod 8280404: Unexpected exception thrown when CEN file entry comment length is not valid Reviewed-by: alanb - PR: https://git.openjdk.java.net/jdk/pull/7673
Re: RFR: 8282662: Use List/Set.of() factory methods to reduce memory consumption
On Mon, 7 Mar 2022 15:54:02 GMT, liach wrote: > Notice list.of will have the downside of copying the input array when the > size is not small while arrays aslist does not. Is the tradeoff worth it? A good observation. In a couple of these places we could probably use `JavaUtilCollectionAccess.listFromTrustedArray` to avoid such copies. - PR: https://git.openjdk.java.net/jdk/pull/7729
Re: RFR: 8282662: Use List/Set.of() factory methods to reduce memory consumption
On Mon, 7 Mar 2022 15:11:50 GMT, Сергей Цыпанов wrote: > `List.of()` along with `Set.of()` create unmodifiable `List/Set` but with > smaller footprint comparing to `Arrays.asList()` / `new HashSet()` when > called with vararg of size 0, 1, 2. > > In general replacement of `Arrays.asList()` with `List.of()` is dubious as > the latter is null-hostile, however in some cases we are sure that arguments > are non-null. Into this PR I've included the following cases (in addition to > those where the argument is proved to be non-null at compile-time): > - `MethodHandles.longestParameterList()` never returns null > - parameter types are never null > - interfaces used for proxy construction and returned from > `Class.getInterfaces()` are never null > - exceptions types of method signature are never null Notice list.of will have the downside of copying the input array when the size is not small while arrays aslist does not. Is the tradeoff worth it? - PR: https://git.openjdk.java.net/jdk/pull/7729
Re: RFR: 8282662: Use List/Set.of() factory methods to reduce memory consumption
On Mon, 7 Mar 2022 15:11:50 GMT, Сергей Цыпанов wrote: > `List.of()` along with `Set.of()` create unmodifiable `List/Set` but with > smaller footprint comparing to `Arrays.asList()` / `new HashSet()` when > called with vararg of size 0, 1, 2. > > In general replacement of `Arrays.asList()` with `List.of()` is dubious as > the latter is null-hostile, however in some cases we are sure that arguments > are non-null. Into this PR I've included the following cases (in addition to > those where the argument is proved to be non-null at compile-time): > - `MethodHandles.longestParameterList()` never returns null > - parameter types are never null > - interfaces used for proxy construction and returned from > `Class.getInterfaces()` are never null > - exceptions types of method signature are never null There's also another difference: they have different serial forms. From my cursory inspection it doesn't look like the returned list are put in serializable fields but it could be good to double check it. - PR: https://git.openjdk.java.net/jdk/pull/7729
RFR: 8282662: Use List/Set.of() factory methods to reduce memory consumption
`List.of()` along with `Set.of()` create unmodifiable `List/Set` but with smaller footprint comparing to `Arrays.asList()` / `new HashSet()` when called with vararg of size 0, 1, 2. In general replacement of `Arrays.asList()` with `List.of()` is dubious as the latter is null-hostile, however in some cases we are sure that arguments are non-null. Into this PR I've included the following cases (in addition to those where the argument is proved to be non-null at compile-time): - `MethodHandles.longestParameterList()` never returns null - parameter types are never null - interfaces used for proxy construction and returned from `Class.getInterfaces()` are never null - exceptions types of method signature are never null - Commit messages: - 8282662: Use List/Set.of() factory methods to save memory Changes: https://git.openjdk.java.net/jdk/pull/7729/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7729=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8282662 Stats: 22 lines in 8 files changed: 1 ins; 3 del; 18 mod Patch: https://git.openjdk.java.net/jdk/pull/7729.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7729/head:pull/7729 PR: https://git.openjdk.java.net/jdk/pull/7729
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v5]
> Several attempts have been made to improve Formatter's numeric performance by
> caching the current Locale zero. Such fixes, however, ignore the real issue,
> which is the slowness of fetching DecimalFormatSymbols. By directly caching
> DecimalFormatSymbols in the Formatter, this enhancement streamlines the
> process of accessing Locale DecimalFormatSymbols and specifically
> getZeroDigit(). The result is a general improvement in the performance of
> numeric formatting.
>
>
> @Benchmark
> public void bigDecimalDefaultLocale() {
> result = String.format("%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
> %1$f", X);
> }
>
> @Benchmark
> public void bigDecimalLocaleAlternate() {
> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
> %1$f %1$f", X);
> other = String.format(DEFAULT, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
> %1$f %1$f", X);
> }
>
> @Benchmark
> public void bigDecimalThaiLocale() {
> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
> %1$f %1$f", X);
> }
>
> @Benchmark
> public void integerDefaultLocale() {
> result = String.format("%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
> %1$d", x);
> }
>
> @Benchmark
> public void integerLocaleAlternate() {
> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
> %1$d %1$d", x);
> other = String.format(DEFAULT, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
> %1$d %1$d", x);
> }
>
> @Benchmark
> public void integerThaiLocale() {
> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
> %1$d %1$d", x);
> }
>
>
> Before:
> Benchmark Mode Cnt Score Error Units
> MyBenchmark.bigDecimalDefaultLocale thrpt 25 75498.923 ± 3686.966 ops/s
> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 39068.721 ± 162.983 ops/s
> MyBenchmark.bigDecimalThaiLocale thrpt 25 77256.530 ± 294.743 ops/s
> MyBenchmark.integerDefaultLocale thrpt 25 344093.071 ± 6189.002 ops/s
> MyBenchmark.integerLocaleAlternate thrpt 25 165685.488 ± 440.857 ops/s
> MyBenchmark.integerThaiLocale thrpt 25 327461.302 ± 1168.243 ops/s
>
> After:
> Benchmark Mode Cnt Score Error Units
> MyBenchmark.bigDecimalDefaultLocale thrpt 25 94735.293 ± 674.587 ops/s
> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 44215.547 ± 291.664 ops/s
> MyBenchmark.bigDecimalThaiLocale thrpt 25 91390.997 ± 658.677 ops/s
> MyBenchmark.integerDefaultLocale thrpt 25 363722.977 ± 2864.554 ops/s
> MyBenchmark.integerLocaleAlternate thrpt 25 165789.514 ± 779.656 ops/s
> MyBenchmark.integerThaiLocale thrpt 25 351400.818 ± 1030.246 ops/s
Jim Laskey has updated the pull request incrementally with one additional
commit since the last revision:
Drop DecimalFormatSymbols.getLocale change
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/7703/files
- new: https://git.openjdk.java.net/jdk/pull/7703/files/fcbf66a2..b93cdb03
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=7703=04
- incr: https://webrevs.openjdk.java.net/?repo=jdk=7703=03-04
Stats: 9 lines in 1 file changed: 0 ins; 9 del; 0 mod
Patch: https://git.openjdk.java.net/jdk/pull/7703.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/7703/head:pull/7703
PR: https://git.openjdk.java.net/jdk/pull/7703
Integrated: JDK-8282696: Add constructors taking a cause to InvalidObjectException and InvalidClassException
On Sat, 5 Mar 2022 02:57:47 GMT, Joe Darcy wrote: > Occasionally in core-libs we've discussed whether or not to do a pass over > the exception classes and proactively add any of four missing convention > constructors per java.lang.Throwable (no-arg, string, cause, cause and > string). Last time this came up, we decided a wide-scale effort wasn't > worthwhile. > > Prompted by some other recent work, I decided to take a quick look at the > dual-approach: grep for calls to initCause in java.base and seeing which > exception classes repeated were initCaused. Those exception classes are good > candidates for cause-taking constructors. > > Two such exception classes area InvalidObjectException and > InvalidClassException, along with their superclass ObjectStreamException. > > Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282697 This pull request has now been integrated. Changeset: 104e3cb2 Author:Joe Darcy URL: https://git.openjdk.java.net/jdk/commit/104e3cb24b4de5512abf9f5491f9c530b26838d3 Stats: 212 lines in 7 files changed: 176 ins; 14 del; 22 mod 8282696: Add constructors taking a cause to InvalidObjectException and InvalidClassException Reviewed-by: lancea - PR: https://git.openjdk.java.net/jdk/pull/7711
Re: RFR: JDK-8282696: Add constructors taking a cause to InvalidObjectException and InvalidClassException [v6]
> Occasionally in core-libs we've discussed whether or not to do a pass over > the exception classes and proactively add any of four missing convention > constructors per java.lang.Throwable (no-arg, string, cause, cause and > string). Last time this came up, we decided a wide-scale effort wasn't > worthwhile. > > Prompted by some other recent work, I decided to take a quick look at the > dual-approach: grep for calls to initCause in java.base and seeing which > exception classes repeated were initCaused. Those exception classes are good > candidates for cause-taking constructors. > > Two such exception classes area InvalidObjectException and > InvalidClassException, along with their superclass ObjectStreamException. > > Please also review the CSR: https://bugs.openjdk.java.net/browse/JDK-8282697 Joe Darcy has updated the pull request incrementally with one additional commit since the last revision: Reflow per review comments. - Changes: - all: https://git.openjdk.java.net/jdk/pull/7711/files - new: https://git.openjdk.java.net/jdk/pull/7711/files/2de370f7..2aeb7f03 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=7711=05 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7711=04-05 Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/7711.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/7711/head:pull/7711 PR: https://git.openjdk.java.net/jdk/pull/7711
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v4]
> Several attempts have been made to improve Formatter's numeric performance by
> caching the current Locale zero. Such fixes, however, ignore the real issue,
> which is the slowness of fetching DecimalFormatSymbols. By directly caching
> DecimalFormatSymbols in the Formatter, this enhancement streamlines the
> process of accessing Locale DecimalFormatSymbols and specifically
> getZeroDigit(). The result is a general improvement in the performance of
> numeric formatting.
>
>
> @Benchmark
> public void bigDecimalDefaultLocale() {
> result = String.format("%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
> %1$f", X);
> }
>
> @Benchmark
> public void bigDecimalLocaleAlternate() {
> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
> %1$f %1$f", X);
> other = String.format(DEFAULT, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
> %1$f %1$f", X);
> }
>
> @Benchmark
> public void bigDecimalThaiLocale() {
> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
> %1$f %1$f", X);
> }
>
> @Benchmark
> public void integerDefaultLocale() {
> result = String.format("%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
> %1$d", x);
> }
>
> @Benchmark
> public void integerLocaleAlternate() {
> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
> %1$d %1$d", x);
> other = String.format(DEFAULT, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
> %1$d %1$d", x);
> }
>
> @Benchmark
> public void integerThaiLocale() {
> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
> %1$d %1$d", x);
> }
>
>
> Before:
> Benchmark Mode Cnt Score Error Units
> MyBenchmark.bigDecimalDefaultLocale thrpt 25 75498.923 ± 3686.966 ops/s
> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 39068.721 ± 162.983 ops/s
> MyBenchmark.bigDecimalThaiLocale thrpt 25 77256.530 ± 294.743 ops/s
> MyBenchmark.integerDefaultLocale thrpt 25 344093.071 ± 6189.002 ops/s
> MyBenchmark.integerLocaleAlternate thrpt 25 165685.488 ± 440.857 ops/s
> MyBenchmark.integerThaiLocale thrpt 25 327461.302 ± 1168.243 ops/s
>
> After:
> Benchmark Mode Cnt Score Error Units
> MyBenchmark.bigDecimalDefaultLocale thrpt 25 94735.293 ± 674.587 ops/s
> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 44215.547 ± 291.664 ops/s
> MyBenchmark.bigDecimalThaiLocale thrpt 25 91390.997 ± 658.677 ops/s
> MyBenchmark.integerDefaultLocale thrpt 25 363722.977 ± 2864.554 ops/s
> MyBenchmark.integerLocaleAlternate thrpt 25 165789.514 ± 779.656 ops/s
> MyBenchmark.integerThaiLocale thrpt 25 351400.818 ± 1030.246 ops/s
Jim Laskey has updated the pull request incrementally with two additional
commits since the last revision:
- Cache DecimalFormatSymbols in DecimalFormatSymbols instead of Formatter. No
significant performance degradation.
- Edits from PR comments
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/7703/files
- new: https://git.openjdk.java.net/jdk/pull/7703/files/9ac0c283..fcbf66a2
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=7703=03
- incr: https://webrevs.openjdk.java.net/?repo=jdk=7703=02-03
Stats: 36 lines in 2 files changed: 17 ins; 9 del; 10 mod
Patch: https://git.openjdk.java.net/jdk/pull/7703.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/7703/head:pull/7703
PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v3]
On Mon, 7 Mar 2022 13:05:09 GMT, Jim Laskey wrote: >> Would it be just as effective and improve performance more broadly to cache >> in DecimalFormatSymbols.getInstance()? >> >> Declarations should be private unless there is a package need. >> In this case, the only access to should be via the method. > > Will investigate Performance is on par when cached in DecimalFormatSymbols. Will switch to that direction. - PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: 8282657: Code cleanup: removing double semicolons at the end of lines
On Sat, 5 Mar 2022 06:49:16 GMT, Julian Waters wrote: > Should I change the JBS issue title to match the PR title, or is it preferred > for the PR title to change? They need to match. You can either do it manually, or change the title to just the bug number and the bot will change it for you. - PR: https://git.openjdk.java.net/jdk/pull/7268
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v3]
On Mon, 7 Mar 2022 08:25:19 GMT, Stephen Colebourne wrote: >> Jim Laskey has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Too many 'the' > > Just to note that there is also some caching in `DecimalStyle`. It might be > possible to reuse `DecimalStyle` here (if it was extended to support grouping > separators). @jodastephen True that. It would also be nice if `DecimalFormatSymbols` also contained grouping size. Will investigate but will use a separate PR for the fallout. - PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v3]
On Fri, 4 Mar 2022 20:04:42 GMT, Roger Riggs wrote:
>> Jim Laskey has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Too many 'the'
>
> src/java.base/share/classes/java/util/Formatter.java line 2025:
>
>> 2023:
>> 2024: // Caching zero.
>> 2025: static char getZero(Locale locale) {
>
> Perhaps should be private.
> The comment says caching zero, but its really the DecimalFormatSymbols that
> are cached.
Noted.
-
PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v3]
On Mon, 7 Mar 2022 00:33:53 GMT, Naoto Sato wrote: >>> will now try and update/use this cached class level static state DFS. That >>> would thus require some kind of thread safety semantics to be implemented >>> for this new getDecimalFormatSymbols(Locale) method, isn't it? >> >> A bit more closer look at the code and it appears to me that the use of : >> >> DecimalFormatSymbols dfs = DFS; >> >> and then working off that local variable prevents any kind of race issues >> that might be caused due to multi-thread access. Of course it still means >> that multiple threads might still go ahead and do a: >> >> dfs = DecimalFormatSymbols.getInstance(locale); >> >> on first access (when `DFS` is null) but that in itself should be harmless. >> >> If this is intentional (which I suspect it is), should some comment be added >> in this method explaining this multi-thread access detail? > > Initially, I thought of the same thing (not the `Formatter` but > `DecimalFormatSymbols` itself, as it is also not thread safe), but I > concluded it was OK, as there is no mutation going on. I agree with Jaikiran > that some comments might help here. Noted - PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v3]
On Fri, 4 Mar 2022 19:02:29 GMT, Naoto Sato wrote:
>> Jim Laskey has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Too many 'the'
>
> src/java.base/share/classes/java/util/Formatter.java line 2026:
>
>> 2024: // Caching zero.
>> 2025: static char getZero(Locale locale) {
>> 2026: return locale == null ? '0' :
>> getDecimalFormatSymbols(locale).getZeroDigit();
>
> While we are at it, it would be beneficial to cache locale-dependent grouping
> and decimal separators too.
Noted
-
PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v3]
On Fri, 4 Mar 2022 21:14:26 GMT, Roger Riggs wrote: >> As a separate/future issue, perhaps the constructors should be deprecated to >> nudge people to using the static `getInstance` methods. > > Would it be just as effective and improve performance more broadly to cache > in DecimalFormatSymbols.getInstance()? > > Declarations should be private unless there is a package need. > In this case, the only access to should be via the method. Will investigate - PR: https://git.openjdk.java.net/jdk/pull/7703
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v17]
On Sat, 5 Mar 2022 19:54:44 GMT, Joe Darcy wrote: >> This is an early review of changes to better model JVM access flags, that is >> "modifiers" like public, protected, etc. but explicitly at a VM level. >> >> Language level modifiers and JVM level access flags are closely related, but >> distinct. There are concepts that overlap in the two domains (public, >> private, etc.), others that only have a language-level modifier (sealed), >> and still others that only have an access flag (synthetic). >> >> The existing java.lang.reflect.Modifier class is inadequate to model these >> subtleties. For example, the bit positions used by access flags on different >> kinds of elements overlap (such as "volatile" for fields and "bridge" for >> methods. Just having a raw integer does not provide sufficient context to >> decode the corresponding language-level string. Methods like >> Modifier.methodModifiers() were introduced to cope with this situation. >> >> With additional modifiers and flags on the horizon with projects like >> Valhalla, addressing the existent modeling deficiency now ahead of time is >> reasonable before further strain is introduced. >> >> This PR in its current form is meant to give the overall shape of the API. >> It is missing implementations to map from, say, method modifiers to access >> flags, taking into account overlaps in bit positions. >> >> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in >> once the API is further along. > > Joe Darcy 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 26 additional commits since > the last revision: > > - Respond to review feedback. > - Merge branch 'master' into JDK-8266670 > - Make workding changes suggested in review feedback. > - Merge branch 'master' into JDK-8266670 > - Typo fix; add implSpec to Executable. > - Appease jcheck. > - Fix some bugs found by inspection, docs cleanup. > - Merge branch 'master' into JDK-8266670 > - Initial support for accessFlags methods > - Add mask to access flag functionality. > - ... and 16 more: > https://git.openjdk.java.net/jdk/compare/6eb09aa6...14980605 The mapping from Location to AccessFlag(s) could be implemented event without using a Map. You just have to be careful not to use EnumSet for that (i.e. before AccessFlag enum constants are fully initialized) - an ArrayList is better for this case anyway. Also, conversion from ModuleDescriptor modifier(s) to AccessFlag(s) could be done without first creating a bitmask and then re-constructing a set of AccessFlag(s) from it. Reflective object modifiers can only be translated via bitmask, but various ModuleDescriptor Modifier(s) may have a direct mapping to corresponding AccessFlag(s). For example: https://github.com/plevart/jdk/commit/5a3cd8f6851a0deae7e3e5028bba9a51d7863929 - PR: https://git.openjdk.java.net/jdk/pull/7445
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v12]
On Mon, 28 Feb 2022 18:20:13 GMT, ExE Boss wrote:
>> It does because of the AccessFlags.MODULE constant.
>
>> It does because of the AccessFlags.MODULE constant.
>
> But that’s exactly what the unqualified `MODULE` identifier refers to, and
> there’s no other bare `MODULE` identifier in scope that would shadow the
> `AccessFlag.MODULE` constant from the POV of `AccessFlag.LocationToFlags`.
This is just implementation detail, but I think the reverse mapping from
Location to AccessFlag(s) could be established implicitly during the
initialization of the AccessFlag enum since it is only used from within code of
that enum class. Like this:
Index: src/java.base/share/classes/java/lang/reflect/AccessFlag.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===
diff --git a/src/java.base/share/classes/java/lang/reflect/AccessFlag.java
b/src/java.base/share/classes/java/lang/reflect/AccessFlag.java
--- a/src/java.base/share/classes/java/lang/reflect/AccessFlag.java
(revision 1498060544413cb67de8b1a82fbbf15d388d62c3)
+++ b/src/java.base/share/classes/java/lang/reflect/AccessFlag.java (date
1646651355828)
@@ -26,8 +26,13 @@
package java.lang.reflect;
import java.util.Collections;
+import java.util.EnumMap;
+import java.util.EnumSet;
+import java.util.HashMap;
import java.util.Map;
import java.util.Set;
+import java.util.function.Function;
+
import static java.util.Map.entry;
/**
@@ -248,6 +253,14 @@
this.mask = mask;
this.sourceModifier = sourceModifier;
this.locations = locations;
+for (var location : locations) {
+LocationToFlags.MAP.computeIfAbsent(location, new Function<>() {
+@Override
+public Set apply(Location location) {
+return EnumSet.noneOf(AccessFlag.class);
+}
+}).add(this);
+}
}
/**
@@ -283,7 +296,7 @@
*/
public static Set maskToAccessFlags(int mask, Location
location) {
Set result = java.util.EnumSet.noneOf(AccessFlag.class);
-for (var accessFlag : LocationToFlags.locationToFlags.get(location)) {
+for (var accessFlag : LocationToFlags.MAP.get(location)) {
int accessMask = accessFlag.mask();
if ((mask & accessMask) != 0) {
result.add(accessFlag);
@@ -363,34 +376,7 @@
}
private static class LocationToFlags {
-private static Map> locationToFlags =
-Map.ofEntries(entry(Location.CLASS,
-Set.of(PUBLIC, FINAL, SUPER,
- INTERFACE, ABSTRACT,
- SYNTHETIC, ANNOTATION,
- ENUM, AccessFlag.MODULE)),
- entry(Location.FIELD,
-Set.of(PUBLIC, PRIVATE, PROTECTED,
- STATIC, FINAL, VOLATILE,
- TRANSIENT, SYNTHETIC, ENUM)),
- entry(Location.METHOD,
-Set.of(PUBLIC, PRIVATE, PROTECTED,
- STATIC, FINAL, SYNCHRONIZED,
- BRIDGE, VARARGS, NATIVE,
- ABSTRACT, STRICT, SYNTHETIC)),
- entry(Location.INNER_CLASS,
-Set.of(PUBLIC, PRIVATE, PROTECTED,
- STATIC, FINAL, INTERFACE, ABSTRACT,
- SYNTHETIC, ANNOTATION, ENUM)),
- entry(Location.METHOD_PARAMETER,
-Set.of(FINAL, SYNTHETIC, MANDATED)),
- entry(Location.MODULE,
-Set.of(OPEN, SYNTHETIC, MANDATED)),
- entry(Location.MODULE_REQUIRES,
-Set.of(TRANSITIVE, STATIC_PHASE, SYNTHETIC,
MANDATED)),
- entry(Location.MODULE_EXPORTS,
-Set.of(SYNTHETIC, MANDATED)),
- entry(Location.MODULE_OPENS,
-Set.of(SYNTHETIC, MANDATED)));
+private static final Map> MAP =
+new EnumMap<>(Location.class);
}
}
-
PR: https://git.openjdk.java.net/jdk/pull/7445
Re: RFR: 8282429: StringBuilder/StringBuffer.toString() skip compressing for UTF16 strings
On Thu, 3 Mar 2022 02:36:58 GMT, Xin Liu wrote: > If AbstractStringBuilder only grow, the inflated value which has been encoded > in UTF16 can't be compressed. > toString() can skip compression in this case. This can save an > ArrayAllocation in StringUTF16::compress(). > > java.io.BufferedRead::readLine() is a case that StringBuilder grows only. > > In microbench, we expect to see that allocation/op reduces 20%. The initial > capacity of StringBuilder is S in bytes. When it encounters the 1st character > that can't be encoded in LATIN1, it inflates and allocate a new array of 2*S. > `toString()` will try to compress that value so it need to allocate S bytes. > The last step allocates 2*S bytes because it has to copy the string. so it > requires to allocate 5 * S bytes in total. By skipping the failed > compression, it only allocates 4 * S bytes. that's 20%. In real execution, > we observe 16% allocation reduction, tracked by JMH GC profiler > `gc.alloc.rate.norm `. I think it's because HotSpot can't track all > allocations. > > Not only allocation drops, the runtime performance(ns/op) also increases from > 3.34% to 18.91%. > > Before: > > $$make test > TEST="micro:org.openjdk.bench.java.lang.StringBuilders.toStringWithMixedChars" > MICRO="OPTIONS=-prof gc -gc true -o before.log -jvm > $HOME/Devel/jdk_baseline/bin/java" > > Benchmark > (MIXED_SIZE) Mode Cnt Score Error Units > StringBuilders.toStringWithMixedChars >128 avgt 15 649.846 ± 76.291 ns/op > StringBuilders.toStringWithMixedChars:·gc.alloc.rate >128 avgt 15 872.855 ± 128.259 MB/sec > StringBuilders.toStringWithMixedChars:·gc.alloc.rate.norm >128 avgt 15 880.121 ± 0.050B/op > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space >128 avgt 15 707.730 ± 194.421 MB/sec > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space.norm >128 avgt 15 706.602 ± 94.504B/op > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space >128 avgt 15 0.001 ± 0.002 MB/sec > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space.norm >128 avgt 15 0.001 ± 0.001B/op > StringBuilders.toStringWithMixedChars:·gc.count >128 avgt 15 113.000counts > StringBuilders.toStringWithMixedChars:·gc.time >128 avgt 1585.000ms > StringBuilders.toStringWithMixedChars >256 avgt 15 1316.652 ± 112.771 ns/op > StringBuilders.toStringWithMixedChars:·gc.alloc.rate >256 avgt 15 800.864 ± 76.869 MB/sec > StringBuilders.toStringWithMixedChars:·gc.alloc.rate.norm >256 avgt 15 1648.288 ± 0.162B/op > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space >256 avgt 15 599.736 ± 174.001 MB/sec > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space.norm >256 avgt 15 1229.669 ± 318.518B/op > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space >256 avgt 15 0.001 ± 0.001 MB/sec > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space.norm >256 avgt 15 0.001 ± 0.002B/op > StringBuilders.toStringWithMixedChars:·gc.count >256 avgt 15 133.000counts > StringBuilders.toStringWithMixedChars:·gc.time >256 avgt 1592.000ms > StringBuilders.toStringWithMixedChars > 1024 avgt 15 5204.303 ± 418.115 ns/op > StringBuilders.toStringWithMixedChars:·gc.alloc.rate > 1024 avgt 15 768.730 ± 72.945 MB/sec > StringBuilders.toStringWithMixedChars:·gc.alloc.rate.norm > 1024 avgt 15 6256.844 ± 0.358B/op > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space > 1024 avgt 15 655.852 ± 121.602 MB/sec > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Eden_Space.norm > 1024 avgt 15 5315.265 ± 578.878B/op > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space > 1024 avgt 15 0.002 ± 0.002 MB/sec > StringBuilders.toStringWithMixedChars:·gc.churn.G1_Survivor_Space.norm > 1024 avgt 15 0.014 ± 0.011B/op > StringBuilders.toStringWithMixedChars:·gc.count > 1024 avgt 1596.000counts > StringBuilders.toStringWithMixedChars:·gc.time > 1024 avgt 1586.000ms > > >
Re: RFR: JDK-8266670: Better modeling of access flags in core reflection [v17]
On Sat, 5 Mar 2022 19:54:44 GMT, Joe Darcy wrote: >> This is an early review of changes to better model JVM access flags, that is >> "modifiers" like public, protected, etc. but explicitly at a VM level. >> >> Language level modifiers and JVM level access flags are closely related, but >> distinct. There are concepts that overlap in the two domains (public, >> private, etc.), others that only have a language-level modifier (sealed), >> and still others that only have an access flag (synthetic). >> >> The existing java.lang.reflect.Modifier class is inadequate to model these >> subtleties. For example, the bit positions used by access flags on different >> kinds of elements overlap (such as "volatile" for fields and "bridge" for >> methods. Just having a raw integer does not provide sufficient context to >> decode the corresponding language-level string. Methods like >> Modifier.methodModifiers() were introduced to cope with this situation. >> >> With additional modifiers and flags on the horizon with projects like >> Valhalla, addressing the existent modeling deficiency now ahead of time is >> reasonable before further strain is introduced. >> >> This PR in its current form is meant to give the overall shape of the API. >> It is missing implementations to map from, say, method modifiers to access >> flags, taking into account overlaps in bit positions. >> >> The CSR https://bugs.openjdk.java.net/browse/JDK-8281660 will be filled in >> once the API is further along. > > Joe Darcy 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 26 additional commits since > the last revision: > > - Respond to review feedback. > - Merge branch 'master' into JDK-8266670 > - Make workding changes suggested in review feedback. > - Merge branch 'master' into JDK-8266670 > - Typo fix; add implSpec to Executable. > - Appease jcheck. > - Fix some bugs found by inspection, docs cleanup. > - Merge branch 'master' into JDK-8266670 > - Initial support for accessFlags methods > - Add mask to access flag functionality. > - ... and 16 more: > https://git.openjdk.java.net/jdk/compare/b40644f8...14980605 Changes requested by plevart (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7445
Re: Wrong behavior of standard IO library when interacting with Samba (very serious)
On 7/03/2022 6:28 pm, Alan Bateman wrote: On 07/03/2022 05:33, Glavo wrote: I am a Java application developer. I noticed that when my program runs on Windows in a samba shared folder (mounted as a drive, or accessed via a UNC path), the Java standard IO library has some unusual behavior. Note that these issues only occur when accessing a folder shared by *Samba*, but not for the folder shared via SMB by another Windows host. One of the bugs was reported years ago (JDK-8154915): `Files.isWritable` always returns false for files shared by samba. It's worth noting for this question that `File::canWrite()`'s behavior is normal. (So in my program I pass `!Files.isWritable(p) && p.toFile().canWrite()` to detect if it's shared by samba and give the user a warning) This problem keeps showing up on several of my devices, so it should be fine to reproduce. The reason it wasn't resolved seems to be that the OpenJDK maintainers didn't understand that it came up when interacting with Samba (not just SMB). Testing if a file is writable, without side effects, can be complicated on Windows. File.canXXX only looks at the DOS attribute so can't give an accurate result. Files.isWritable examines the DACL, the legacy DOS attribute, and whether the volume is read-only, so there is more to go wrong. We've looked at many Windows <--> SAMBA interop issues over the years and it's always been that the Windows calls were failing or returning results that suggested the file had a DACL with 0 entries. That sounds similar to this old (but still open) SAMBA bug report: https://bugzilla.samba.org/show_bug.cgi?id=7973 David - I can't tell from your mail where the issue is but some of the behavior you report in your mail is very unusual. The JDK does not cache results so if you are saying that it requires restarting the JDK then it suggests an issue at a lower level. -Alan
Re: RFR: 8280404: Unexpected exception thrown when CEN file entry comment length is not valid [v2]
On Thu, 3 Mar 2022 17:46:53 GMT, Lance Andersen wrote: >> Hi all, >> >> This PR addresses an issue where an unexpected exception is thrown when the >> CEN file entry comment length is not correct. >> >> Mach5 tiers 1 - 3 run clean with this change. > > Lance Andersen has updated the pull request incrementally with one additional > commit since the last revision: > > Add comment indicating that zcp.toString could throw an Exception Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/7673
Re: Wrong behavior of standard IO library when interacting with Samba (very serious)
On 07/03/2022 05:33, Glavo wrote: I am a Java application developer. I noticed that when my program runs on Windows in a samba shared folder (mounted as a drive, or accessed via a UNC path), the Java standard IO library has some unusual behavior. Note that these issues only occur when accessing a folder shared by *Samba*, but not for the folder shared via SMB by another Windows host. One of the bugs was reported years ago (JDK-8154915): `Files.isWritable` always returns false for files shared by samba. It's worth noting for this question that `File::canWrite()`'s behavior is normal. (So in my program I pass `!Files.isWritable(p) && p.toFile().canWrite()` to detect if it's shared by samba and give the user a warning) This problem keeps showing up on several of my devices, so it should be fine to reproduce. The reason it wasn't resolved seems to be that the OpenJDK maintainers didn't understand that it came up when interacting with Samba (not just SMB). Testing if a file is writable, without side effects, can be complicated on Windows. File.canXXX only looks at the DOS attribute so can't give an accurate result. Files.isWritable examines the DACL, the legacy DOS attribute, and whether the volume is read-only, so there is more to go wrong. We've looked at many Windows <--> SAMBA interop issues over the years and it's always been that the Windows calls were failing or returning results that suggested the file had a DACL with 0 entries. I can't tell from your mail where the issue is but some of the behavior you report in your mail is very unusual. The JDK does not cache results so if you are saying that it requires restarting the JDK then it suggests an issue at a lower level. -Alan
Re: RFR: JDK-8282625 Formatter caches Locale/DecimalFormatSymbols poorly [v3]
On Fri, 4 Mar 2022 21:17:50 GMT, Jim Laskey wrote:
>> Several attempts have been made to improve Formatter's numeric performance
>> by caching the current Locale zero. Such fixes, however, ignore the real
>> issue, which is the slowness of fetching DecimalFormatSymbols. By directly
>> caching DecimalFormatSymbols in the Formatter, this enhancement streamlines
>> the process of accessing Locale DecimalFormatSymbols and specifically
>> getZeroDigit(). The result is a general improvement in the performance of
>> numeric formatting.
>>
>>
>> @Benchmark
>> public void bigDecimalDefaultLocale() {
>> result = String.format("%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f", X);
>> }
>>
>> @Benchmark
>> public void bigDecimalLocaleAlternate() {
>> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> other = String.format(DEFAULT, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> }
>>
>> @Benchmark
>> public void bigDecimalThaiLocale() {
>> result = String.format(THAI, "%1$f %1$f %1$f %1$f %1$f %1$f %1$f %1$f
>> %1$f %1$f", X);
>> }
>>
>> @Benchmark
>> public void integerDefaultLocale() {
>> result = String.format("%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d", x);
>> }
>>
>> @Benchmark
>> public void integerLocaleAlternate() {
>> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> other = String.format(DEFAULT, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> }
>>
>> @Benchmark
>> public void integerThaiLocale() {
>> result = String.format(THAI, "%1$d %1$d %1$d %1$d %1$d %1$d %1$d %1$d
>> %1$d %1$d", x);
>> }
>>
>>
>> Before:
>> Benchmark Mode Cnt Score Error Units
>> MyBenchmark.bigDecimalDefaultLocale thrpt 25 75498.923 ± 3686.966 ops/s
>> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 39068.721 ± 162.983 ops/s
>> MyBenchmark.bigDecimalThaiLocale thrpt 25 77256.530 ± 294.743 ops/s
>> MyBenchmark.integerDefaultLocale thrpt 25 344093.071 ± 6189.002 ops/s
>> MyBenchmark.integerLocaleAlternate thrpt 25 165685.488 ± 440.857 ops/s
>> MyBenchmark.integerThaiLocale thrpt 25 327461.302 ± 1168.243 ops/s
>>
>> After:
>> Benchmark Mode Cnt Score Error Units
>> MyBenchmark.bigDecimalDefaultLocale thrpt 25 94735.293 ± 674.587 ops/s
>> MyBenchmark.bigDecimalLocaleAlternate thrpt 25 44215.547 ± 291.664 ops/s
>> MyBenchmark.bigDecimalThaiLocale thrpt 25 91390.997 ± 658.677 ops/s
>> MyBenchmark.integerDefaultLocale thrpt 25 363722.977 ± 2864.554 ops/s
>> MyBenchmark.integerLocaleAlternate thrpt 25 165789.514 ± 779.656 ops/s
>> MyBenchmark.integerThaiLocale thrpt 25 351400.818 ± 1030.246 ops/s
>
> Jim Laskey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Too many 'the'
Just to note that there is also some caching in `DecimalStyle`. It might be
possible to reuse `DecimalStyle` here (if it was extended to support grouping
separators).
-
PR: https://git.openjdk.java.net/jdk/pull/7703
