Re: RFR: 8301226: Add clamp() methods to java.lang.Math and to StrictMath [v8]
On Tue, 7 Feb 2023 12:53:25 GMT, Tagir F. Valeev wrote: >> clamp() methods added to Math and StrictMath >> >> `int clamp(long, int, int)` is somewhat different, as it accepts a `long` >> value and safely clamps it to an `int` range. Other overloads work with a >> particular type (long, float and double). Using similar approach in other >> cases (e.g. `float clamp(double, float, float)`) may cause accidental >> precision loss even if the value is within range, so I decided to avoid this. >> >> In all cases, `max >= min` precondition should met. For double and float we >> additionally order `-0.0 < 0.0`, similarly to what Math.max or >> Double.compare do. In double and float overloads I try to keep at most one >> arg-check comparison on common path, so the order of checks might look >> unusual. >> >> For tests, I noticed that tests in java/lang/Math don't use any testing >> framework (even newer tests), so I somehow mimic the approach of neighbour >> tests. > > Tagir F. Valeev has updated the pull request incrementally with one > additional commit since the last revision: > > Comments in tests Is it ever possible to optimize clamp() impl ? I suppose 2 if conditions can be simplified: return (int) Math.min(max, Math.max(value, min)); Maybe hotspot c2 can deduce it or using intrinsics: If value<=min: return min If value>=max: return max return value (NaN compatible=>NaN) - PR: https://git.openjdk.org/jdk/pull/12428
Re: RFR: 8301627: System.exit and Runtime.exit debug logging [v2]
On Mon, 13 Feb 2023 10:57:35 GMT, Daniel Fuchs wrote:
>> src/java.base/share/classes/java/lang/Shutdown.java line 162:
>>
>>> 160: * If the system logger {@code java.lang.Runtime} is enabled for
>>> logging level DEBUG/FINE
>>> 161: * the stack trace of the call to {@code Runtime.exit()} or {@code
>>> System.exit()}
>>> 162: * is logged.
>>
>> Shutdown is not a public class so this impNote won't appear in the APIs
>> docs. Should it move to Runtime.exit and System.exit? If it moved to a
>> public API then "system logger" could link to System.Logger. Also would it
>> be more correct to say "a system logger named "java.lang.Runtime" is enabled
>> for logging level DEBUG"?
>
> FINE is not a level supported by the System.Logger, it is the level to which
> DEBUG is mapped when the backend is java.util.logging. I suggest to remove
> FINE from this description and add an `{@link Loger.Level#DEBUG DEBUG}`
> around DEBUG.
Roger has updated this but it's still a comment on a non-public class. I think
the main question here is whether there should be a note in the System.exit and
Runtime.exit to document that these methods log? If not, will it be documented
anywhere, maybe a troubleshooting guide to help track down what/who is calling
System.exit?
-
PR: https://git.openjdk.org/jdk/pull/12517
Re: RFR: JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) [v18]
On Wed, 9 Nov 2022 21:06:50 GMT, iaroslavski wrote: >> Sorting: >> >> - adopt radix sort for sequential and parallel sorts on >> int/long/float/double arrays (almost random and length > 6K) >> - fix tryMergeRuns() to better handle case when the last run is a single >> element >> - minor javadoc and comment changes >> >> Testing: >> - add new data inputs in tests for sorting >> - add min/max/infinity values to float/double testing >> - add tests for radix sort > > iaroslavski has updated the pull request incrementally with one additional > commit since the last revision: > > JDK-8266431: Dual-Pivot Quicksort improvements (Radix sort) > > * Fixed microbenchmarking tests I am currently preparing my PR against latest jdk to start an official review... - PR: https://git.openjdk.org/jdk/pull/3938
Integrated: 8302214: Typo in javadoc of Arrays.compare and Arrays.mismatch
On Thu, 9 Feb 2023 10:13:03 GMT, Eirik Bjorsnos wrote:
> The Javadocs of Arrays.compare and Arrays.mismatch uses the incorrect
> capitalization `atoIndex` and `btoIndex` when referencing the corresponding
> `aToIndex` and `bToIndex` parameters.
>
> Sample:
>
>
> * specified ranges [{@code aFromIndex}, {@code atoIndex}) and
> * [{@code bFromIndex}, {@code btoIndex}) respectively:
>
>
> This PR changes the capitalization to `aToIndex` and `bToIndex`. This change
> was performed using case-sensitive search / replace in an IDE.
This pull request has now been integrated.
Changeset: d782125c
Author:Eirik Bjorsnos
Committer: Jaikiran Pai
URL:
https://git.openjdk.org/jdk/commit/d782125c8f3bfe087269e4430dd12328d8cc77f8
Stats: 58 lines in 1 file changed: 0 ins; 0 del; 58 mod
8302214: Typo in javadoc of Arrays.compare and Arrays.mismatch
Reviewed-by: jpai
-
PR: https://git.openjdk.org/jdk/pull/12488
Re: RFR: 8302204: Optimize BigDecimal.divide
On Fri, 10 Feb 2023 10:00:05 GMT, Xiaowei Lu wrote:
> [JDK-8269667](https://bugs.openjdk.org/browse/JDK-8269667) has uncovered the
> poor performance of BigDecimal.divide under certain circumstance.
>
> We confront similar situations when benchmarking Spark3 on TPC-DS test kit.
> According to the flame-graph below, it is StripZeros that spends most of the
> time of BigDecimal.divide. Hence we propose this patch to optimize stripping
> zeros.
> 
>
> Currently, createAndStripZerosToMatchScale() is performed linearly. That is,
> the target value is parsed from back to front, each time stripping out single
> ‘0’. To optimize, we can adopt the method of binary search. That is, each
> time we try to strip out ${scale/2} ‘0’s.
>
> The performance looks good. Therotically, time complexity of our method is
> O(log n), while the current one is O(n). In practice, benchmarks on Spark3
> show that 1/3 less time (102s->68s) is spent on TPC-DS query4. We also runs
> Jtreg and JCK to check correctness, and it seems fine.
>
> More about environment:
> we run Spark3.3.0 on Openjdk11, but it seems jdk version doesn’t have much
> impact on BigDecimal. Spark cluster consists of a main node and 2 core nodes,
> each has 4cores, 16g memory and 4x500GB storage.
> _Mailing list message from [Louis Wasserman](mailto:lowas...@google.com) on
> [core-libs-dev](mailto:core-libs-...@mail.openjdk.org):_
>
> Could you do that benchmark with e.g. JMH rather than taking the difference
> of System.currentTimeMillis? That would probably make it easier to read and
> trust the results.
>
> On Sun, Feb 12, 2023, 7:56 PM Sergey Kuksenko
> wrote:
>
> -- next part -- An HTML attachment was scrubbed...
> URL:
>
Sure. I have written a microbenchmark using JMH and it shows
before optimization
DecimalBenchmark.divide_by_2 thrpt5 334549.891 ± 3028.946 ops/s
DecimalBenchmark.divide_by_2_to_100 thrpt515874.537 ±38.942 ops/s
DecimalBenchmark.divide_by_3 thrpt5 6190583.950 ± 91116.750 ops/s
after optimization
DecimalBenchmark.divide_by_2 thrpt5 1479106.480 ± 5950.440 ops/s
DecimalBenchmark.divide_by_2_to_100 thrpt532730.634 ±81.891 ops/s
DecimalBenchmark.divide_by_3 thrpt5 6233700.252 ± 21043.571 ops/s
The following is the benchmark code
@State(Scope.Benchmark)
@BenchmarkMode(Mode.Throughput)
@Warmup(iterations = 5, time = 10)
@Measurement(iterations = 5, time = 10)
@Fork(value = 1)
public class DecimalBenchmark {
private static final MathContext mc = new MathContext(38,
RoundingMode.HALF_UP);
@Benchmark
public void divide_by_2_to_100() {
for(long i = 2; i <= 100; i++) {
BigDecimal divisor = BigDecimal.valueOf(i);
BigDecimal.ONE.divide(divisor, mc);
}
}
@Benchmark
public void divide_by_2() {
BigDecimal.ONE.divide(BigDecimal.valueOf(2L), mc);
}
@Benchmark
public void divide_by_3() {
BigDecimal.ONE.divide(BigDecimal.valueOf(3L), mc);
}
}
-
PR: https://git.openjdk.org/jdk/pull/12509
Integrated: 8301226: Add clamp() methods to java.lang.Math and to StrictMath
On Sat, 4 Feb 2023 13:24:11 GMT, Tagir F. Valeev wrote: > clamp() methods added to Math and StrictMath > > `int clamp(long, int, int)` is somewhat different, as it accepts a `long` > value and safely clamps it to an `int` range. Other overloads work with a > particular type (long, float and double). Using similar approach in other > cases (e.g. `float clamp(double, float, float)`) may cause accidental > precision loss even if the value is within range, so I decided to avoid this. > > In all cases, `max >= min` precondition should met. For double and float we > additionally order `-0.0 < 0.0`, similarly to what Math.max or Double.compare > do. In double and float overloads I try to keep at most one arg-check > comparison on common path, so the order of checks might look unusual. > > For tests, I noticed that tests in java/lang/Math don't use any testing > framework (even newer tests), so I somehow mimic the approach of neighbour > tests. This pull request has now been integrated. Changeset: 94e7cc85 Author:Tagir F. Valeev URL: https://git.openjdk.org/jdk/commit/94e7cc8587356988e713d23d1653bdd5c43fb3f1 Stats: 500 lines in 3 files changed: 499 ins; 0 del; 1 mod 8301226: Add clamp() methods to java.lang.Math and to StrictMath Reviewed-by: qamai, darcy - PR: https://git.openjdk.org/jdk/pull/12428
Re: RFR: 8302204: Optimize BigDecimal.divide
On Tue, 14 Feb 2023 03:20:14 GMT, Sergey Kuksenko wrote: > The pr looks promising in terms of performance. What makes sense to do: > > *) Don't rely on external benchmarks. It's fine if such exists, but anyway > set of microbenchmarks (using JMH) will be much better. More clear, readable > results, etc. E.g., it may show that other operations (for example, sqrt) > were speeded up too. > > *) Find boundaries. "divideAndRemainder(bigTenToThe(scaleStep))" may produce > non-zero reminder. Find conditions when it happens. How big is performance > regression in such cases? > > Some other optimizations: *) Current code checks only the lowest bit (odd or > even) to cut off indivisible cases. Making > "divideAndRemainder(bigTenToThe(scaleStep))" - you make check scaleStep > lowest bits to do cut off. See "BigInteger.getLowestSetBit()" > > *) BigInteger division by int value is faster. It's a special case. What is > faster, doing the single division by bigTenToThe(scaleStep) or doing several > divisions (split scaleStep to fit into int)? Exploration is required. Good idea! Thanks. I will look into such cases and try to explore more. - PR: https://git.openjdk.org/jdk/pull/12509
Re: RFR: 8302204: Optimize BigDecimal.divide
On Fri, 10 Feb 2023 10:00:05 GMT, Xiaowei Lu wrote:
> [JDK-8269667](https://bugs.openjdk.org/browse/JDK-8269667) has uncovered the
> poor performance of BigDecimal.divide under certain circumstance.
>
> We confront similar situations when benchmarking Spark3 on TPC-DS test kit.
> According to the flame-graph below, it is StripZeros that spends most of the
> time of BigDecimal.divide. Hence we propose this patch to optimize stripping
> zeros.
> 
>
> Currently, createAndStripZerosToMatchScale() is performed linearly. That is,
> the target value is parsed from back to front, each time stripping out single
> ‘0’. To optimize, we can adopt the method of binary search. That is, each
> time we try to strip out ${scale/2} ‘0’s.
>
> The performance looks good. Therotically, time complexity of our method is
> O(log n), while the current one is O(n). In practice, benchmarks on Spark3
> show that 1/3 less time (102s->68s) is spent on TPC-DS query4. We also runs
> Jtreg and JCK to check correctness, and it seems fine.
>
> More about environment:
> we run Spark3.3.0 on Openjdk11, but it seems jdk version doesn’t have much
> impact on BigDecimal. Spark cluster consists of a main node and 2 core nodes,
> each has 4cores, 16g memory and 4x500GB storage.
The pr looks promising in terms of performance.
What makes sense to do:
*) Don't rely on external benchmarks. It's fine if such exists, but anyway set
of microbenchmarks (using JMH) will be much better. More clear, readable
results, etc. E.g., it may show that other operations (for example, sqrt) were
speeded up too.
*) Find boundaries.
"divideAndRemainder(bigTenToThe(scaleStep))" may produce non-zero reminder.
Find conditions when it happens. How big is performance regression in such
cases?
Some other optimizations:
*)
Current code checks only the lowest bit (odd or even) to cut off indivisible
cases.
Making "divideAndRemainder(bigTenToThe(scaleStep))" - you make check scaleStep
lowest bits to do cut off. See "BigInteger.getLowestSetBit()"
*)
BigInteger division by int value is faster. It's a special case. What is
faster, doing the single division by bigTenToThe(scaleStep) or doing several
divisions (split scaleStep to fit into int)? Exploration is required.
-
PR: https://git.openjdk.org/jdk/pull/12509
Re: RFR: 8299748: java/util/zip/Deinflate.java failing on s390x
On Tue, 7 Feb 2023 07:07:54 GMT, Alan Bateman wrote:
>>> Hi @AlanBateman ,
>>> with latest changes (doing inflate/deinflate) test passes over s390 and x86
>>> as well. Please take a look now.
>>
>> Good. One thing to try is to just deflate/inflate into out1/out2, no need
>> for the intermediate BAOS, try this:
>>
>>
>> --- a/test/jdk/java/util/zip/DeInflate.java
>> +++ b/test/jdk/java/util/zip/DeInflate.java
>> @@ -1,5 +1,5 @@
>> /*
>> - * Copyright (c) 2011, 2018, Oracle and/or its affiliates. All rights
>> reserved.
>> + * Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights
>> reserved.
>> * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
>> *
>> * This code is free software; you can redistribute it and/or modify it
>> @@ -127,11 +127,25 @@ public class DeInflate {
>>
>> def.setInput(in, 0, len);
>> def.finish();
>> -int m = def.deflate(out1);
>> +int m = 0;
>> +while (!def.finished()) {
>> +int remaining = out1.length - m;
>> +if (remaining == 0) {
>> +throw new RuntimeException("out1 is small");
>> +}
>> +m += def.deflate(out1, m, remaining);
>> +}
>>
>> Inflater inf = new Inflater(nowrap);
>> inf.setInput(out1, 0, m);
>> -int n = inf.inflate(out2);
>> +int n = 0;
>> +while (!inf.finished()) {
>> +int remaining = out2.length - n;
>> +if (remaining == 0) {
>> +throw new RuntimeException("out2 is small");
>> +}
>> +n += inf.inflate(out2, n, remaining);
>> +}
>>
>> if (n != len ||
>> !Arrays.equals(Arrays.copyOf(in, len), Arrays.copyOf(out2,
>> len)) ||
>
>> @AlanBateman should we proceed with the current changes then ?
>
> The BAOS in your current proposal shouldn't be needed so I suspect there is
> something else "interesting" with this zlib implementation. For the patch
> above, can you remove the checking for remaining == 0 and print out the value
> from deflate at each iterate of the loop. I'm wondering if it returns 0
> before def.finished() returns true.
Hi @AlanBateman, what should be our next step for this PR :)
-
PR: https://git.openjdk.org/jdk/pull/12283
Re: RFR: JDK-8300808: Accelerate Base64 on x86 for AVX2 [v15]
On Thu, 9 Feb 2023 18:08:15 GMT, Scott Gibbons wrote:
>> Added code for Base64 acceleration (encode and decode) which will accelerate
>> ~4x for AVX2 platforms.
>>
>> Encode performance:
>> **Old:**
>>
>> Benchmark (maxNumBytes) Mode Cnt Score Error
>> Units
>> Base64Encode.testBase64Encode 1024 thrpt3 4309.439 ± 2.632
>> ops/ms
>>
>>
>> **New:**
>>
>> Benchmark (maxNumBytes) Mode Cnt Score
>> Error Units
>> Base64Encode.testBase64Encode 1024 thrpt3 24211.397 ±
>> 102.026 ops/ms
>>
>>
>> Decode performance:
>> **Old:**
>>
>> Benchmark (errorIndex) (lineSize) (maxNumBytes)
>> Mode Cnt ScoreError Units
>> Base64Decode.testBase64Decode 144 4 1024
>> thrpt3 3961.768 ± 93.409 ops/ms
>>
>> **New:**
>> Benchmark (errorIndex) (lineSize) (maxNumBytes)
>> Mode Cnt ScoreError Units
>> Base64Decode.testBase64Decode 144 4 1024
>> thrpt3 14738.051 ± 24.383 ops/ms
>
> Scott Gibbons has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Add URL to microbenchmark
src/hotspot/cpu/x86/stubGenerator_x86_64.cpp line 2399:
> 2397: VM_Version::supports_avx512bw()) {
> 2398: __ cmpl(length, 31); // 32-bytes is break-even for AVX-512
> 2399: __ jcc(Assembler::lessEqual, L_bruteForce);
The avx2 code needs the length to be atleast 0x2c (44) bytes. We could directly
go to non-avx code instead of L_bruteForce here. We will save one
subtract/branch.
src/hotspot/cpu/x86/stubGenerator_x86_64.cpp line 2658:
> 2656: // Check for buffer too small (for algorithm)
> 2657: __ subl(length, 0x2c);
> 2658: __ jcc(Assembler::lessEqual, L_tailProc);
This could be Assembler::less instead of Assembler::lessEqual.
src/hotspot/cpu/x86/stubGenerator_x86_64.cpp line 2699:
> 2697: __ addptr(dest, 0x18);
> 2698: __ subl(length, 0x20);
> 2699: __ jcc(Assembler::lessEqual, L_tailProc);
This could be Assembler::less instead of Assembler::lessEqual.
-
PR: https://git.openjdk.org/jdk/pull/12126
Re: RFR: 8301627: System.exit and Runtime.exit debug logging [v2]
> It can be difficult to find the cause of calls to > `java.lang.System.exit(status)` and `Runtime.exit(status)` because the Java > runtime exits. > The status value and stack trace are logged using the System Logger named > `java.lang.Runtime` with message level `System.Logger.Level.DEBUG`. Roger Riggs has updated the pull request incrementally with one additional commit since the last revision: Added try/catch around lookup of logger so exceptions do not prevent System.exit. Added test case with console logger (when java.util.logging) not present. Removed @implNote tag its not appropriate in implementation javadoc. Still looking into when and where the log configuration should be described. - Changes: - all: https://git.openjdk.org/jdk/pull/12517/files - new: https://git.openjdk.org/jdk/pull/12517/files/7f149916..76c4d61f Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=12517=01 - incr: https://webrevs.openjdk.org/?repo=jdk=12517=00-01 Stats: 26 lines in 2 files changed: 14 ins; 3 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/12517.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12517/head:pull/12517 PR: https://git.openjdk.org/jdk/pull/12517
Re: RFR: 8301226: Add clamp() methods to java.lang.Math and to StrictMath [v8]
On Tue, 7 Feb 2023 12:53:25 GMT, Tagir F. Valeev wrote: >> clamp() methods added to Math and StrictMath >> >> `int clamp(long, int, int)` is somewhat different, as it accepts a `long` >> value and safely clamps it to an `int` range. Other overloads work with a >> particular type (long, float and double). Using similar approach in other >> cases (e.g. `float clamp(double, float, float)`) may cause accidental >> precision loss even if the value is within range, so I decided to avoid this. >> >> In all cases, `max >= min` precondition should met. For double and float we >> additionally order `-0.0 < 0.0`, similarly to what Math.max or >> Double.compare do. In double and float overloads I try to keep at most one >> arg-check comparison on common path, so the order of checks might look >> unusual. >> >> For tests, I noticed that tests in java/lang/Math don't use any testing >> framework (even newer tests), so I somehow mimic the approach of neighbour >> tests. > > Tagir F. Valeev has updated the pull request incrementally with one > additional commit since the last revision: > > Comments in tests Marked as reviewed by darcy (Reviewer). - PR: https://git.openjdk.org/jdk/pull/12428
Re: RFR: JDK-8302026: Port fdlibm inverse trig functions (asin, acos, atan) to Java
On Mon, 13 Feb 2023 22:03:14 GMT, Joe Darcy wrote: > Proceeding down the line of FDLIBM functions to be ported, next up are asin, > acos, and atan. > > Diffs of the various versions will follow in a separate message. > > There were no unusual coding idioms encountered in porting these methods. > Proceeding down the line of FDLIBM functions to be ported, next up are asin, > acos, and atan. > > Diffs of the various versions will follow in a separate message. > > There were no unusual coding idioms encountered in porting these methods. > ### Progress > > * [ ] Change must be properly reviewed (1 review required, with at least > 1 [Reviewer](https://openjdk.org/bylaws#reviewer)) > > * [x] Change must not contain extraneous whitespace > > * [x] Commit message must refer to an issue > > > ### Issue > > * [JDK-8302026](https://bugs.openjdk.org/browse/JDK-8302026): Port fdlibm > inverse trig functions (asin, acos, atan) to Java > > > ### Reviewing > Using `git` > Using Skara CLI tools > Using diff file PS In one-off local testing, testing all float arguments for acos/asin/atan passed when run against both JDK 20 and JDK 21, increasing confidence that both the transliteration port is equivalent to the original C code and that the idiomatic port is equivalent to the transliteration one. - PR: https://git.openjdk.org/jdk/pull/12545
Re: RFR: JDK-8302026: Port fdlibm inverse trig functions (asin, acos, atan) to Java
On Mon, 13 Feb 2023 22:03:14 GMT, Joe Darcy wrote:
> Proceeding down the line of FDLIBM functions to be ported, next up are asin,
> acos, and atan.
>
> Diffs of the various versions will follow in a separate message.
>
> There were no unusual coding idioms encountered in porting these methods.
Original C vs transliteration port:
$ diff -w InverseTrig.c InverseTrig.translit.java
1c1,2
< /* __ieee754_asin(x)
---
> /** Returns the arcsine of x.
> *
30,38c31,32
<
<
< #include "fdlibm.h"
<
< #ifdef __STDC__
< static const double
< #else
< static double
< #endif
---
> static class Asin {
> private static final double
56,62c50
< #ifdef __STDC__
< double __ieee754_asin(double x)
< #else
< double __ieee754_asin(x)
< double x;
< #endif
< {
---
> static double compute(double x) {
83c71
< w = one-fabs(x);
---
> w = one-Math.abs(x);
87c75
< s = sqrt(t);
---
> s = Math.sqrt(t);
93c81,82
< __LO(w) = 0;
---
> // __LO(w) = 0;
> w = __LO(w, 0);
102c91,93
< /* __ieee754_acos(x)
---
> }
>
> /** Returns the arccosine of x.
125,132c116,117
<
< #include "fdlibm.h"
<
< #ifdef __STDC__
< static const double
< #else
< static double
< #endif
---
> static class Acos {
> private static final double
148,154c133
< #ifdef __STDC__
< double __ieee754_acos(double x)
< #else
< double __ieee754_acos(x)
< double x;
< #endif
< {
---
> static double compute(double x) {
177c156
< s = sqrt(z);
---
> s = Math.sqrt(z);
183c162
< s = sqrt(z);
---
> s = Math.sqrt(z);
185c164,165
< __LO(df) = 0;
---
> // __LO(df) = 0;
> df = __LO(df, 0);
194c174,176
< /* atan(x)
---
> }
>
> /* Returns the arctangent of x.
213,220c195,196
<
< #include "fdlibm.h"
<
< #ifdef __STDC__
< static const double atanhi[] = {
< #else
< static double atanhi[] = {
< #endif
---
> static class Atan {
> private static final double atanhi[] = {
227,231c203
< #ifdef __STDC__
< static const double atanlo[] = {
< #else
< static double atanlo[] = {
< #endif
---
> private static final double atanlo[] = {
238,242c210
< #ifdef __STDC__
< static const double aT[] = {
< #else
< static double aT[] = {
< #endif
---
> private static final double aT[] = {
256,260c224
< #ifdef __STDC__
< static const double
< #else
< static double
< #endif
---
> private static final double
264,270c228
< #ifdef __STDC__
< double atan(double x)
< #else
< double atan(x)
< double x;
< #endif
< {
---
> static double compute(double x) {
288c246
< x = fabs(x);
---
> x = Math.abs(x);
313a272
> }
Transliteration port vs more idiomatic port:
$ diff -w InverseTrig.translit.java InverseTrig.fdlibm.java
31a32,33
> private Asin() {throw new UnsupportedOperationException();}
>
33,48c35,48
< one = 1.e+00, /* 0x3FF0, 0x */
< huge = 1.000e+300,
< pio2_hi = 1.57079632679489655800e+00, /* 0x3FF921FB, 0x54442D18
*/
< pio2_lo = 6.12323399573676603587e-17, /* 0x3C91A626, 0x33145C07
*/
< pio4_hi = 7.85398163397448278999e-01, /* 0x3FE921FB, 0x54442D18
*/
< /* coefficient for R(x^2) */
< pS0 = 1.66657415e-01, /* 0x3FC5, 0x */
< pS1 = -3.25565818622400915405e-01, /* 0xBFD4D612, 0x03EB6F7D */
< pS2 = 2.01212532134862925881e-01, /* 0x3FC9C155, 0x0E884455 */
< pS3 = -4.00555345006794114027e-02, /* 0xBFA48228, 0xB5688F3B */
< pS4 = 7.91534994289814532176e-04, /* 0x3F49EFE0, 0x7501B288 */
< pS5 = 3.47933107596021167570e-05, /* 0x3F023DE1, 0x0DFDF709 */
< qS1 = -2.40339491173441421878e+00, /* 0xC0033A27, 0x1C8A2D4B */
< qS2 = 2.02094576023350569471e+00, /* 0x40002AE5, 0x9C598AC8 */
< qS3 = -6.88283971605453293030e-01, /* 0xBFE6066C, 0x1B8D0159 */
< qS4 = 7.70381505559019352791e-02; /* 0x3FB3B8C5, 0xB12E9282 */
---
> pio2_hi = 0x1.921fb54442d18p0, // 1.57079632679489655800e+00
> pio2_lo = 0x1.1a62633145c07p-54, // 6.12323399573676603587e-17
> pio4_hi = 0x1.921fb54442d18p-1, // 7.85398163397448278999e-01
> // coefficient for R(x^2)
> pS0 = 0x1.5p-3, // 1.66657415e-01
> pS1 = -0x1.4d61203eb6f7dp-2, // -3.25565818622400915405e-01
> pS2 = 0x1.9c1550e884455p-3, // 2.01212532134862925881e-01
> pS3 = -0x1.48228b5688f3bp-5, // -4.00555345006794114027e-02
> pS4 = 0x1.9efe07501b288p-11,// 7.91534994289814532176e-04
> pS5 = 0x1.23de10dfdf709p-15,// 3.47933107596021167570e-05
> qS1
RFR: JDK-8302026: Port fdlibm inverse trig functions (asin, acos, atan) to Java
Proceeding down the line of FDLIBM functions to be ported, next up are asin, acos, and atan. Diffs of the various versions will follow in a separate message. There were no unusual coding idioms encountered in porting these methods. - Commit messages: - Appease jcheck. - Add regression test. - Switch to hex literals, misc. updates. - Update exhausting tests. - Merge branch 'master' into JDK-8302026 - Reformat test. - Merge branch 'master' into JDK-8302026 - JDK-8302026: Port fdlibm inverse trig functions (asin, acos, atan) to Java Changes: https://git.openjdk.org/jdk/pull/12545/files Webrev: https://webrevs.openjdk.org/?repo=jdk=12545=00 Issue: https://bugs.openjdk.org/browse/JDK-8302026 Stats: 1041 lines in 6 files changed: 1034 ins; 0 del; 7 mod Patch: https://git.openjdk.org/jdk/pull/12545.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12545/head:pull/12545 PR: https://git.openjdk.org/jdk/pull/12545
RFR: JDK-8301460: Clean up LambdaForm to reference BasicType enums directly
`LambdaForm` declares int constants for `BasicType::ordinal` to workaround JDK-8161245. Now these int constants are no longer needed.This removes these int constants and reference `BasicType` enums directly. - Commit messages: - JDK-8301460: Code in LambdaForm.java still refers to resolved JDK-8161245 Changes: https://git.openjdk.org/jdk/pull/12546/files Webrev: https://webrevs.openjdk.org/?repo=jdk=12546=00 Issue: https://bugs.openjdk.org/browse/JDK-8301460 Stats: 29 lines in 3 files changed: 9 ins; 10 del; 10 mod Patch: https://git.openjdk.org/jdk/pull/12546.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12546/head:pull/12546 PR: https://git.openjdk.org/jdk/pull/12546
Re: RFR: 8302260: VarHandle.describeConstable() fails to return a nominal descriptor for static public fields
On Mon, 13 Feb 2023 19:35:52 GMT, Mandy Chung wrote: > I overlooked in the fix for JDK-8297757 that it should have passed the > declaring class of the static fields rather than the reference class passed > to `Lookup::findStaticVarHandle`. `InternalError` is thrown if the field is not found as it's not declared in `refc`. Or if the field is declared in the superclass or superinterface and just happens to find a field at the same offset in `refc`, a wrong `VarHandleDesc` is produced. - PR: https://git.openjdk.org/jdk/pull/12543
Re: RFR: 8301873: Avoid string decoding in ZipFile.Source.getEntryPos [v9]
On Thu, 9 Feb 2023 12:07:04 GMT, Eirik Bjorsnos wrote: >> After finding a hash match, getEntryPos needs to compare the lookup name up >> to the encoded entry name in the CEN. This comparison is done by decoding >> the entry name into a String. The names can then be compared using the >> String API. This decoding step adds a significat cost to this method. >> >> This PR suggest to update the string comparison such that in the common case >> where both the lookup name and the entry name are encoded in >> ASCII-compatible UTF-8, decoding can be avoided and the byte arrays can >> instead be compared direcly. >> >> ZipCoder is updated with a new method to compare a string with an encoded >> byte array range. The default implementation decodes to string (like the >> current code), while the UTF-8 implementation uses >> JavaLangAccess.getBytesNoRepl to get the bytes. Both methods thes uses >> Arrays.mismatch for comparison with or without matching trailing slashes. >> >> Additionally, this PR suggest to make the following updates to getEntryPos: >> >> - The try/catch for IAE is redundant and can be safely removed. (initCEN >> already checks this and will throws IAE for invalid UTF-8). This seems to >> give a 3-4% speedup on micros) >> - A new test InvalidBytesInEntryNameOrComment is a added to verify that >> initCEN does in fact reject invalid UTF-8 in CEN file names and comments. (I >> found no existing test coverage for this) >> - The recursion when looking for "name/" matches is replaced with iteration. >> We keep track of any "name/" match and return it at the end of the search. >> (I feel this is easier to follow and it also gives a ~30% speedup for >> addSlash lookups with no regression on regular lookups) >> >> (My though is that including these additional updates in this PR might >> reduce reviewer overhead given that it touches the exact same code. I might >> be wrong on this, please advise :) >> >> I'm seeing a ~17% saving on the micro ZipFileGetEntry.getEntryHit (modified >> to use xalan.jar): >> >> Baseline: >> >> Benchmark (size) Mode CntScore Error >> Units >> ZipFileGetEntry.getEntryHit 512 avgt 15 74.941 ± 1.004 >> ns/op >> ZipFileGetEntry.getEntryHit 1024 avgt 15 84.943 ± 1.320 >> ns/op >> ZipFileGetEntry.getEntryHitUncached 512 avgt 15 120.371 ± 2.386 >> ns/op >> ZipFileGetEntry.getEntryHitUncached 1024 avgt 15 126.128 ± 1.075 >> ns/op >> ZipFileGetEntry.getEntryMiss 512 avgt 15 23.818 ± 0.838 >> ns/op >> ZipFileGetEntry.getEntryMiss1024 avgt 15 29.762 ± 5.998 >> ns/op >> ZipFileGetEntry.getEntryMissUncached 512 avgt 15 59.405 ± 0.545 >> ns/op >> ZipFileGetEntry.getEntryMissUncached1024 avgt 15 71.840 ± 2.455 >> ns/op >> ZipFileGetEntry.getEntrySlash512 avgt 15 135.621 ± 4.341 >> ns/op >> ZipFileGetEntry.getEntrySlash 1024 avgt 15 134.190 ± 2.141 >> ns/op >> >> >> >> PR: >> >> >> Benchmark (size) Mode CntScore Error >> Units >> ZipFileGetEntry.getEntryHit 512 avgt 15 62.267 ± 1.329 >> ns/op >> ZipFileGetEntry.getEntryHit 1024 avgt 15 72.916 ± 2.428 >> ns/op >> ZipFileGetEntry.getEntryHitUncached 512 avgt 15 101.630 ± 1.154 >> ns/op >> ZipFileGetEntry.getEntryHitUncached 1024 avgt 15 113.161 ± 0.502 >> ns/op >> ZipFileGetEntry.getEntryMiss 512 avgt 15 23.003 ± 1.191 >> ns/op >> ZipFileGetEntry.getEntryMiss1024 avgt 15 23.236 ± 1.114 >> ns/op >> ZipFileGetEntry.getEntryMissUncached 512 avgt 15 56.781 ± 1.505 >> ns/op >> ZipFileGetEntry.getEntryMissUncached1024 avgt 15 67.767 ± 1.963 >> ns/op >> ZipFileGetEntry.getEntrySlash512 avgt 15 73.745 ± 2.717 >> ns/op >> ZipFileGetEntry.getEntrySlash 1024 avgt 15 75.784 ± 1.051 >> ns/op >> >> >> To assess the impact on startup/warmup, I made a main method class which >> measures the total time of calling ZipFile.getEntry for all entries in the >> 109 JAR file dependenies of spring-petclinic. The shows a nice improvement >> (time in micros): >> >> >> Percentile Baseline Patch >> 50 % 23155 21149 >> 75 % 23598 21454 >> 90 % 23989 21691 >> 95 % 24238 21973 >> 99 % 25270 22446 >> STDEV 792 549 >> Count 500 500 > > Eirik Bjorsnos has updated the pull request incrementally with one additional > commit since the last revision: > > Revert accidental removal of UTF8ZipCoder.compare Thank you Eirik for your efforts here. I am running your latest changes through our internal Mach5 systems. Additional comments below which are meant to add clarity for future maintainers src/java.base/share/classes/java/util/zip/ZipCoder.java line 66: > 64: NO_MATCH > 65:
Re: RFR: 8302260: VarHandle.describeConstable() fails to return a nominal descriptor for static public fields
On Mon, 13 Feb 2023 19:35:52 GMT, Mandy Chung wrote: > I overlooked in the fix for JDK-8297757 that it should have passed the > declaring class of the static fields rather than the reference class passed > to `Lookup::findStaticVarHandle`. Doh! I think the previous fix may have also result in an InternalError in some cases? since the field may not be found from the declaring class in `VarHandles::getStaticFieldFromBaseAndOffset`? - PR: https://git.openjdk.org/jdk/pull/12543
Re: RFR: 8205592: BigDecimal.doubleValue() is depressingly slow
On Mon, 13 Feb 2023 17:53:46 GMT, Andriy Plokhotnyuk wrote:
>> This PR is waiting for a review
>
> @rgiulietti Thanks for keeping making JDK faster!
>
> I have a couple of review comments:
>
> 1) For the line
> https://github.com/openjdk/jdk/pull/9410/files#diff-94d400b99466045dd76001c37eada6b24d086d8d115b49c439752bbceb233772L3746
>
> It seems that removing this check for 22-bit number introduces an extra
> rounding error.
>
> Please consider just increasing the constant up to `1L << 32` and using
> `DOUBLE_10_POW` instead of `FLOAT_10_POW`.
>
> Bellow are REPL expressions that illustrate my thoughts (sorry for using
> Scala):
>
> $ scala
> Welcome to Scala 3.2.0 (17.0.5, Java OpenJDK 64-Bit Server VM).
> Type in expressions for evaluation. Or try :help.
>
>
>
> scala> new java.math.BigDecimal("167772170E-1").floatValue
> val res0: Float = 1.6777216E7
>
>
>
> scala> 167772170E-1f
> val res1: Float = 1.6777216E7
>
>
>
> scala> 167772170 / 10.0f
> val res2: Float = 1.6777218E7
>
> scala> (167772170 / 10.0).toFloat
> val res3: Float = 1.6777216E7
>
> 2) For the line:
> https://github.com/openjdk/jdk/pull/9410/files#diff-94d400b99466045dd76001c37eada6b24d086d8d115b49c439752bbceb233772L3791
>
> It seems that removing this check for 52-bit number introduces an extra
> rounding error:
>
> $ scala
> Welcome to Scala 3.2.0 (17.0.6, Java OpenJDK 64-Bit Server VM).
> Type in expressions for evaluation. Or try :help.
>
> scala> new java.math.BigDecimal("90071992547409930E-1").doubleValue
> val res0: Double = 9.007199254740992E15
>
>
>
> scala> 90071992547409930E-1
> val res1: Double = 9.007199254740992E15
>
>
>
> scala> 90071992547409930L / 10.0
> val res2: Double = 9.007199254740994E15
>
> 3) For the line:
> https://github.com/openjdk/jdk/pull/9410/files#diff-94d400b99466045dd76001c37eada6b24d086d8d115b49c439752bbceb233772L3800
> Here before falling back to the costly fallback with big number operations we
> can [get a number of digits in
> intCompact](https://github.com/plokhotnyuk/jsoniter-scala/blob/6f702ce5cae05df91b5aa6e4bd61acdf43bf18f6/jsoniter-scala-core/jvm/src/main/scala/com/github/plokhotnyuk/jsoniter_scala/core/JsonWriter.scala#L1930)
> and use [a trick with two
> multiplications](https://github.com/plokhotnyuk/jsoniter-scala/blob/6f702ce5cae05df91b5aa6e4bd61acdf43bf18f6/jsoniter-scala-core/jvm/src/main/scala/com/github/plokhotnyuk/jsoniter_scala/core/JsonReader.scala#L1459)
@plokhotnyuk
The expression `new java.math.BigDecimal("90071992547409930E-1").doubleValue()`
would be processed by the proposed code as follows.
We have:
`intCompact = 90071992547409930L`
`scale = 1`
L.3838 sets `v == 9.007199254740994E16`
The test `(long) v == intCompact` on L. 3848 fails, so the fast path is not
executed.
The slow path then ensures that the conversion is affected by at most one
rounding error.
On the other hand, the expression
`scala> 90071992547409930L / 10.0`
is subject to two rounding errors, but it i executed neither by the current nor
by the proposed code.
A similar analysis holds for the `float` example.
As for the last paragraph, I'll have to have a deeper look. Stay tuned ;-)
-
PR: https://git.openjdk.org/jdk/pull/9410
Re: RFR: 8302337: JDK crashes if lib/modules contains non-zero byte containing ATTRIBUTE_END
On Mon, 13 Feb 2023 16:57:17 GMT, Severin Gehwolf wrote: > The `jimage` location attributes are terminated with `ATTRIBUTE_END`-kinds. > However, > the byte containing `ATTRIBUTE_END` (most significant 5 bits, represent > `kind`), might > be non-zero in the lower 3 bits (values up to `0x07` represent > `ATTRIBUTE_END`). The JDK code > handles this case correctly in > [`ImageLocation.decompress()`](https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/jdk/internal/jimage/ImageLocation.java#L69..L71). > However, the `libjimage` > code in `java.base` doesn't. That can lead to segfaults reading random bytes > and offsets. > > I propose to break the loop if `ATTRIBUTE_END` is being encountered so that > reading stops. > > Testing: > - [x] `test/jdk/tools/jimage` and `test/jdk/jdk/internal/jimage` tests. > - [x] Manual testing with a patched JDK to write non-zero bytes containing > `ATTRIBUTE_END` into the jimage. Segfaults before, runs fine after. > - [ ] GHA. In progress. > > Thoughts? Marked as reviewed by alanb (Reviewer). The change looks good, I'm just curious whether you observed a crash or whether this was noticed by inspection. - PR: https://git.openjdk.org/jdk/pull/12539
Re: RFR: 8302337: JDK crashes if lib/modules contains non-zero byte containing ATTRIBUTE_END
On Mon, 13 Feb 2023 16:57:17 GMT, Severin Gehwolf wrote: > The `jimage` location attributes are terminated with `ATTRIBUTE_END`-kinds. > However, > the byte containing `ATTRIBUTE_END` (most significant 5 bits, represent > `kind`), might > be non-zero in the lower 3 bits (values up to `0x07` represent > `ATTRIBUTE_END`). The JDK code > handles this case correctly in > [`ImageLocation.decompress()`](https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/jdk/internal/jimage/ImageLocation.java#L69..L71). > However, the `libjimage` > code in `java.base` doesn't. That can lead to segfaults reading random bytes > and offsets. > > I propose to break the loop if `ATTRIBUTE_END` is being encountered so that > reading stops. > > Testing: > - [x] `test/jdk/tools/jimage` and `test/jdk/jdk/internal/jimage` tests. > - [x] Manual testing with a patched JDK to write non-zero bytes containing > `ATTRIBUTE_END` into the jimage. Segfaults before, runs fine after. > - [ ] GHA. In progress. > > Thoughts? LGTM - Marked as reviewed by jlaskey (Reviewer). PR: https://git.openjdk.org/jdk/pull/12539
RFR: 8302260: VarHandle.describeConstable() fails to return a nominal descriptor for static public fields
I overlooked in the fix for JDK-8297757 that it should have passed the declaring class of the static fields rather than the reference class passed to `Lookup::findStaticVarHandle`. - Commit messages: - 8302260: VarHandle.describeConstable() fails to return a nominal descriptor for static public fields Changes: https://git.openjdk.org/jdk/pull/12543/files Webrev: https://webrevs.openjdk.org/?repo=jdk=12543=00 Issue: https://bugs.openjdk.org/browse/JDK-8302260 Stats: 231 lines in 7 files changed: 190 ins; 0 del; 41 mod Patch: https://git.openjdk.org/jdk/pull/12543.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12543/head:pull/12543 PR: https://git.openjdk.org/jdk/pull/12543
Re: RFR: 8302337: JDK crashes if lib/modules contains non-zero byte containing ATTRIBUTE_END
On Mon, 13 Feb 2023 16:57:17 GMT, Severin Gehwolf wrote: > The `jimage` location attributes are terminated with `ATTRIBUTE_END`-kinds. > However, > the byte containing `ATTRIBUTE_END` (most significant 5 bits, represent > `kind`), might > be non-zero in the lower 3 bits (values up to `0x07` represent > `ATTRIBUTE_END`). The JDK code > handles this case correctly in > [`ImageLocation.decompress()`](https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/jdk/internal/jimage/ImageLocation.java#L69..L71). > However, the `libjimage` > code in `java.base` doesn't. That can lead to segfaults reading random bytes > and offsets. > > I propose to break the loop if `ATTRIBUTE_END` is being encountered so that > reading stops. > > Testing: > - [x] `test/jdk/tools/jimage` and `test/jdk/jdk/internal/jimage` tests. > - [x] Manual testing with a patched JDK to write non-zero bytes containing > `ATTRIBUTE_END` into the jimage. Segfaults before, runs fine after. > - [ ] GHA. In progress. > > Thoughts? Your patch looks fine to me. Actually slightly clearer to the casual reader than the comparison <=7 the JDK does. The (preexisting) loop condition is weird; when would data ever become NULL? src/java.base/share/native/libjimage/imageFile.cpp line 132: > 130: // Extract kind from header byte. > 131: u1 kind = attribute_kind(byte); > 132: assert(kind < ATTRIBUTE_COUNT && "invalid image location > attribute"); Not your patch, but the assert is superfluous since attribute_kind already asserts. - Marked as reviewed by stuefe (Reviewer). PR: https://git.openjdk.org/jdk/pull/12539
Re: RFR: 8205592: BigDecimal.doubleValue() is depressingly slow
On Tue, 31 Jan 2023 13:39:46 GMT, Raffaello Giulietti
wrote:
>> A reimplementation of `BigDecimal.[double|float]Value()` to enhance
>> performance, avoiding an intermediate string and its subsequent parsing on
>> the slow path.
>
> This PR is waiting for a review
@rgiulietti Thanks for keeping making JDK faster!
I have a couple of review comments:
1) For the line
https://github.com/openjdk/jdk/pull/9410/files#diff-94d400b99466045dd76001c37eada6b24d086d8d115b49c439752bbceb233772L3746
It seems that removing this check for 22-bit number introduces an extra
rounding error.
Please consider just increasing the constant up to `1L << 32` and using
`DOUBLE_10_POW` instead of `FLOAT_10_POW`.
Bellow are REPL expressions that illustrate my thoughts (sorry for using Scala):
$ scala
Welcome to Scala 3.2.0 (17.0.5, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.
scala> new java.math.BigDecimal("167772170E-1").floatValue
val res0: Float = 1.6777216E7
scala> 167772170E-1f
val res1: Float = 1.6777216E7
scala> 167772170 / 10.0f
val res2: Float = 1.6777218E7
scala> (167772170 / 10.0).toFloat
val res3: Float = 1.6777216E7
2) For the line:
https://github.com/openjdk/jdk/pull/9410/files#diff-94d400b99466045dd76001c37eada6b24d086d8d115b49c439752bbceb233772L3791
It seems that removing this check for 52-bit number introduces an extra
rounding error:
$ scala
Welcome to Scala 3.2.0 (17.0.6, Java OpenJDK 64-Bit Server VM).
Type in expressions for evaluation. Or try :help.
scala> new java.math.BigDecimal("90071992547409930E-1").doubleValue
val res0: Double = 9.007199254740992E15
scala> 90071992547409930E-1
val res1: Double = 9.007199254740992E15
scala> 90071992547409930L / 10.0
val res2: Double = 9.007199254740994E15
3) For the line:
https://github.com/openjdk/jdk/pull/9410/files#diff-94d400b99466045dd76001c37eada6b24d086d8d115b49c439752bbceb233772L3800
Here before falling back to the costly fallback with big number operations we
can [get a number of digits in
intCompact](https://github.com/plokhotnyuk/jsoniter-scala/blob/6f702ce5cae05df91b5aa6e4bd61acdf43bf18f6/jsoniter-scala-core/jvm/src/main/scala/com/github/plokhotnyuk/jsoniter_scala/core/JsonWriter.scala#L1930)
and use [a trick with two
multiplications](https://github.com/plokhotnyuk/jsoniter-scala/blob/6f702ce5cae05df91b5aa6e4bd61acdf43bf18f6/jsoniter-scala-core/jvm/src/main/scala/com/github/plokhotnyuk/jsoniter_scala/core/JsonReader.scala#L1459)
-
PR: https://git.openjdk.org/jdk/pull/9410
Re: RFR: JDK-8300808: Accelerate Base64 on x86 for AVX2 [v15]
On Fri, 10 Feb 2023 23:18:47 GMT, Claes Redestad wrote: >> Scott Gibbons has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Add URL to microbenchmark > > Marked as reviewed by redestad (Reviewer). @cl4es Can you please initiate the in-depth testing required for this fix? Thanks. - PR: https://git.openjdk.org/jdk/pull/12126
RFR: 8302267: [jittester] Improve separation of test generation and execution
Please review a set of improvements that should improve working with other fuzzing generators and usage of JitTesterDriver with tests generated not by the JITTester: - Provide better separation of individual test generation from java file writing, compiling, executing, etc.; - Introduce distinct Phases of the generation process (Generation, Compilation, GoldRun and VerificationRun); - Extract JItTesterDriver headers generation so that it would be possible to provide other header generators; - Introduce error tolerance to not get distracted by OOMEs, intrinsics missing in the compiled code, etc.; - Make it possible to specify time limit for an individual test generation; - Give better control over temp/workdir creation and cleaning; - Unify external process running; - Introduce UTF-8 support in external processes' output and human-readable escaping of it; - Commit messages: - Remove tabs from a test to silence jcheck - 8302267: [jittester] Improve separation of test generation and execution Changes: https://git.openjdk.org/jdk/pull/12527/files Webrev: https://webrevs.openjdk.org/?repo=jdk=12527=00 Issue: https://bugs.openjdk.org/browse/JDK-8302267 Stats: 1044 lines in 14 files changed: 834 ins; 161 del; 49 mod Patch: https://git.openjdk.org/jdk/pull/12527.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12527/head:pull/12527 PR: https://git.openjdk.org/jdk/pull/12527
RFR: 8302337: JDK crashes if lib/modules contains non-zero byte containing ATTRIBUTE_END
The `jimage` location attributes are terminated with `ATTRIBUTE_END`-kinds. However, the byte containing `ATTRIBUTE_END` (most significant 5 bits, represent `kind`), might be non-zero in the lower 3 bits (values up to `0x07` represent `ATTRIBUTE_END`). The JDK code handles this case correctly in [`ImageLocation.decompress()`](https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/jdk/internal/jimage/ImageLocation.java#L69..L71). However, the `libjimage` code in `java.base` doesn't. That can lead to segfaults reading random bytes and offsets. I propose to break the loop if `ATTRIBUTE_END` is being encountered so that reading stops. Testing: - [x] `test/jdk/tools/jimage` and `test/jdk/jdk/internal/jimage` tests. - [x] Manual testing with a patched JDK to write non-zero bytes containing `ATTRIBUTE_END` into the jimage. Segfaults before, runs fine after. - [ ] GHA. In progress. Thoughts? - Commit messages: - 8302337: JDK crashes if lib/modules contains non-zero ATTRIBUTE_END bytes Changes: https://git.openjdk.org/jdk/pull/12539/files Webrev: https://webrevs.openjdk.org/?repo=jdk=12539=00 Issue: https://bugs.openjdk.org/browse/JDK-8302337 Stats: 3 lines in 1 file changed: 3 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/12539.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12539/head:pull/12539 PR: https://git.openjdk.org/jdk/pull/12539
Re: RFR: 8301226: Add clamp() methods to java.lang.Math and to StrictMath [v8]
On Mon, 13 Feb 2023 17:05:06 GMT, Quan Anh Mai wrote: >> Tagir F. Valeev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Comments in tests > > Just a small point: > >> Using similar approach in other cases (e.g. `float clamp(double, float, >> float))` may cause accidental precision loss even if the value is within >> range > > May I ask at which values of the parameters does this occur? AFAIU, every > `float` may be represented exactly by a `double`, which is the same situation > with `long` and `int`. @merykitty That clamp method would round a `double` to a `float`, an accidental precision loss. - PR: https://git.openjdk.org/jdk/pull/12428
Re: RFR: 8301226: Add clamp() methods to java.lang.Math and to StrictMath [v8]
On Tue, 7 Feb 2023 12:53:25 GMT, Tagir F. Valeev wrote: >> clamp() methods added to Math and StrictMath >> >> `int clamp(long, int, int)` is somewhat different, as it accepts a `long` >> value and safely clamps it to an `int` range. Other overloads work with a >> particular type (long, float and double). Using similar approach in other >> cases (e.g. `float clamp(double, float, float)`) may cause accidental >> precision loss even if the value is within range, so I decided to avoid this. >> >> In all cases, `max >= min` precondition should met. For double and float we >> additionally order `-0.0 < 0.0`, similarly to what Math.max or >> Double.compare do. In double and float overloads I try to keep at most one >> arg-check comparison on common path, so the order of checks might look >> unusual. >> >> For tests, I noticed that tests in java/lang/Math don't use any testing >> framework (even newer tests), so I somehow mimic the approach of neighbour >> tests. > > Tagir F. Valeev has updated the pull request incrementally with one > additional commit since the last revision: > > Comments in tests Ah yes that's my bad - PR: https://git.openjdk.org/jdk/pull/12428
Re: RFR: 8301226: Add clamp() methods to java.lang.Math and to StrictMath [v8]
On Tue, 7 Feb 2023 12:53:25 GMT, Tagir F. Valeev wrote: >> clamp() methods added to Math and StrictMath >> >> `int clamp(long, int, int)` is somewhat different, as it accepts a `long` >> value and safely clamps it to an `int` range. Other overloads work with a >> particular type (long, float and double). Using similar approach in other >> cases (e.g. `float clamp(double, float, float)`) may cause accidental >> precision loss even if the value is within range, so I decided to avoid this. >> >> In all cases, `max >= min` precondition should met. For double and float we >> additionally order `-0.0 < 0.0`, similarly to what Math.max or >> Double.compare do. In double and float overloads I try to keep at most one >> arg-check comparison on common path, so the order of checks might look >> unusual. >> >> For tests, I noticed that tests in java/lang/Math don't use any testing >> framework (even newer tests), so I somehow mimic the approach of neighbour >> tests. > > Tagir F. Valeev has updated the pull request incrementally with one > additional commit since the last revision: > > Comments in tests Just a small point: > Using similar approach in other cases (e.g. `float clamp(double, float, > float))` may cause accidental precision loss even if the value is within range May I ask at which values of the parameters does this occur? AFAIU, every `float` may be represented exactly by a `double`, which is the same situation with `long` and `int`. - PR: https://git.openjdk.org/jdk/pull/12428
Re: RFR: 8302163: Speed up various String comparison methods with ArraysSupport.mismatch [v2]
On Mon, 13 Feb 2023 16:10:14 GMT, Claes Redestad wrote: >> We can improve various String methods such as `startsWith`, `endsWith` and >> `regionMatches` by leveraging the intrinsified mismatch methods in >> `ArraysSupport`. > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > Clarify coder shift in startsWith Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.org/jdk/pull/12528
Re: RFR: 8301226: Add clamp() methods to java.lang.Math and to StrictMath [v8]
On Tue, 7 Feb 2023 12:53:25 GMT, Tagir F. Valeev wrote: >> clamp() methods added to Math and StrictMath >> >> `int clamp(long, int, int)` is somewhat different, as it accepts a `long` >> value and safely clamps it to an `int` range. Other overloads work with a >> particular type (long, float and double). Using similar approach in other >> cases (e.g. `float clamp(double, float, float)`) may cause accidental >> precision loss even if the value is within range, so I decided to avoid this. >> >> In all cases, `max >= min` precondition should met. For double and float we >> additionally order `-0.0 < 0.0`, similarly to what Math.max or >> Double.compare do. In double and float overloads I try to keep at most one >> arg-check comparison on common path, so the order of checks might look >> unusual. >> >> For tests, I noticed that tests in java/lang/Math don't use any testing >> framework (even newer tests), so I somehow mimic the approach of neighbour >> tests. > > Tagir F. Valeev has updated the pull request incrementally with one > additional commit since the last revision: > > Comments in tests Marked as reviewed by qamai (Committer). - PR: https://git.openjdk.org/jdk/pull/12428
Re: RFR: 8302163: Speed up various String comparison methods with ArraysSupport.mismatch [v2]
On Mon, 13 Feb 2023 16:10:14 GMT, Claes Redestad wrote:
>> We can improve various String methods such as `startsWith`, `endsWith` and
>> `regionMatches` by leveraging the intrinsified mismatch methods in
>> `ArraysSupport`.
>
> Claes Redestad has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Clarify coder shift in startsWith
Case-insensitive regionMatches could be improved by using
ArraysSupport.mismatch to skip over long common substrings:
PR:
Benchmark (size) (utf16) Mode CntScore Error
Units
StringComparisons.regionMatchesCI 6false avgt 158.248 ± 0.427
ns/op
StringComparisons.regionMatchesCI 15false avgt 15 11.408 ± 0.429
ns/op
StringComparisons.regionMatchesCI1024false avgt 15 328.796 ± 7.191
ns/op
Arrays.mismatch:
Benchmark (size) (utf16) Mode Cnt Score Error
Units
StringComparisons.regionMatchesCI 6false avgt 15 10.608 ± 0.302
ns/op
StringComparisons.regionMatchesCI 15false avgt 15 8.772 ± 0.347
ns/op
StringComparisons.regionMatchesCI1024false avgt 15 31.070 ± 1.783
ns/op
Patch on top of your PR:
Index: src/java.base/share/classes/java/lang/StringLatin1.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===
diff --git a/src/java.base/share/classes/java/lang/StringLatin1.java
b/src/java.base/share/classes/java/lang/StringLatin1.java
--- a/src/java.base/share/classes/java/lang/StringLatin1.java (revision
6cac333d8f9f34e16168447c60f28a6b0d31623f)
+++ b/src/java.base/share/classes/java/lang/StringLatin1.java (date
1676305817928)
@@ -384,6 +384,14 @@
byte[] other, int ooffset, int len) {
int last = toffset + len;
while (toffset < last) {
+int mismatch = ArraysSupport.mismatch(value, toffset, other,
ooffset, len);
+if (mismatch == -1) {
+return true;
+} else {
+toffset += mismatch;
+ooffset += mismatch;
+len -= mismatch + 1;
+}
char c1 = (char)(value[toffset++] & 0xff);
char c2 = (char)(other[ooffset++] & 0xff);
if (c1 == c2) {
Index: test/micro/org/openjdk/bench/java/lang/StringComparisons.java
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===
diff --git a/test/micro/org/openjdk/bench/java/lang/StringComparisons.java
b/test/micro/org/openjdk/bench/java/lang/StringComparisons.java
--- a/test/micro/org/openjdk/bench/java/lang/StringComparisons.java
(revision 6cac333d8f9f34e16168447c60f28a6b0d31623f)
+++ b/test/micro/org/openjdk/bench/java/lang/StringComparisons.java (date
1676305817914)
@@ -49,6 +49,7 @@
public String endsWithA;
public String endsWithB;
public String startsWithA;
+public String endsWitha;
@Setup
public void setup() {
@@ -56,6 +57,7 @@
string = c.repeat(size);
equalString = c.repeat(size);
endsWithA = c.repeat(size).concat("A");
+endsWitha = c.repeat(size).concat("a");
endsWithB = c.repeat(size).concat("B");
startsWithA = "A" + (c.repeat(size));
}
@@ -75,6 +77,11 @@
return endsWithA.regionMatches(0, endsWithB, 0, endsWithB.length());
}
+@Benchmark
+public boolean regionMatchesIC() {
+return endsWithA.regionMatches(true,0, endsWitha, 0,
endsWitha.length());
+}
+
@Benchmark
public boolean regionMatchesRange() {
return startsWithA.regionMatches(1, endsWithB, 0, endsWithB.length() -
1);
-
PR: https://git.openjdk.org/jdk/pull/12528
Re: RFR: 8302163: Speed up various String comparison methods with ArraysSupport.mismatch
On Mon, 13 Feb 2023 09:59:24 GMT, Claes Redestad wrote: > We can improve various String methods such as `startsWith`, `endsWith` and > `regionMatches` by leveraging the intrinsified mismatch methods in > `ArraysSupport`. src/java.base/share/classes/java/lang/String.java line 2272: > 2270: toffset <<= coder; > 2271: return ArraysSupport.mismatch(ta, toffset, > 2272: pa, 0, pc) < 0; `offset <<= coder` is only obvious if the reader knows the value of LATIN1, maybe it would be simpler to read if you kept "int to"? - PR: https://git.openjdk.org/jdk/pull/12528
Re: RFR: 8302163: Speed up various String comparison methods with ArraysSupport.mismatch
On Mon, 13 Feb 2023 09:59:24 GMT, Claes Redestad wrote: > We can improve various String methods such as `startsWith`, `endsWith` and > `regionMatches` by leveraging the intrinsified mismatch methods in > `ArraysSupport`. Look good. - Marked as reviewed by rriggs (Reviewer). PR: https://git.openjdk.org/jdk/pull/12528
Re: RFR: 8302163: Speed up various String comparison methods with ArraysSupport.mismatch
On Mon, 13 Feb 2023 09:59:24 GMT, Claes Redestad wrote: > We can improve various String methods such as `startsWith`, `endsWith` and > `regionMatches` by leveraging the intrinsified mismatch methods in > `ArraysSupport`. Looks simple and harmless. - Marked as reviewed by stsypanov (Author). PR: https://git.openjdk.org/jdk/pull/12528
RFR: 8302325: Wrong comment in java.base/share/native/libjimage/imageFile.hpp
Could I please get a review of this trivial comment-only change? `imageFile.hpp` describes some properties of the jimage file `lib/modules`. However, I don't think the comment example matches current code in the JDK. [`ATTRIBUTE_OFFSET`](https://github.com/openjdk/jdk/blob/master/src/java.base/share/classes/jdk/internal/jimage/ImageLocation.java#L44) get written to the image file with value `0x05` while the comment mentions it gets written as `0x04`. I propose to fix the comment so that it matches the code. In passing, I've also fixed a comment related to `ATTRIBUTE_END`. I think the point being made there is about byte values of `[0x01..0x07]`, all would represent `ATTRIBUTE_END`, as the upper `5` bits would be `0`. Therefore, byte `0x01` would equally represent `ATTRIBUTE_END` as would `0x00` and `0x07` or any value in between. Thoughts? - Commit messages: - 8302325: Wrong comment in java.base/share/native/libjimage/imageFile.hpp Changes: https://git.openjdk.org/jdk/pull/12533/files Webrev: https://webrevs.openjdk.org/?repo=jdk=12533=00 Issue: https://bugs.openjdk.org/browse/JDK-8302325 Stats: 4 lines in 2 files changed: 0 ins; 0 del; 4 mod Patch: https://git.openjdk.org/jdk/pull/12533.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12533/head:pull/12533 PR: https://git.openjdk.org/jdk/pull/12533
Re: RFR: 8301226: Add clamp() methods to java.lang.Math and to StrictMath [v8]
On Tue, 7 Feb 2023 12:53:25 GMT, Tagir F. Valeev wrote: >> clamp() methods added to Math and StrictMath >> >> `int clamp(long, int, int)` is somewhat different, as it accepts a `long` >> value and safely clamps it to an `int` range. Other overloads work with a >> particular type (long, float and double). Using similar approach in other >> cases (e.g. `float clamp(double, float, float)`) may cause accidental >> precision loss even if the value is within range, so I decided to avoid this. >> >> In all cases, `max >= min` precondition should met. For double and float we >> additionally order `-0.0 < 0.0`, similarly to what Math.max or >> Double.compare do. In double and float overloads I try to keep at most one >> arg-check comparison on common path, so the order of checks might look >> unusual. >> >> For tests, I noticed that tests in java/lang/Math don't use any testing >> framework (even newer tests), so I somehow mimic the approach of neighbour >> tests. > > Tagir F. Valeev has updated the pull request incrementally with one > additional commit since the last revision: > > Comments in tests Gentle ping: please review this pull request. CSR is already approved. Thank you. - PR: https://git.openjdk.org/jdk/pull/12428
Re: RFR: 8302163: Speed up various String comparison methods with ArraysSupport.mismatch
On Mon, 13 Feb 2023 09:59:24 GMT, Claes Redestad wrote: > We can improve various String methods such as `startsWith`, `endsWith` and > `regionMatches` by leveraging the intrinsified mismatch methods in > `ArraysSupport`. ` cat PR >> https://cl4es.github.io/` - PR: https://git.openjdk.org/jdk/pull/12528
Re: RFR: 8301958: Reduce Arrays.copyOf/-Range overheads [v10]
On Mon, 13 Feb 2023 09:59:52 GMT, Claes Redestad wrote: >> This patch adds special-cases to `Arrays.copyOf` and `Arrays.copyOfRange` to >> copy arrays more efficiently when exactly the whole input array is to be >> copied. This helps eliminate range checks and has been verified to help >> various String operations. Example: >> >> Baseline >> >> Benchmark(size) Mode Cnt >> Score Error Units >> StringConstructor.newStringFromArray 7 avgt 15 >> 16.817 ± 0.369 ns/op >> StringConstructor.newStringFromArrayWithCharset 7 avgt 15 >> 16.866 ± 0.449 ns/op >> StringConstructor.newStringFromArrayWithCharsetName 7 avgt 15 >> 22.198 ± 0.396 ns/op >> >> Patch: >> >> Benchmark(size) Mode Cnt >> Score Error Units >> StringConstructor.newStringFromArray 7 avgt 15 >> 14.666 ± 0.336 ns/op >> StringConstructor.newStringFromArrayWithCharset 7 avgt 15 >> 14.582 ± 0.288 ns/op >> StringConstructor.newStringFromArrayWithCharsetName 7 avgt 15 >> 20.339 ± 0.328 ns/op > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > Address review comments Thanks for reviewng! I've filed https://bugs.openjdk.org/browse/JDK-8302315 to investigate the clone/arraycopy performance discrepancy. Ideally we should be able to just do `array.clone()` here and get optimal performance with less code. - PR: https://git.openjdk.org/jdk/pull/12453
Re: RFR: 8301958: Reduce Arrays.copyOf/-Range overheads [v10]
On Mon, 13 Feb 2023 09:59:52 GMT, Claes Redestad wrote: >> This patch adds special-cases to `Arrays.copyOf` and `Arrays.copyOfRange` to >> copy arrays more efficiently when exactly the whole input array is to be >> copied. This helps eliminate range checks and has been verified to help >> various String operations. Example: >> >> Baseline >> >> Benchmark(size) Mode Cnt >> Score Error Units >> StringConstructor.newStringFromArray 7 avgt 15 >> 16.817 ± 0.369 ns/op >> StringConstructor.newStringFromArrayWithCharset 7 avgt 15 >> 16.866 ± 0.449 ns/op >> StringConstructor.newStringFromArrayWithCharsetName 7 avgt 15 >> 22.198 ± 0.396 ns/op >> >> Patch: >> >> Benchmark(size) Mode Cnt >> Score Error Units >> StringConstructor.newStringFromArray 7 avgt 15 >> 14.666 ± 0.336 ns/op >> StringConstructor.newStringFromArrayWithCharset 7 avgt 15 >> 14.582 ± 0.288 ns/op >> StringConstructor.newStringFromArrayWithCharsetName 7 avgt 15 >> 20.339 ± 0.328 ns/op > > Claes Redestad has updated the pull request incrementally with one additional > commit since the last revision: > > Address review comments Marked as reviewed by alanb (Reviewer). - PR: https://git.openjdk.org/jdk/pull/12453
Integrated: 8301958: Reduce Arrays.copyOf/-Range overheads
On Tue, 7 Feb 2023 13:30:35 GMT, Claes Redestad wrote: > This patch adds special-cases to `Arrays.copyOf` and `Arrays.copyOfRange` to > copy arrays more efficiently when exactly the whole input array is to be > copied. This helps eliminate range checks and has been verified to help > various String operations. Example: > > Baseline > > Benchmark(size) Mode Cnt > Score Error Units > StringConstructor.newStringFromArray 7 avgt 15 > 16.817 ± 0.369 ns/op > StringConstructor.newStringFromArrayWithCharset 7 avgt 15 > 16.866 ± 0.449 ns/op > StringConstructor.newStringFromArrayWithCharsetName 7 avgt 15 > 22.198 ± 0.396 ns/op > > Patch: > > Benchmark(size) Mode Cnt > Score Error Units > StringConstructor.newStringFromArray 7 avgt 15 > 14.666 ± 0.336 ns/op > StringConstructor.newStringFromArrayWithCharset 7 avgt 15 > 14.582 ± 0.288 ns/op > StringConstructor.newStringFromArrayWithCharsetName 7 avgt 15 > 20.339 ± 0.328 ns/op This pull request has now been integrated. Changeset: 1f9c110c Author:Claes Redestad URL: https://git.openjdk.org/jdk/commit/1f9c110c1f9ea6f5c3621a25692ce9d7abf245d4 Stats: 209 lines in 2 files changed: 179 ins; 21 del; 9 mod 8301958: Reduce Arrays.copyOf/-Range overheads Reviewed-by: alanb, smarks - PR: https://git.openjdk.org/jdk/pull/12453
Integrated: 8300139 : [AIX] Use pthreads to avoid JNI_createVM call from primordial thread
On Tue, 31 Jan 2023 05:24:35 GMT, Varada M wrote:
> 1. test/jdk/jni/nullCaller/NullCallerTest.java
> 2. test/jdk/java/lang/reflect/exeCallerAccessTest/CallerAccessTest.java
> 3. test/hotspot/jtreg/runtime/jni/CalleeSavedRegisters/FPRegs.java
>
> The above tests were blocked on AIX [@require os.family != "aix"] because
> these tests are failing to call JNI_CreateJavaVM. This is solved by
> implementing JNI_CreateJavaVM call via POSIX threads.
>Similarly there are tests which are not blocked and still failing to call
> JNI_CreateJavaVM on AIX :
>
> 4. test/hotspot/jtreg/runtime/jni/daemonDestroy/TestDaemonDestroy.java { PR :
> [12006](https://github.com/openjdk/jdk/pull/12006) }
> 5. test/lib-test/jdk/test/lib/process/TestNativeProcessBuilder.java
>
> The reported issue : [8300139](https://bugs.openjdk.org/browse/JDK-8300139l)
This pull request has now been integrated.
Changeset: cb810730
Author:Varada M
Committer: Thomas Stuefe
URL:
https://git.openjdk.org/jdk/commit/cb8107303ed0563e06b1e2009d521869f4ca21e8
Stats: 129 lines in 7 files changed: 88 ins; 12 del; 29 mod
8300139: [AIX] Use pthreads to avoid JNI_createVM call from primordial thread
Reviewed-by: dholmes, stuefe
-
PR: https://git.openjdk.org/jdk/pull/12302
Re: RFR: 8301627: System.exit and Runtime.exit debug logging
On Sun, 12 Feb 2023 18:15:25 GMT, Alan Bateman wrote:
>> It can be difficult to find the cause of calls to
>> `java.lang.System.exit(status)` and `Runtime.exit(status)` because the Java
>> runtime exits.
>> The status value and stack trace are logged using the System Logger named
>> `java.lang.Runtime` with message level `System.Logger.Level.DEBUG`.
>
> src/java.base/share/classes/java/lang/Shutdown.java line 162:
>
>> 160: * If the system logger {@code java.lang.Runtime} is enabled for
>> logging level DEBUG/FINE
>> 161: * the stack trace of the call to {@code Runtime.exit()} or {@code
>> System.exit()}
>> 162: * is logged.
>
> Shutdown is not a public class so this impNote won't appear in the APIs docs.
> Should it move to Runtime.exit and System.exit? If it moved to a public API
> then "system logger" could link to System.Logger. Also would it be more
> correct to say "a system logger named "java.lang.Runtime" is enabled for
> logging levels DEBUG or FINE"?
FINE is not a level supported by the System.Logger, it is the level to which
DEBUG is mapped when the backend is java.util.logging. I suggest to remove FINE
from this description and add an `{@link Loger.Level#DEBUG DEBUG}` around DEBUG.
> test/jdk/java/lang/runtime/RuntimeExitLogTest.java line 89:
>
>> 87: }
>> 88: cmd.add(this.getClass().getName());
>> 89: cmd.add(Integer.toString(status));
>
> Another possibility for testing this is to launch with ` --limit-modules
> java.base -Djdk.system.logger.level=DEBUG` to use the simple console
> implementation that is in java.base and avoid needing properties files for
> j.u.logging. Just mentioning is an option to make it simple.
Good point - though maybe both configurations should be tested. The
j.u.l.LogManager registers some shutdown hook so I believe it's important to
test with j.u.l present too.
-
PR: https://git.openjdk.org/jdk/pull/12517
Re: RFR: 8302163: Speed up various String comparison methods with ArraysSupport.mismatch
On Mon, 13 Feb 2023 09:59:24 GMT, Claes Redestad wrote: > We can improve various String methods such as `startsWith`, `endsWith` and > `regionMatches` by leveraging the intrinsified mismatch methods in > `ArraysSupport`. Amazing gains! Congratulations - PR: https://git.openjdk.org/jdk/pull/12528
Re: RFR: 8302226 failure_handler native.core should wait for coredump to finish [v2]
> Update open/test/failure_handler/src/share/conf/linux.properties to handle > core dumps more correctly. Ludvig Janiuk has updated the pull request incrementally with one additional commit since the last revision: add mac - Changes: - all: https://git.openjdk.org/jdk/pull/12515/files - new: https://git.openjdk.org/jdk/pull/12515/files/9c79974a..8f4026f6 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=12515=01 - incr: https://webrevs.openjdk.org/?repo=jdk=12515=00-01 Stats: 5 lines in 1 file changed: 3 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/12515.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12515/head:pull/12515 PR: https://git.openjdk.org/jdk/pull/12515
Re: RFR: 8302163: Speed up various String comparison methods with ArraysSupport.mismatch
On Mon, 13 Feb 2023 09:59:24 GMT, Claes Redestad wrote: > We can improve various String methods such as `startsWith`, `endsWith` and > `regionMatches` by leveraging the intrinsified mismatch methods in > `ArraysSupport`. Microbenchmarking shows decent improvements on small data, scaling up to some impressive gains on large inputs when vectorization kicks in (~41x on regionMatches for size = 1024 on my x64 sandybridge setup, ~34x on m1 in the same config) macosx-aarch64, m1, 21-b9: Benchmark (size) (utf16) Mode Cnt Score Error Units StringComparisons.endsWith 6 true avgt 15 5,949 ± 0,051 ns/op StringComparisons.endsWith 6false avgt 15 4,063 ± 0,038 ns/op StringComparisons.endsWith15 true avgt 1510,758 ± 0,132 ns/op StringComparisons.endsWith15false avgt 15 6,487 ± 0,052 ns/op StringComparisons.endsWith 1024 true avgt 15 653,750 ± 2,835 ns/op StringComparisons.endsWith 1024false avgt 15 314,219 ± 1,264 ns/op StringComparisons.regionMatches6 true avgt 1512,460 ± 0,043 ns/op StringComparisons.regionMatches6false avgt 15 6,647 ± 0,026 ns/op StringComparisons.regionMatches 15 true avgt 1530,502 ± 0,193 ns/op StringComparisons.regionMatches 15false avgt 1515,073 ± 0,030 ns/op StringComparisons.regionMatches 1024 true avgt 15 2147,480 ± 4,622 ns/op StringComparisons.regionMatches 1024false avgt 15 1068,787 ± 14,098 ns/op StringComparisons.regionMatchesCI 6 true avgt 1511,680 ± 0,106 ns/op StringComparisons.regionMatchesCI 6false avgt 15 4,577 ± 0,101 ns/op StringComparisons.regionMatchesCI 15 true avgt 1514,422 ± 0,132 ns/op StringComparisons.regionMatchesCI 15false avgt 15 6,904 ± 0,124 ns/op StringComparisons.regionMatchesCI 1024 true avgt 15 273,810 ± 3,446 ns/op StringComparisons.regionMatchesCI 1024false avgt 15 226,040 ± 2,886 ns/op StringComparisons.regionMatchesRange 6 true avgt 1511,896 ± 0,110 ns/op StringComparisons.regionMatchesRange 6false avgt 15 6,044 ± 0,034 ns/op StringComparisons.regionMatchesRange 15 true avgt 1529,508 ± 0,093 ns/op StringComparisons.regionMatchesRange 15false avgt 1514,336 ± 0,020 ns/op StringComparisons.regionMatchesRange1024 true avgt 15 2187,600 ± 80,285 ns/op StringComparisons.regionMatchesRange1024false avgt 15 1105,813 ± 28,260 ns/op StringComparisons.startsWith 6 true avgt 15 5,315 ± 0,087 ns/op StringComparisons.startsWith 6false avgt 15 3,588 ± 0,020 ns/op StringComparisons.startsWith 15 true avgt 15 9,823 ± 0,144 ns/op StringComparisons.startsWith 15false avgt 15 5,963 ± 0,253 ns/op StringComparisons.startsWith1024 true avgt 15 441,854 ± 9,295 ns/op StringComparisons.startsWith1024false avgt 15 224,386 ± 6,876 ns/op macosx-aarch64. m1, patched: Benchmark (size) (utf16) Mode CntScore Error Units StringComparisons.endsWith 6 true avgt 153,185 ± 0,063 ns/op StringComparisons.endsWith 6false avgt 154,507 ± 0,447 ns/op StringComparisons.endsWith15 true avgt 156,097 ± 0,142 ns/op StringComparisons.endsWith15false avgt 155,736 ± 0,025 ns/op StringComparisons.endsWith 1024 true avgt 15 60,283 ± 4,109 ns/op StringComparisons.endsWith 1024false avgt 15 31,011 ± 0,080 ns/op StringComparisons.regionMatches6 true avgt 153,993 ± 0,063 ns/op StringComparisons.regionMatches6false avgt 154,836 ± 0,474 ns/op StringComparisons.regionMatches 15 true avgt 153,641 ± 0,012 ns/op StringComparisons.regionMatches 15false avgt 152,849 ± 0,065 ns/op StringComparisons.regionMatches 1024 true avgt 15 57,739 ± 0,748 ns/op StringComparisons.regionMatches 1024false avgt 15 30,943 ± 0,423 ns/op StringComparisons.regionMatchesCI 6 true avgt 15 11,729 ± 0,142 ns/op StringComparisons.regionMatchesCI 6false avgt 154,562 ± 0,125 ns/op StringComparisons.regionMatchesCI 15 true avgt 15 14,611 ± 0,227 ns/op StringComparisons.regionMatchesCI 15false avgt 156,970 ± 0,083 ns/op StringComparisons.regionMatchesCI 1024 true avgt 15
RFR: 8302163: Speed up various String comparison methods with ArraysSupport.mismatch
We can improve various String methods such as `startsWith`, `endsWith` and `regionMatches` by leveraging the intrinsified mismatch methods in `ArraysSupport`. - Commit messages: - Remove overlapping micros, extend testing to endsWith, regionCI and some minor improvements to String::regionMatches - Expand micro coverage - Add micro from @eirbjo - Revert UTF16.compareValues - Add a few micros, apply optimization to StringUTF16.compareValues - Speed up various String comparison methods with ArraysSupport.mismatch Changes: https://git.openjdk.org/jdk/pull/12528/files Webrev: https://webrevs.openjdk.org/?repo=jdk=12528=00 Issue: https://bugs.openjdk.org/browse/JDK-8302163 Stats: 170 lines in 5 files changed: 105 ins; 44 del; 21 mod Patch: https://git.openjdk.org/jdk/pull/12528.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12528/head:pull/12528 PR: https://git.openjdk.org/jdk/pull/12528
Re: RFR: 8301958: Reduce Arrays.copyOf/-Range overheads [v10]
> This patch adds special-cases to `Arrays.copyOf` and `Arrays.copyOfRange` to > copy arrays more efficiently when exactly the whole input array is to be > copied. This helps eliminate range checks and has been verified to help > various String operations. Example: > > Baseline > > Benchmark(size) Mode Cnt > Score Error Units > StringConstructor.newStringFromArray 7 avgt 15 > 16.817 ± 0.369 ns/op > StringConstructor.newStringFromArrayWithCharset 7 avgt 15 > 16.866 ± 0.449 ns/op > StringConstructor.newStringFromArrayWithCharsetName 7 avgt 15 > 22.198 ± 0.396 ns/op > > Patch: > > Benchmark(size) Mode Cnt > Score Error Units > StringConstructor.newStringFromArray 7 avgt 15 > 14.666 ± 0.336 ns/op > StringConstructor.newStringFromArrayWithCharset 7 avgt 15 > 14.582 ± 0.288 ns/op > StringConstructor.newStringFromArrayWithCharsetName 7 avgt 15 > 20.339 ± 0.328 ns/op Claes Redestad has updated the pull request incrementally with one additional commit since the last revision: Address review comments - Changes: - all: https://git.openjdk.org/jdk/pull/12453/files - new: https://git.openjdk.org/jdk/pull/12453/files/350612d4..1031dd85 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=12453=09 - incr: https://webrevs.openjdk.org/?repo=jdk=12453=08-09 Stats: 31 lines in 1 file changed: 7 ins; 8 del; 16 mod Patch: https://git.openjdk.org/jdk/pull/12453.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12453/head:pull/12453 PR: https://git.openjdk.org/jdk/pull/12453
