Re: RFR: JDK-8277095 : Empty streams create too many objects [v2]
On Tue, 16 Nov 2021 20:53:26 GMT, kabutz wrote: >> This is a draft proposal for how we could improve stream performance for the >> case where the streams are empty. Empty collections are common-place. If we >> iterate over them with an Iterator, we would have to create one small >> Iterator object (which could often be eliminated) and if it is empty we are >> done. However, with Streams we first have to build up the entire pipeline, >> until we realize that there is no work to do. With this example, we change >> Collection#stream() to first check if the collection is empty, and if it is, >> we simply return an EmptyStream. We also have EmptyIntStream, >> EmptyLongStream and EmptyDoubleStream. We have taken great care for these to >> have the same characteristics and behaviour as the streams returned by >> Stream.empty(), IntStream.empty(), etc. >> >> Some of the JDK tests fail with this, due to ClassCastExceptions (our >> EmptyStream is not an AbstractPipeline) and AssertionError, since we can >> call some methods repeatedly on the stream without it failing. On the plus >> side, creating a complex stream on an empty stream gives us upwards of 50x >> increase in performance due to a much smaller object allocation rate. This >> PR includes the code for the change, unit tests and also a JMH benchmark to >> demonstrate the improvement. > > kabutz has updated the pull request incrementally with one additional commit > since the last revision: > > Refactored empty stream implementations to reduce duplicate code and > improved unordered() > Added StreamSupport.empty for primitive spliterators and use that in > Arrays.stream() Add another comment to keep this active. - PR: https://git.openjdk.java.net/jdk/pull/6275
Re: [jdk18] RFR: 8277964: ClassCastException with no stack trace is thrown with -Xcomp in method handle invocation
On Wed, 15 Dec 2021 05:59:45 GMT, Vladimir Kozlov wrote:
> A proper fix for this is to use the catchException combination. However, that
> introduces significant cold startup performance regression. JDK-8278447
> tracks the work to address the performance regression using catchException
> and asSpreader combinator. It may require significant work and refactoring
> which is risky for JDK 18.
>
> It is proposed to implement a workaround in C2 to white list the relevant
> methods (all methods in sun.invoke.util.ValueConversions class) not to omit
> stack trace when exception is thrown in them.
>
> Added new regression test. Tested tier1-3.
src/hotspot/share/oops/method.cpp line 827:
> 825: */
> 826: bool Method::can_omit_stack_trace() {
> 827: if (method_holder()->class_loader_data()->is_boot_class_loader_data())
> {
Do you actually need to check this?
-
PR: https://git.openjdk.java.net/jdk18/pull/27
[jdk18] RFR: 8277964: ClassCastException with no stack trace is thrown with -Xcomp in method handle invocation
A proper fix for this is to use the catchException combination. However, that introduces significant cold startup performance regression. JDK-8278447 tracks the work to address the performance regression using catchException and asSpreader combinator. It may require significant work and refactoring which is risky for JDK 18. It is proposed to implement a workaround in C2 to white list the relevant methods (all methods in sun.invoke.util.ValueConversions class) not to omit stack trace when exception is thrown in them. Added new regression test. Tested tier1-3. - Commit messages: - 8277964: ClassCastException with no stack trace is thrown with -Xcomp in method handle invocation Changes: https://git.openjdk.java.net/jdk18/pull/27/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk18=27=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8277964 Stats: 151 lines in 7 files changed: 149 ins; 1 del; 1 mod Patch: https://git.openjdk.java.net/jdk18/pull/27.diff Fetch: git fetch https://git.openjdk.java.net/jdk18 pull/27/head:pull/27 PR: https://git.openjdk.java.net/jdk18/pull/27
[jdk18] Integrated: 8278233: [macos] tools/jpackage tests timeout due to /usr/bin/osascript
On Tue, 14 Dec 2021 06:10:51 GMT, Alexander Matveev wrote: > This is regression from JDK-8276837. exec() was passing INFINITE_TIMEOUT > instead of actual value of timeout variable. Execution of osascript was > running without timeout and thus several tests timeout. Osascript hang during > test execution is intermittent issue. > > Also, removed IconTest.java and MultiNameTwoPhaseTest.java from > ProblemList.txt. This pull request has now been integrated. Changeset: 918e3397 Author:Alexander Matveev URL: https://git.openjdk.java.net/jdk18/commit/918e3397858c425e9c3b82c9a918b7626603a59c Stats: 4 lines in 2 files changed: 0 ins; 3 del; 1 mod 8278233: [macos] tools/jpackage tests timeout due to /usr/bin/osascript Reviewed-by: herrick, asemenyuk - PR: https://git.openjdk.java.net/jdk18/pull/18
Re: RFR: JDK-8277520: Implement JDK-8 default methods for IdentityHashMap
On Wed, 24 Nov 2021 05:16:40 GMT, liach wrote: > Might need a CSR as now `computeIfAbsent` `computeIfPresent` `compute` > `merge` would throw CME if the functions modified the map itself, and there > are corresponding specification changes. Just curious, what else is recommended for such additions? Is there any recommended tests (though previous additions of `forEach` and `replaceAll` did not add any) - PR: https://git.openjdk.java.net/jdk/pull/6532
Re: RFR: 8278518: String(byte[], int, int, Charset) constructor and String.translateEscapes() miss bounds check elimination
On Tue, 14 Dec 2021 19:12:29 GMT, Mai Đặng Quân Anh wrote: > The problem, at first glance, seems to be that our compiled code is trying to > compute this mysterious number @merykitty how do you view it? 樂 - PR: https://git.openjdk.java.net/jdk/pull/6812
Integrated: 8278028: [test-library] Warnings cleanup of the test library
On Wed, 1 Dec 2021 14:47:54 GMT, Roger Riggs wrote:
> Compilation warnings of the test library introduce noise in test output and
> should be addressed or suppressed.
> Changes include:
> - SuppressWarnings("deprecation") and SuppressWarnings("removal")
> - Adding type parameters to Raw types
> - Adding a hashCode method where equals was already present
This pull request has now been integrated.
Changeset: 03f647f4
Author:Roger Riggs
URL:
https://git.openjdk.java.net/jdk/commit/03f647f4bb640bf5df1c461eec9860c7ac3eb076
Stats: 28 lines in 14 files changed: 10 ins; 1 del; 17 mod
8278028: [test-library] Warnings cleanup of the test library
Reviewed-by: dfuchs, mchung, naoto, lancea, lmesnik
-
PR: https://git.openjdk.java.net/jdk/pull/6638
Re: [jdk18] RFR: 8278233: [macos] tools/jpackage tests timeout due to /usr/bin/osascript
On Tue, 14 Dec 2021 06:10:51 GMT, Alexander Matveev wrote: > This is regression from JDK-8276837. exec() was passing INFINITE_TIMEOUT > instead of actual value of timeout variable. Execution of osascript was > running without timeout and thus several tests timeout. Osascript hang during > test execution is intermittent issue. > > Also, removed IconTest.java and MultiNameTwoPhaseTest.java from > ProblemList.txt. Marked as reviewed by asemenyuk (Reviewer). - PR: https://git.openjdk.java.net/jdk18/pull/18
Re: RFR: 8278518: String(byte[], int, int, Charset) constructor and String.translateEscapes() miss bounds check elimination
On Mon, 13 Dec 2021 09:39:55 GMT, Сергей Цыпанов wrote:
> Originally this was spotted by by Amir Hadadi in
> https://stackoverflow.com/questions/70272651/missing-bounds-checking-elimination-in-string-constructor
>
> It looks like in the following code in `String(byte[], int, int, Charset)`
>
> while (offset < sl) {
> int b1 = bytes[offset];
> if (b1 >= 0) {
> dst[dp++] = (byte)b1;
> offset++; // <---
> continue;
> }
> if ((b1 == (byte)0xc2 || b1 == (byte)0xc3) &&
> offset + 1 < sl) {
> int b2 = bytes[offset + 1];
> if (!isNotContinuation(b2)) {
> dst[dp++] = (byte)decode2(b1, b2);
> offset += 2;
> continue;
> }
> }
> // anything not a latin1, including the repl
> // we have to go with the utf16
> break;
> }
>
> bounds check elimination is not executed when accessing byte array via
> `bytes[offset].
>
> The reason, I guess, is that offset variable is modified within the loop
> (marked with arrow).
>
> Possible fix for this could be changing:
>
> `while (offset < sl)` ---> `while (offset >= 0 && offset < sl)`
>
> However the best is to invest in C2 optimization to handle all such cases.
>
> The following benchmark demonstrates good improvement:
>
> @State(Scope.Thread)
> @BenchmarkMode(Mode.AverageTime)
> @OutputTimeUnit(TimeUnit.NANOSECONDS)
> public class StringConstructorBenchmark {
> private byte[] array;
> private String str;
>
> @Setup
> public void setup() {
> str = "Quizdeltagerne spiste jordbær med fløde, mens cirkusklovnen.
> Я";//Latin1 ending with Russian
> array = str.getBytes(StandardCharsets.UTF_8);
> }
>
> @Benchmark
> public String newString() {
> return new String(array, 0, array.length, StandardCharsets.UTF_8);
> }
>
> @Benchmark
> public String translateEscapes() {
> return str.translateEscapes();
> }
> }
>
> Results:
>
> //baseline
> Benchmark Mode Cnt Score Error Units
> StringConstructorBenchmark.newString avgt 50 173,092 ± 3,048 ns/op
>
> //patched
> Benchmark Mode Cnt Score Error Units
> StringConstructorBenchmark.newString avgt 50 126,908 ± 2,355 ns/op
>
> The same is observed in String.translateEscapes() for the same String as in
> the benchmark above:
>
> //baseline
> Benchmark Mode Cnt Score Error Units
> StringConstructorBenchmark.translateEscapes avgt 100 53,627 ± 0,850 ns/op
>
> //patched
> Benchmark Mode Cnt Score Error Units
> StringConstructorBenchmark.translateEscapes avgt 100 48,087 ± 1,129 ns/op
>
> Also I've looked into this with `LinuxPerfAsmProfiler`, full output for
> baseline is available here
> https://gist.github.com/stsypanov/d2524f98477d633fb1d4a2510fedeea6 and for
> patched code here
> https://gist.github.com/stsypanov/16c787e4f9fa3dd122522f16331b68b7.
>
> Here's the part of baseline assembly responsible for `while` loop:
>
> 3.62% │││ 0x7fed70eb4c1c: mov%ebx,%ecx
> 2.29% │││ 0x7fed70eb4c1e: mov%edx,%r9d
> 2.22% │││ 0x7fed70eb4c21: mov(%rsp),%r8
>;*iload_2 {reexecute=0 rethrow=0 return_oop=0}
> │││
>; - java.lang.String::init@107 (line 537)
> 2.32% ↘││ 0x7fed70eb4c25: cmp%r13d,%ecx
>││ 0x7fed70eb4c28: jge
> 0x7fed70eb5388 ;*if_icmpge {reexecute=0 rethrow=0 return_oop=0}
>││
>; - java.lang.String::init@110 (line 537)
> 3.05%││ 0x7fed70eb4c2e: cmp0x8(%rsp),%ecx
>││ 0x7fed70eb4c32: jae
> 0x7fed70eb5319
> 2.38%││ 0x7fed70eb4c38: mov%r8,(%rsp)
> 2.64%││ 0x7fed70eb4c3c: movslq %ecx,%r8
> 2.46%││ 0x7fed70eb4c3f: mov%rax,%rbx
> 3.44%││ 0x7fed70eb4c42: sub%r8,%rbx
> 2.62%││ 0x7fed70eb4c45: add$0x1,%rbx
> 2.64%││ 0x7fed70eb4c49: and
> $0xfffe,%rbx
> 2.30%││ 0x7fed70eb4c4d: mov%ebx,%r8d
> 3.08%││ 0x7fed70eb4c50: add%ecx,%r8d
> 2.55%││ 0x7fed70eb4c53: movslq %r8d,%r8
> 2.45%││ 0x7fed70eb4c56: add
> $0xfffe,%r8
> 2.13%││ 0x7fed70eb4c5a: cmp(%rsp),%r8
>││ 0x7fed70eb4c5e: jae
> 0x7fed70eb5319
> 3.36%││ 0x7fed70eb4c64: mov%ecx,%edi
>;*aload_1
Re: RFR: 8278587: StringTokenizer(String, String, boolean) documentation bug
On Tue, 14 Dec 2021 18:25:45 GMT, Naoto Sato wrote: > This is a doc fix to `StringTokenizer`, where the original spec does not > account for the delimiter's length in the case of a supplementary character. > Corresponding CSR has been drafted: > https://bugs.openjdk.java.net/browse/JDK-8278814 Marked as reviewed by lancea (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6836
Re: RFR: 8278587: StringTokenizer(String, String, boolean) documentation bug
On Tue, 14 Dec 2021 18:25:45 GMT, Naoto Sato wrote: > This is a doc fix to `StringTokenizer`, where the original spec does not > account for the delimiter's length in the case of a supplementary character. > Corresponding CSR has been drafted: > https://bugs.openjdk.java.net/browse/JDK-8278814 Marked as reviewed by joehw (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/6836
Re: RFR: 8278587: StringTokenizer(String, String, boolean) documentation bug
On Tue, 14 Dec 2021 18:25:45 GMT, Naoto Sato wrote: > This is a doc fix to `StringTokenizer`, where the original spec does not > account for the delimiter's length in the case of a supplementary character. > Corresponding CSR has been drafted: > https://bugs.openjdk.java.net/browse/JDK-8278814 Corresponding CSR also Reviewed. - Marked as reviewed by iris (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/6836
RFR: 8278587: StringTokenizer(String, String, boolean) documentation bug
This is a doc fix to `StringTokenizer`, where the original spec does not account for the delimiter's length in the case of a supplementary character. Corresponding CSR has been drafted: https://bugs.openjdk.java.net/browse/JDK-8278814 - Commit messages: - 8278587: StringTokenizer(String, String, boolean) documentation bug Changes: https://git.openjdk.java.net/jdk/pull/6836/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6836=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8278587 Stats: 5 lines in 1 file changed: 1 ins; 0 del; 4 mod Patch: https://git.openjdk.java.net/jdk/pull/6836.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6836/head:pull/6836 PR: https://git.openjdk.java.net/jdk/pull/6836
Re: RFR: JDK-8277795: ldap connection timeout not honoured under contention [v3]
> This fix attemps to resolve an issue where threads can stack up on each other > while waiting to get a connection from the ldap pool to an unreachable > server. It does this by having each thread start a countdown prior to holding > the pools' lock. (which has been changed to a ReentrantLock) Once the lock > has been grabbed, the timeout is adjusted to take the waiting time into > account and the process of getting a connection from the pool or creating a > new one commences. > > Note: this fix also changes the meaning of the connection pools initSize > somewhat. In a situation where we have a large initSize and a small timeout > the first thread could actually exhaust the timeout before creating all of > its initial connections. Instead this fix simply creates a single connection > per pool-connection-request. It continues to do so for subsequent requests > regardless of whether an existing unused connection is available in the pool > until initSize is exhausted. As such it may require a CSR. Rob McKenna has updated the pull request incrementally with one additional commit since the last revision: Allow the test to pass on MacOSX - Changes: - all: https://git.openjdk.java.net/jdk/pull/6568/files - new: https://git.openjdk.java.net/jdk/pull/6568/files/1b679497..7c277622 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk=6568=02 - incr: https://webrevs.openjdk.java.net/?repo=jdk=6568=01-02 Stats: 22 lines in 1 file changed: 10 ins; 1 del; 11 mod Patch: https://git.openjdk.java.net/jdk/pull/6568.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6568/head:pull/6568 PR: https://git.openjdk.java.net/jdk/pull/6568
Re: RFR: 8278028: [test-library] Warnings cleanup of the test library [v2]
On Tue, 14 Dec 2021 03:14:10 GMT, Leonid Mesnik wrote: > Is test/micro/org/openjdk/bench/java/io/SerialFilterOverhead.java added > accidentally or by purpose? By mistake, thanks for noticing. - PR: https://git.openjdk.java.net/jdk/pull/6638
Re: RFR: 8278028: [test-library] Warnings cleanup of the test library [v3]
On Tue, 14 Dec 2021 15:20:31 GMT, Roger Riggs wrote:
>> Compilation warnings of the test library introduce noise in test output and
>> should be addressed or suppressed.
>> Changes include:
>> - SuppressWarnings("deprecation") and SuppressWarnings("removal")
>> - Adding type parameters to Raw types
>> - Adding a hashCode method where equals was already present
>
> Roger Riggs has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Remove a test/micro file added by mistake.
Marked as reviewed by lmesnik (Reviewer).
-
PR: https://git.openjdk.java.net/jdk/pull/6638
Re: RFR: 8278028: [test-library] Warnings cleanup of the test library [v3]
> Compilation warnings of the test library introduce noise in test output and
> should be addressed or suppressed.
> Changes include:
> - SuppressWarnings("deprecation") and SuppressWarnings("removal")
> - Adding type parameters to Raw types
> - Adding a hashCode method where equals was already present
Roger Riggs has updated the pull request incrementally with one additional
commit since the last revision:
Remove a test/micro file added by mistake.
-
Changes:
- all: https://git.openjdk.java.net/jdk/pull/6638/files
- new: https://git.openjdk.java.net/jdk/pull/6638/files/b1bbdfc7..895acede
Webrevs:
- full: https://webrevs.openjdk.java.net/?repo=jdk=6638=02
- incr: https://webrevs.openjdk.java.net/?repo=jdk=6638=01-02
Stats: 129 lines in 1 file changed: 0 ins; 129 del; 0 mod
Patch: https://git.openjdk.java.net/jdk/pull/6638.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/6638/head:pull/6638
PR: https://git.openjdk.java.net/jdk/pull/6638
Re: RFR: 8278518: String(byte[], int, int, Charset) constructor and String.translateEscapes() miss bounds check elimination
On Tue, 14 Dec 2021 13:20:46 GMT, Alan Bateman wrote: >>> Originally this was spotted by by Amir Hadadi in >>> https://stackoverflow.com/questions/70272651/missing-bounds-checking-elimination-in-string-constructor >> >> Before anyone looks at this, can you confirm that the patch does not include >> any code or tests/benchmarks that were taken from SO? > >> @AlanBateman the benchmark is mine along with the changes for >> `translateEscapes` and `newStringUTF8NoRepl`, the change for constructor is >> from SO. > > I don't know how we can progress this PR if the patch includes code copied > from SO. Maybe the PR should be closed, the JBS issue unassigned, and leave > it to someone else to start again? Maybe you could get Amit to sign the OCA > and you co-contribute/author the PR? I can't look at the patch so I don't > know how significant the changes, maybe there are other options. @AlanBateman @stsypanov I have no problem signing the OCA. However like @stsypanov mentioned, I don't think this is the right approach. Adding explicit check that the offset is non negative makes no semantic difference, it just helps the optimizer to figure out that no bounds checking is needed. A better approach is to improve the optimizer so that it can apply bounds checking elimination in this case (and hopefully in many other cases). - PR: https://git.openjdk.java.net/jdk/pull/6812
RFR: 8278642: Refactor java.util.Formatter
A few refactorings to how `java.util.Formatter` sets up `FormatString`s, aligning the implementation with changes explored by the TemplatedStrings JEP and ever so slightly improving performance: - turn `Flags` into an `int` (fewer allocations in the complex case) - remove some superfluous varargs: `checkBadFlags(Flags.PARENTHESES, ...` -> `checkBadFlags(Flags.Parentheses | ...` - again less allocations in the complex cases since these varargs arrays were being allocated. Also improves error messages since all bad flags will be listed in the exception message. - make `FormatSpecifier` and `FixedString` static, reducing size of these objects slightly. Baseline: Benchmark Mode Cnt Score Error Units StringFormat.complexFormat avgt 25 8977.043 ± 246.810 ns/op StringFormat.complexFormat:·gc.alloc.rate.norm avgt 25 2144.170 ± 0.012B/op StringFormat.stringFormat avgt 25 252.109 ± 2.732 ns/op StringFormat.stringFormat:·gc.alloc.rate.norm avgt 25 256.019 ± 0.001B/op StringFormat.stringIntFormatavgt 25 576.423 ± 4.596 ns/op StringFormat.stringIntFormat:·gc.alloc.rate.normavgt 25 432.034 ± 0.002B/op StringFormat.widthStringFormat avgt 25 999.835 ± 20.127 ns/op StringFormat.widthStringFormat:·gc.alloc.rate.norm avgt 25 525.509 ± 14.742B/op StringFormat.widthStringIntFormat avgt 25 1332.163 ± 30.901 ns/op StringFormat.widthStringIntFormat:·gc.alloc.rate.norm avgt 25 720.715 ± 8.798B/op Patch: Benchmark Mode Cnt ScoreError Units StringFormat.complexFormat avgt 25 8840.089 ± 51.222 ns/op StringFormat.complexFormat:·gc.alloc.rate.norm avgt 25 1736.151 ± 0.009B/op StringFormat.stringFormat avgt 25 247.310 ± 2.091 ns/op StringFormat.stringFormat:·gc.alloc.rate.norm avgt 25 248.018 ± 0.001B/op StringFormat.stringIntFormatavgt 25 565.487 ± 6.572 ns/op StringFormat.stringIntFormat:·gc.alloc.rate.normavgt 25 408.032 ± 0.002B StringFormat.widthStringFormat avgt 25 970.015 ± 32.915 ns/op StringFormat.widthStringFormat:·gc.alloc.rate.norm avgt 25 449.991 ± 25.716B/op StringFormat.widthStringIntFormat avgt 25 1284.572 ± 28.829 ns/op StringFormat.widthStringIntFormat:·gc.alloc.rate.norm avgt 25 636.872 ± 7.331B/op - Commit messages: - Fix call with null Formatter - 8278642: Refactor Formatter Changes: https://git.openjdk.java.net/jdk/pull/6821/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6821=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8278642 Stats: 333 lines in 1 file changed: 0 ins; 39 del; 294 mod Patch: https://git.openjdk.java.net/jdk/pull/6821.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/6821/head:pull/6821 PR: https://git.openjdk.java.net/jdk/pull/6821
Re: [jdk18] RFR: 8278233: [macos] tools/jpackage tests timeout due to /usr/bin/osascript
On Tue, 14 Dec 2021 06:10:51 GMT, Alexander Matveev wrote: > This is regression from JDK-8276837. exec() was passing INFINITE_TIMEOUT > instead of actual value of timeout variable. Execution of osascript was > running without timeout and thus several tests timeout. Osascript hang during > test execution is intermittent issue. > > Also, removed IconTest.java and MultiNameTwoPhaseTest.java from > ProblemList.txt. Marked as reviewed by herrick (Reviewer). - PR: https://git.openjdk.java.net/jdk18/pull/18
Re: RFR: 8276766: Enable jar and jmod to produce deterministic timestamped content [v20]
On Fri, 10 Dec 2021 19:40:59 GMT, Lance Andersen wrote: >> Looks like the CSR has been approved. I have a mach5 run that should >> hopefully finish sooner rather than later and if that remains happy, I will >> approve the PR > >> @LanceAndersen let me know if mach5 looks good please, then I think we're >> good to go.. > > As soon as the Mach5 finishes, I will let you know @LanceAndersen fyi.i've raised a docs enhancement for the closed repo "man" pages to be updated for this: https://bugs.openjdk.java.net/browse/JDK-8278764 - PR: https://git.openjdk.java.net/jdk/pull/6481
Re: RFR: 8278518: String(byte[], int, int, Charset) constructor and String.translateEscapes() miss bounds check elimination
On Mon, 13 Dec 2021 09:55:36 GMT, Alan Bateman wrote:
>> Originally this was spotted by by Amir Hadadi in
>> https://stackoverflow.com/questions/70272651/missing-bounds-checking-elimination-in-string-constructor
>>
>> It looks like in the following code in `String(byte[], int, int, Charset)`
>>
>> while (offset < sl) {
>> int b1 = bytes[offset];
>> if (b1 >= 0) {
>> dst[dp++] = (byte)b1;
>> offset++; // <---
>> continue;
>> }
>> if ((b1 == (byte)0xc2 || b1 == (byte)0xc3) &&
>> offset + 1 < sl) {
>> int b2 = bytes[offset + 1];
>> if (!isNotContinuation(b2)) {
>> dst[dp++] = (byte)decode2(b1, b2);
>> offset += 2;
>> continue;
>> }
>> }
>> // anything not a latin1, including the repl
>> // we have to go with the utf16
>> break;
>> }
>>
>> bounds check elimination is not executed when accessing byte array via
>> `bytes[offset].
>>
>> The reason, I guess, is that offset variable is modified within the loop
>> (marked with arrow).
>>
>> Possible fix for this could be changing:
>>
>> `while (offset < sl)` ---> `while (offset >= 0 && offset < sl)`
>>
>> However the best is to invest in C2 optimization to handle all such cases.
>>
>> The following benchmark demonstrates good improvement:
>>
>> @State(Scope.Thread)
>> @BenchmarkMode(Mode.AverageTime)
>> @OutputTimeUnit(TimeUnit.NANOSECONDS)
>> public class StringConstructorBenchmark {
>> private byte[] array;
>> private String str;
>>
>> @Setup
>> public void setup() {
>> str = "Quizdeltagerne spiste jordbær med fløde, mens cirkusklovnen.
>> Я";//Latin1 ending with Russian
>> array = str.getBytes(StandardCharsets.UTF_8);
>> }
>>
>> @Benchmark
>> public String newString() {
>> return new String(array, 0, array.length, StandardCharsets.UTF_8);
>> }
>>
>> @Benchmark
>> public String translateEscapes() {
>> return str.translateEscapes();
>> }
>> }
>>
>> Results:
>>
>> //baseline
>> Benchmark Mode Cnt Score Error Units
>> StringConstructorBenchmark.newString avgt 50 173,092 ± 3,048 ns/op
>>
>> //patched
>> Benchmark Mode Cnt Score Error Units
>> StringConstructorBenchmark.newString avgt 50 126,908 ± 2,355 ns/op
>>
>> The same is observed in String.translateEscapes() for the same String as in
>> the benchmark above:
>>
>> //baseline
>> Benchmark Mode Cnt Score Error Units
>> StringConstructorBenchmark.translateEscapes avgt 100 53,627 ± 0,850 ns/op
>>
>> //patched
>> Benchmark Mode Cnt Score Error Units
>> StringConstructorBenchmark.translateEscapes avgt 100 48,087 ± 1,129 ns/op
>>
>> Also I've looked into this with `LinuxPerfAsmProfiler`, full output for
>> baseline is available here
>> https://gist.github.com/stsypanov/d2524f98477d633fb1d4a2510fedeea6 and for
>> patched code here
>> https://gist.github.com/stsypanov/16c787e4f9fa3dd122522f16331b68b7.
>>
>> Here's the part of baseline assembly responsible for `while` loop:
>>
>> 3.62% │││ 0x7fed70eb4c1c: mov%ebx,%ecx
>> 2.29% │││ 0x7fed70eb4c1e: mov%edx,%r9d
>> 2.22% │││ 0x7fed70eb4c21: mov(%rsp),%r8
>> ;*iload_2 {reexecute=0 rethrow=0 return_oop=0}
>> │││
>> ; - java.lang.String::init@107 (line 537)
>> 2.32% ↘││ 0x7fed70eb4c25: cmp%r13d,%ecx
>>││ 0x7fed70eb4c28: jge
>> 0x7fed70eb5388 ;*if_icmpge {reexecute=0 rethrow=0 return_oop=0}
>>││
>> ; - java.lang.String::init@110 (line 537)
>> 3.05%││ 0x7fed70eb4c2e: cmp
>> 0x8(%rsp),%ecx
>>││ 0x7fed70eb4c32: jae
>> 0x7fed70eb5319
>> 2.38%││ 0x7fed70eb4c38: mov%r8,(%rsp)
>> 2.64%││ 0x7fed70eb4c3c: movslq %ecx,%r8
>> 2.46%││ 0x7fed70eb4c3f: mov%rax,%rbx
>> 3.44%││ 0x7fed70eb4c42: sub%r8,%rbx
>> 2.62%││ 0x7fed70eb4c45: add$0x1,%rbx
>> 2.64%││ 0x7fed70eb4c49: and
>> $0xfffe,%rbx
>> 2.30%││ 0x7fed70eb4c4d: mov%ebx,%r8d
>> 3.08%││ 0x7fed70eb4c50: add%ecx,%r8d
>> 2.55%││ 0x7fed70eb4c53: movslq %r8d,%r8
>> 2.45%││ 0x7fed70eb4c56: add
>> $0xfffe,%r8
>> 2.13%││ 0x7fed70eb4c5a: cmp(%rsp),%r8
>>││ 0x7fed70eb4c5e: jae
>>
Re: [jdk18] RFR: 8278607: Misc issues in foreign API javadoc [v4]
> This patch fixes a number of issues and typos in the foreign API javadoc > following another internal round of reviews. Maurizio Cimadamore has updated the pull request incrementally with one additional commit since the last revision: Fix typo as per review comment. Improve javadoc for VaList. - Changes: - all: https://git.openjdk.java.net/jdk18/pull/17/files - new: https://git.openjdk.java.net/jdk18/pull/17/files/603df4ec..5f1ee0ed Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk18=17=03 - incr: https://webrevs.openjdk.java.net/?repo=jdk18=17=02-03 Stats: 22 lines in 2 files changed: 13 ins; 0 del; 9 mod Patch: https://git.openjdk.java.net/jdk18/pull/17.diff Fetch: git fetch https://git.openjdk.java.net/jdk18 pull/17/head:pull/17 PR: https://git.openjdk.java.net/jdk18/pull/17
Re: [jdk18] RFR: 8278607: Misc issues in foreign API javadoc [v3]
On Tue, 14 Dec 2021 02:07:00 GMT, Maurizio Cimadamore wrote: >> This patch fixes a number of issues and typos in the foreign API javadoc >> following another internal round of reviews. > > Maurizio Cimadamore has updated the pull request incrementally with one > additional commit since the last revision: > > Clarify sentence in package-info LGTM - Marked as reviewed by sundar (Reviewer). PR: https://git.openjdk.java.net/jdk18/pull/17
