Re: RFR: 8335366: Improve String.format performance with fastpath [v9]
On Sun, 30 Jun 2024 18:21:52 GMT, Shaojin Wen wrote:
>> We need a String format solution with good performance. String Template was
>> once expected, but it has been removed. j.u.Formatter is powerful, but its
>> performance is not good enough.
>>
>> This PR implements a subset of j.u.Formatter capabilities. The performance
>> is good enough that it is a fastpath for commonly used functions. When the
>> supported functions are exceeded, it will fall back to using j.u.Formatter.
>>
>> The performance of this implementation is good enough, the fastpath has low
>> detection cost, There is no noticeable performance degradation when falling
>> back to j.u.Formatter via fastpath.
>>
>> Below is a comparison of String.format and concat-based and StringBuilder:
>>
>> * benchmark java code
>>
>> public class StringFormat {
>> @Benchmark
>> public String stringIntFormat() {
>> return "%s %d".formatted(s, i);
>> }
>>
>> @Benchmark
>> public String stringIntConcat() {
>> return s + " " + i;
>> }
>>
>> @Benchmark
>> public String stringIntStringBuilder() {
>> return new StringBuilder(s).append(" ").append(i).toString();
>> }
>> }
>>
>>
>> * benchmark number on macbook m1 pro
>>
>> BenchmarkMode Cnt Score Error Units
>> StringFormat.stringIntConcat avgt 15 6.541 ? 0.056 ns/op
>> StringFormat.stringIntFormat avgt 15 17.399 ? 0.133 ns/op
>> StringFormat.stringIntStringBuilder avgt 15 8.004 ? 0.063 ns/op
>>
>>
>> From the above data, we can see that the implementation of fastpath reduces
>> the performance difference between String.format and StringBuilder from 10
>> times to 2~3 times.
>>
>> The implementation of fastpath supports the following four specifiers, which
>> can appear at most twice and support a width of 1 to 9.
>>
>> d
>> x
>> X
>> s
>>
>> If necessary, we can add a few more.
>>
>>
>> Below is a comparison of performance numbers running on a MacBook M1,
>> showing a significant performance improvement.
>>
>> -Benchmark Mode CntScoreError Units
>> (baseline)
>> -StringFormat.complexFormat avgt 15 895.954 ? 52.541 ns/op
>> -StringFormat.decimalFormat avgt 15 277.420 ? 18.254 ns/op
>> -StringFormat.stringFormat avgt 15 66.787 ? 2.715 ns/op
>> -StringFormat.stringIntFormat avgt 15 81.046 ? 1.879 ns/op
>> -StringFormat.widthStringFormat avgt 15 38.897 ? 0.114 ns/op
>> -StringFormat.widthStringIntFormat avgt 15 109.841 ? 1.028 ns/op
>>
>> +Benchmark...
>
> Shaojin Wen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> code style
As promising as the performance number is, I think we need to ensure two things:
1. Correctness: this patch adds a lot of special cases; not sure if the current
test cases already cover all of them. In addition, format is i18n stuff, which
will need extra review besides the review for performance gains.
2. Validity: the existing benchmarks don't have profile pollution: see
`ReflectionSpeedBenchmark`
https://github.com/openjdk/jdk/blob/d9bcf061450ebfb7fe02b5a50c855db1d9178e5d/test/micro/org/openjdk/bench/java/lang/reflect/ReflectionSpeedBenchmark.java#L291
where the `Method.invoke` and `Constructor.newInstance` are called to tamper
JIT's profiling, as JIT can conclude that only one format shape is ever used in
your benchmark, which is unlikely in production. You should call
`String.format` and `String.formatted` with varied format strings and arguments
in setup for profile pollution.
An extreme example would be at
https://github.com/openjdk/jdk/pull/14944#issuecomment-1644050455, where
`Arrays.hashCode(Object[])` where every element is an `Integer` is extremely
fast, but slows down drastically once different arrays are passed.
-
PR Comment: https://git.openjdk.org/jdk/pull/19956#issuecomment-2199327844
Re: RFR: 8333396: Performance regression of DecimalFormat.format [v15]
> ### Performance regression of DecimalFormat.format
> From the output of perf, we can see the hottest regions contain atomic
> instructions. But when run with JDK 11, there is no such problem. The reason
> is the removed biased locking.
> The DecimalFormat uses StringBuffer everywhere, and StringBuffer itself
> contains many synchronized methods.
> So I added support for some new methods that accept StringBuilder which is
> lock-free.
>
> ### Benchmark testcase
>
> @BenchmarkMode(Mode.AverageTime)
> @Warmup(iterations = 5, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @Measurement(iterations = 10, time = 500, timeUnit = TimeUnit.MILLISECONDS)
> @State(Scope.Thread)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class JmhDecimalFormat {
>
> private DecimalFormat format;
>
> @Setup(Level.Trial)
> public void setup() {
> format = new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testNewAndFormat() throws InterruptedException {
> new DecimalFormat("#0.0").format(9524234.1236457);
> }
>
> @Benchmark
> public void testNewOnly() throws InterruptedException {
> new DecimalFormat("#0.0");
> }
>
> @Benchmark
> public void testFormatOnly() throws InterruptedException {
> format.format(9524234.1236457);
> }
> }
>
>
> ### Test result
> Current JDK before optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnly avgt 50 642.099 ? 1.253 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 989.307 ? 3.676 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 303.381 ? 5.252 ns/op
>
>
>
> Current JDK after optimize
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 351.499 ? 0.761 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 615.145 ? 2.478 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 209.874 ? 9.951 ns/op
>
>
> ### JDK 11
>
> Benchmark Mode CntScore Error Units
> JmhDecimalFormat.testFormatOnlyavgt 50 364.214 ? 1.191 ns/op
> JmhDecimalFormat.testNewAndFormat avgt 50 658.699 ? 2.311 ns/op
> JmhDecimalFormat.testNewOnly avgt 50 248.300 ? 5.158 ns/op
lingjun-cg has updated the pull request incrementally with one additional
commit since the last revision:
896: Performance regression of DecimalFormat.format
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19513/files
- new: https://git.openjdk.org/jdk/pull/19513/files/b5bdc733..da09d85a
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=19513&range=14
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=19513&range=13-14
Stats: 9 lines in 3 files changed: 0 ins; 6 del; 3 mod
Patch: https://git.openjdk.org/jdk/pull/19513.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19513/head:pull/19513
PR: https://git.openjdk.org/jdk/pull/19513
Time-ordered UUID support, for improved database locality
Today, the JDK includes support for generating random UUIDs ("v4") and
name-based UUID's ("v3"). These two types have random leading bits, and
that results in poor performance in database systems that have sorted
indexes (i.e. most of them).
RFC 9562 [1], an update to the original UUID RFC, solves this by including
standard formats for time-ordered UUIDs, namely the v7 variant:
> UUID versions that are not time ordered, such as UUIDv4 (described in Section
5.4 ), have poor
database-index locality. This means that new values created in succession
are not close to each other in the index; thus, they require inserts to be
performed at random locations. The resulting negative performance effects
on the common structures used for this (B-tree and its variants) can be
dramatic.
> Time-ordered monotonic UUIDs benefit from greater database-index locality
because the new values are near each other in the index. As a result,
objects are more easily clustered together for better performance. The
real-world differences in this approach of index locality versus random
data inserts can be one order of magnitude or more.
Now that v7 UUIDs are well-defined, does anyone think it would be useful to
have in-jdk support for their generation? I am willing to do the work to
shepherd a proposal forward, given consensus and interest.
[1] https://www.rfc-editor.org/rfc/rfc9562.html
--
Confidentiality note: This e-mail may contain confidential information
from Nu Holdings Ltd and/or its affiliates. If you have received it by
mistake, please let us know by e-mail reply and delete it from your system;
you may not copy this message or disclose its contents to anyone; for
details about what personal information we collect and why, please refer to
our privacy policy
Re: java.util.zip.ZipError seems unused
The code you point to below is leveraging ZipFS, which provided as a demo in JDK 8 and was not a supported part of Java until JDK 9 at which point ZipFS did not use ZipError. So I understand your point but we also need to keep in mind that demo != supported as we consider the eventual removal of this class. On Jun 30, 2024, at 10:18 AM, Glavo wrote: I am the maintainer of HMCL and we need to catch ZipError in our program because our program needs to be compatible with Java 8: https://github.com/HMCL-dev/HMCL/blob/85b68ad135267bc33e03c3624b1bced9b7804c39/HMCLCore/src/main/java/org/jackhuang/hmcl/util/io/CompressingUtils.java#L218-L220 I'd love to see it deprecated, but I'm sure there are some use cases for it, so please evaluate the risks carefully. Glavo On Sun, Jun 30, 2024 at 5:42 PM Eirik Bjørsnøs mailto:eir...@gmail.com>> wrote: Hi! The java.util.zip.ZipError class seems unused in OpenJDK. I assume this is legacy from the native ZIP implementation in Java 8. This exception class extends InternalError and seems to have been added in Java 6 to help compatibility with existing code catching InternalError (JDK-4615343) This change also introduced the TestZipError test, which verified that ZipError was thrown while enumerating a ZIP file which was changed after being opened. The reimplementation of the ZIP implementation to Java (JDK-8145260) updated this test to expect a ZipException instead of the ZipError. Given that this class has now fallen out of use in OpenJDK, should we: 1: Deprecate it 2: Deprecate it for removal 3: Do nothing, keeping it around has a low cost 4: Something else It would also be useful if someone with access to a large code corpus could search for usages of this class so we could assess compatibility concerns of removing it. Thanks, Eirik. [oracle_sig_logo.gif] Lance Andersen | Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: RFR: 8335366: Improve String.format performance with fastpath [v9]
> We need a String format solution with good performance. String Template was
> once expected, but it has been removed. j.u.Formatter is powerful, but its
> performance is not good enough.
>
> This PR implements a subset of j.u.Formatter capabilities. The performance is
> good enough that it is a fastpath for commonly used functions. When the
> supported functions are exceeded, it will fall back to using j.u.Formatter.
>
> The performance of this implementation is good enough, the fastpath has low
> detection cost, There is no noticeable performance degradation when falling
> back to j.u.Formatter via fastpath.
>
> Below is a comparison of String.format and concat-based and StringBuilder:
>
> * benchmark java code
>
> public class StringFormat {
> @Benchmark
> public String stringIntFormat() {
> return "%s %d".formatted(s, i);
> }
>
> @Benchmark
> public String stringIntConcat() {
> return s + " " + i;
> }
>
> @Benchmark
> public String stringIntStringBuilder() {
> return new StringBuilder(s).append(" ").append(i).toString();
> }
> }
>
>
> * benchmark number on macbook m1 pro
>
> BenchmarkMode Cnt Score Error Units
> StringFormat.stringIntConcat avgt 15 6.541 ? 0.056 ns/op
> StringFormat.stringIntFormat avgt 15 17.399 ? 0.133 ns/op
> StringFormat.stringIntStringBuilder avgt 15 8.004 ? 0.063 ns/op
>
>
> From the above data, we can see that the implementation of fastpath reduces
> the performance difference between String.format and StringBuilder from 10
> times to 2~3 times.
>
> The implementation of fastpath supports the following four specifiers, which
> can appear at most twice and support a width of 1 to 9.
>
> d
> x
> X
> s
>
> If necessary, we can add a few more.
>
>
> Below is a comparison of performance numbers running on a MacBook M1, showing
> a significant performance improvement.
>
> -Benchmark Mode CntScoreError Units
> (baseline)
> -StringFormat.complexFormat avgt 15 895.954 ? 52.541 ns/op
> -StringFormat.decimalFormat avgt 15 277.420 ? 18.254 ns/op
> -StringFormat.stringFormat avgt 15 66.787 ? 2.715 ns/op
> -StringFormat.stringIntFormat avgt 15 81.046 ? 1.879 ns/op
> -StringFormat.widthStringFormat avgt 15 38.897 ? 0.114 ns/op
> -StringFormat.widthStringIntFormat avgt 15 109.841 ? 1.028 ns/op
>
> +Benchmark Mode CntScoreError Units
> (current f925339e93fdf7a281462554ce5d73139bd0f0cd)
> +StringFormat.complexF...
Shaojin Wen has updated the pull request incrementally with one additional
commit since the last revision:
code style
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19956/files
- new: https://git.openjdk.org/jdk/pull/19956/files/e7283f0a..9b2bb485
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=19956&range=08
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=19956&range=07-08
Stats: 21 lines in 1 file changed: 6 ins; 5 del; 10 mod
Patch: https://git.openjdk.org/jdk/pull/19956.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19956/head:pull/19956
PR: https://git.openjdk.org/jdk/pull/19956
Re: RFR: 8335366: Improve String.format performance with fastpath [v8]
> We need a String format solution with good performance. String Template was
> once expected, but it has been removed. j.u.Formatter is powerful, but its
> performance is not good enough.
>
> This PR implements a subset of j.u.Formatter capabilities. The performance is
> good enough that it is a fastpath for commonly used functions. When the
> supported functions are exceeded, it will fall back to using j.u.Formatter.
>
> The performance of this implementation is good enough, the fastpath has low
> detection cost, There is no noticeable performance degradation when falling
> back to j.u.Formatter via fastpath.
>
> Below is a comparison of String.format and concat-based and StringBuilder:
>
> * benchmark java code
>
> public class StringFormat {
> @Benchmark
> public String stringIntFormat() {
> return "%s %d".formatted(s, i);
> }
>
> @Benchmark
> public String stringIntConcat() {
> return s + " " + i;
> }
>
> @Benchmark
> public String stringIntStringBuilder() {
> return new StringBuilder(s).append(" ").append(i).toString();
> }
> }
>
>
> * benchmark number on macbook m1 pro
>
> BenchmarkMode Cnt Score Error Units
> StringFormat.stringIntConcat avgt 15 6.541 ? 0.056 ns/op
> StringFormat.stringIntFormat avgt 15 17.399 ? 0.133 ns/op
> StringFormat.stringIntStringBuilder avgt 15 8.004 ? 0.063 ns/op
>
>
> From the above data, we can see that the implementation of fastpath reduces
> the performance difference between String.format and StringBuilder from 10
> times to 2~3 times.
>
> The implementation of fastpath supports the following four specifiers, which
> can appear at most twice and support a width of 1 to 9.
>
> d
> x
> X
> s
>
> If necessary, we can add a few more.
>
>
> Below is a comparison of performance numbers running on a MacBook M1, showing
> a significant performance improvement.
>
> -Benchmark Mode CntScoreError Units
> (baseline)
> -StringFormat.complexFormat avgt 15 895.954 ? 52.541 ns/op
> -StringFormat.decimalFormat avgt 15 277.420 ? 18.254 ns/op
> -StringFormat.stringFormat avgt 15 66.787 ? 2.715 ns/op
> -StringFormat.stringIntFormat avgt 15 81.046 ? 1.879 ns/op
> -StringFormat.widthStringFormat avgt 15 38.897 ? 0.114 ns/op
> -StringFormat.widthStringIntFormat avgt 15 109.841 ? 1.028 ns/op
>
> +Benchmark Mode CntScoreError Units
> (current f925339e93fdf7a281462554ce5d73139bd0f0cd)
> +StringFormat.complexF...
Shaojin Wen has updated the pull request incrementally with one additional
commit since the last revision:
Update src/java.base/share/classes/java/lang/StringFormat.java
Co-authored-by: David Schlosnagle
-
Changes:
- all: https://git.openjdk.org/jdk/pull/19956/files
- new: https://git.openjdk.org/jdk/pull/19956/files/6163e864..e7283f0a
Webrevs:
- full: https://webrevs.openjdk.org/?repo=jdk&pr=19956&range=07
- incr: https://webrevs.openjdk.org/?repo=jdk&pr=19956&range=06-07
Stats: 2 lines in 1 file changed: 0 ins; 1 del; 1 mod
Patch: https://git.openjdk.org/jdk/pull/19956.diff
Fetch: git fetch https://git.openjdk.org/jdk.git pull/19956/head:pull/19956
PR: https://git.openjdk.org/jdk/pull/19956
Re: RFR: 8335366: Improve String.format performance with fastpath [v7]
On Sun, 30 Jun 2024 04:54:35 GMT, Shaojin Wen wrote:
>> We need a String format solution with good performance. String Template was
>> once expected, but it has been removed. j.u.Formatter is powerful, but its
>> performance is not good enough.
>>
>> This PR implements a subset of j.u.Formatter capabilities. The performance
>> is good enough that it is a fastpath for commonly used functions. When the
>> supported functions are exceeded, it will fall back to using j.u.Formatter.
>>
>> The performance of this implementation is good enough, the fastpath has low
>> detection cost, There is no noticeable performance degradation when falling
>> back to j.u.Formatter via fastpath.
>>
>> Below is a comparison of String.format and concat-based and StringBuilder:
>>
>> * benchmark java code
>>
>> public class StringFormat {
>> @Benchmark
>> public String stringIntFormat() {
>> return "%s %d".formatted(s, i);
>> }
>>
>> @Benchmark
>> public String stringIntConcat() {
>> return s + " " + i;
>> }
>>
>> @Benchmark
>> public String stringIntStringBuilder() {
>> return new StringBuilder(s).append(" ").append(i).toString();
>> }
>> }
>>
>>
>> * benchmark number on macbook m1 pro
>>
>> BenchmarkMode Cnt Score Error Units
>> StringFormat.stringIntConcat avgt 15 6.541 ? 0.056 ns/op
>> StringFormat.stringIntFormat avgt 15 17.399 ? 0.133 ns/op
>> StringFormat.stringIntStringBuilder avgt 15 8.004 ? 0.063 ns/op
>>
>>
>> From the above data, we can see that the implementation of fastpath reduces
>> the performance difference between String.format and StringBuilder from 10
>> times to 2~3 times.
>>
>> The implementation of fastpath supports the following four specifiers, which
>> can appear at most twice and support a width of 1 to 9.
>>
>> d
>> x
>> X
>> s
>>
>> If necessary, we can add a few more.
>>
>>
>> Below is a comparison of performance numbers running on a MacBook M1,
>> showing a significant performance improvement.
>>
>> -Benchmark Mode CntScoreError Units
>> (baseline)
>> -StringFormat.complexFormat avgt 15 895.954 ? 52.541 ns/op
>> -StringFormat.decimalFormat avgt 15 277.420 ? 18.254 ns/op
>> -StringFormat.stringFormat avgt 15 66.787 ? 2.715 ns/op
>> -StringFormat.stringIntFormat avgt 15 81.046 ? 1.879 ns/op
>> -StringFormat.widthStringFormat avgt 15 38.897 ? 0.114 ns/op
>> -StringFormat.widthStringIntFormat avgt 15 109.841 ? 1.028 ns/op
>>
>> +Benchmark...
>
> Shaojin Wen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> laze init for `decimal fast path locale`
src/java.base/share/classes/java/lang/StringFormat.java line 142:
> 140: str = String.valueOf(arg1);
> 141: coder |= str.coder();
> 142: arg1 = String.valueOf(str);
nit: `arg1 = String.valueOf(str);` can be avoided by just using `str`:
Suggestion:
arg1 = str = String.valueOf(arg1);
coder |= str.coder();
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19956#discussion_r1660197061
Re: RFR: 8335366: Improve String.format performance with fastpath [v7]
On Sun, 30 Jun 2024 14:57:26 GMT, David Schlosnagle wrote: >> Shaojin Wen has updated the pull request incrementally with one additional >> commit since the last revision: >> >> laze init for `decimal fast path locale` > > src/java.base/share/classes/java/lang/StringFormat.java line 142: > >> 140: str = String.valueOf(arg1); >> 141: coder |= str.coder(); >> 142: arg1 = String.valueOf(str); > > nit: `arg1 = String.valueOf(str);` can be avoided by just using `str`: > > Suggestion: > > arg1 = str = String.valueOf(arg1); > coder |= str.coder(); alternatively: Suggestion: str = String.valueOf(arg1); coder |= str.coder(); arg1 = str; - PR Review Comment: https://git.openjdk.org/jdk/pull/19956#discussion_r1660197249
Re: java.util.zip.ZipError seems unused
I am the maintainer of HMCL and we need to catch ZipError in our program because our program needs to be compatible with Java 8: https://github.com/HMCL-dev/HMCL/blob/85b68ad135267bc33e03c3624b1bced9b7804c39/HMCLCore/src/main/java/org/jackhuang/hmcl/util/io/CompressingUtils.java#L218-L220 I'd love to see it deprecated, but I'm sure there are some use cases for it, so please evaluate the risks carefully. Glavo On Sun, Jun 30, 2024 at 5:42 PM Eirik Bjørsnøs wrote: > Hi! > > The java.util.zip.ZipError class seems unused in OpenJDK. I assume this is > legacy from the native ZIP implementation in Java 8. > > This exception class extends InternalError and seems to have been added in > Java 6 to help compatibility with existing code catching InternalError > (JDK-4615343) > > This change also introduced the TestZipError test, which verified that > ZipError was thrown while enumerating a ZIP file which was changed after > being opened. The reimplementation of the ZIP implementation to Java > (JDK-8145260) updated this test to expect a ZipException instead of the > ZipError. > > Given that this class has now fallen out of use in OpenJDK, should we: > > 1: Deprecate it > 2: Deprecate it for removal > 3: Do nothing, keeping it around has a low cost > 4: Something else > > It would also be useful if someone with access to a large code corpus > could search for usages of this class so we could assess compatibility > concerns of removing it. > > Thanks, > Eirik. >
Re: java.util.zip.ZipError seems unused
Hi Eirik, The removal of ZipError from ZipFile/ZipFileSystem/ZipFileSystemProvider occurred via 8145260 and 8037394 for JDK 9 The test should also be re-written at this point Jai or I can make a pass to see if there are any external usages via a corpus search but I tend to doubt it On Jun 30, 2024, at 3:20 AM, Eirik Bjørsnøs wrote: Hi! The java.util.zip.ZipError class seems unused in OpenJDK. I assume this is legacy from the native ZIP implementation in Java 8. This exception class extends InternalError and seems to have been added in Java 6 to help compatibility with existing code catching InternalError (JDK-4615343) This change also introduced the TestZipError test, which verified that ZipError was thrown while enumerating a ZIP file which was changed after being opened. The reimplementation of the ZIP implementation to Java (JDK-8145260) updated this test to expect a ZipException instead of the ZipError. Given that this class has now fallen out of use in OpenJDK, should we: 1: Deprecate it 2: Deprecate it for removal 3: Do nothing, keeping it around has a low cost 4: Something else It would also be useful if someone with access to a large code corpus could search for usages of this class so we could assess compatibility concerns of removing it. Thanks, Eirik. [oracle_sig_logo.gif] Lance Andersen | Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: RFR: 8327854: Test java/util/stream/test/org/openjdk/tests/java/util/stream/WhileOpStatefulTest.java failed with RuntimeException
On Sat, 1 Jun 2024 11:49:39 GMT, Viktor Klang wrote: > This PR improves the test failure output for the failing test case, and the > underlying issue is likely going to be addressed by > https://github.com/openjdk/jdk/pull/19131 /cc @DougLea @AlanBateman @stuart-marks @PaulSandoz This is a PR which improves debug-ability if the test fails, please review at your discretion if possible. - PR Comment: https://git.openjdk.org/jdk/pull/19508#issuecomment-2198508200
Re: RFR: 8335366: Improve String.format performance with fastpath [v7]
On Sun, 30 Jun 2024 04:54:35 GMT, Shaojin Wen wrote:
>> We need a String format solution with good performance. String Template was
>> once expected, but it has been removed. j.u.Formatter is powerful, but its
>> performance is not good enough.
>>
>> This PR implements a subset of j.u.Formatter capabilities. The performance
>> is good enough that it is a fastpath for commonly used functions. When the
>> supported functions are exceeded, it will fall back to using j.u.Formatter.
>>
>> The performance of this implementation is good enough, the fastpath has low
>> detection cost, There is no noticeable performance degradation when falling
>> back to j.u.Formatter via fastpath.
>>
>> Below is a comparison of String.format and concat-based and StringBuilder:
>>
>> * benchmark java code
>>
>> public class StringFormat {
>> @Benchmark
>> public String stringIntFormat() {
>> return "%s %d".formatted(s, i);
>> }
>>
>> @Benchmark
>> public String stringIntConcat() {
>> return s + " " + i;
>> }
>>
>> @Benchmark
>> public String stringIntStringBuilder() {
>> return new StringBuilder(s).append(" ").append(i).toString();
>> }
>> }
>>
>>
>> * benchmark number on macbook m1 pro
>>
>> BenchmarkMode Cnt Score Error Units
>> StringFormat.stringIntConcat avgt 15 6.541 ? 0.056 ns/op
>> StringFormat.stringIntFormat avgt 15 17.399 ? 0.133 ns/op
>> StringFormat.stringIntStringBuilder avgt 15 8.004 ? 0.063 ns/op
>>
>>
>> From the above data, we can see that the implementation of fastpath reduces
>> the performance difference between String.format and StringBuilder from 10
>> times to 2~3 times.
>>
>> The implementation of fastpath supports the following four specifiers, which
>> can appear at most twice and support a width of 1 to 9.
>>
>> d
>> x
>> X
>> s
>>
>> If necessary, we can add a few more.
>>
>>
>> Below is a comparison of performance numbers running on a MacBook M1,
>> showing a significant performance improvement.
>>
>> -Benchmark Mode CntScoreError Units
>> (baseline)
>> -StringFormat.complexFormat avgt 15 895.954 ? 52.541 ns/op
>> -StringFormat.decimalFormat avgt 15 277.420 ? 18.254 ns/op
>> -StringFormat.stringFormat avgt 15 66.787 ? 2.715 ns/op
>> -StringFormat.stringIntFormat avgt 15 81.046 ? 1.879 ns/op
>> -StringFormat.widthStringFormat avgt 15 38.897 ? 0.114 ns/op
>> -StringFormat.widthStringIntFormat avgt 15 109.841 ? 1.028 ns/op
>>
>> +Benchmark...
>
> Shaojin Wen has updated the pull request incrementally with one additional
> commit since the last revision:
>
> laze init for `decimal fast path locale`
In most cases when calling String.format, the format parameter is a constant.
Is there any chance that C2 can optimize this?
void my_func() {
String str;
int d;
// ..
String info = "info %s %d".formatted(str, d);
}
final class StringFormat {
final class StringFormat {
@ForceInline
static String format(String format, Object... args) {
if (args != null) { // 1
int off = format.indexOf('%'); // 2
if (off == -1) { // 3
return format;
}
int len = format.length(); // 4
if (off + 1 != len) { // 5
int off1 = format.indexOf('%', off + 2); // 6
String s = null;
if (args.length == 1) { // 7
if (off1 == -1) { // 8
s = format1(format, off, args[0]);
}
} else if (args.length == 2) { // 9
if (off1 != -1 && off1 + 1 != len) { // 10
s = format2(format, off, off1, args[0], args[1]);
}
}
if (s != null) {
return s;
}
}
}
return new Formatter().format(format, args).toString();
}
private static String format1(String format, int off, Object arg) {
int len = format.length(); // 11
char conv = format.charAt(off + 1); //12
int width = 0;
if (conv >= '1' && conv <= '9') { // 13
width = conv - '0'; // 14
if (off + 2 < len) { // 15
conv = format.charAt(off + 2); // 16
}
}
if (conv == STRING) { // 17
if (isLong(arg)) {
conv = DECIMAL_INTEGER;
} else {
arg = String.valueOf(arg);
}
}
int size = stringSize(conv, arg);
if (size == -1) {
return null;
}
java.util.zip.ZipError seems unused
Hi! The java.util.zip.ZipError class seems unused in OpenJDK. I assume this is legacy from the native ZIP implementation in Java 8. This exception class extends InternalError and seems to have been added in Java 6 to help compatibility with existing code catching InternalError (JDK-4615343) This change also introduced the TestZipError test, which verified that ZipError was thrown while enumerating a ZIP file which was changed after being opened. The reimplementation of the ZIP implementation to Java (JDK-8145260) updated this test to expect a ZipException instead of the ZipError. Given that this class has now fallen out of use in OpenJDK, should we: 1: Deprecate it 2: Deprecate it for removal 3: Do nothing, keeping it around has a low cost 4: Something else It would also be useful if someone with access to a large code corpus could search for usages of this class so we could assess compatibility concerns of removing it. Thanks, Eirik.
