Re: RFR: 8329597: C2: Intrinsify Reference.clear [v7]

2024-10-03 Thread Aleksey Shipilev
gt; intrinsics. C2 Access also need a support for `AS_NO_KEEPALIVE` for stores. > > Additional testing: > - [ ] Linux x86_64 server fastdebug, `all` > - [ ] Linux AArch64 server fastdebug, `all` Aleksey Shipilev has updated the pull request with a new target base due to a merge or

Re: RFR: 8329597: C2: Intrinsify Reference.clear [v6]

2024-10-03 Thread Aleksey Shipilev
On Wed, 2 Oct 2024 10:47:09 GMT, Tobias Hartmann wrote: >> Aleksey Shipilev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Also dispatch to slow-path on other arches > > test/micro/org/openjdk/bench/jav

Re: RFR: 8329597: C2: Intrinsify Reference.clear [v6]

2024-10-03 Thread Aleksey Shipilev
On Thu, 3 Oct 2024 12:14:08 GMT, Erik Österlund wrote: >> Aleksey Shipilev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Also dispatch to slow-path on other arches > > src/hotspot/share/opto/library_

Re: RFR: 8341135: Incorrect format string after JDK-8339475

2024-10-01 Thread Aleksey Shipilev
On Tue, 1 Oct 2024 07:22:46 GMT, Matthias Baesken wrote: > This fixes a format string issue. > > In the error report it was reported : 'The compiler should be able to catch > this if JLI_ReportErrorMessageSys was declared with something like > __attribute__ ((format (printf, 1, 2))).' > Should

Re: RFR: 8341135: Incorrect format string after JDK-8339475 [v2]

2024-10-01 Thread Aleksey Shipilev
On Tue, 1 Oct 2024 13:15:51 GMT, Matthias Baesken wrote: >> This fixes a format string issue. >> >> In the error report it was reported : 'The compiler should be able to catch >> this if JLI_ReportErrorMessageSys was declared with something like >> __attribute__ ((format (printf, 1, 2))).' >>

Re: RFR: 8329597: C2: Intrinsify Reference.clear [v6]

2024-09-30 Thread Aleksey Shipilev
gt; intrinsics. C2 Access also need a support for `AS_NO_KEEPALIVE` for stores. > > Additional testing: > - [ ] Linux x86_64 server fastdebug, `all` > - [ ] Linux AArch64 server fastdebug, `all` Aleksey Shipilev has updated the pull request incrementally with one additional commit si

Re: RFR: 8329597: C2: Intrinsify Reference.clear [v3]

2024-09-30 Thread Aleksey Shipilev
On Fri, 27 Sep 2024 23:51:13 GMT, Kim Barrett wrote: >> Aleksey Shipilev has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Amend the test case for guaranteing it works under different compilation >> r

Re: RFR: 8329597: C2: Intrinsify Reference.clear [v5]

2024-09-30 Thread Aleksey Shipilev
gt; intrinsics. C2 Access also need a support for `AS_NO_KEEPALIVE` for stores. > > Additional testing: > - [ ] Linux x86_64 server fastdebug, `all` > - [ ] Linux AArch64 server fastdebug, `all` Aleksey Shipilev has updated the pull request incrementally with two additional commits sin

Re: RFR: 8329597: C2: Intrinsify Reference.clear [v3]

2024-09-30 Thread Aleksey Shipilev
On Mon, 30 Sep 2024 15:08:53 GMT, Erik Österlund wrote: > I think we need a new > ZBarrierSetRuntime::no_keepalive_store_barrier_on_oop_field_without_healing(oop* > p) and to make that the selected slow path function when ZBarrierNoKeepalive > is set on a StorePNode. Its implementation would c

Re: RFR: 8329597: C2: Intrinsify Reference.clear [v4]

2024-09-30 Thread Aleksey Shipilev
gt; intrinsics. C2 Access also need a support for `AS_NO_KEEPALIVE` for stores. > > Additional testing: > - [x] Linux x86_64 server fastdebug, `all` > - [x] Linux AArch64 server fastdebug, `all` Aleksey Shipilev has updated the pull request with a new target base due to a merge or a r

Re: RFR: 8329597: C2: Intrinsify Reference.clear [v3]

2024-09-27 Thread Aleksey Shipilev
On Fri, 27 Sep 2024 17:44:38 GMT, Vladimir Kozlov wrote: > Is ZGC affected by this? I see only G1 and Shenandoah changes. Good question. ZGC expands the GC barriers late. This is why the IR test configuration that tests ZGC shows the same result as with other collectors: no additional fluff i

Re: RFR: 8329597: C2: Intrinsify Reference.clear [v3]

2024-09-27 Thread Aleksey Shipilev
On Fri, 19 Jul 2024 15:52:14 GMT, Aleksey Shipilev wrote: >> [JDK-8240696](https://bugs.openjdk.org/browse/JDK-8240696) added the native >> method for `Reference.clear`. The original patch skipped intrinsification of >> this method, because we thought `Reference.clear` is no

Re: RFR: 8340838: Clean up MutableCallSite to use explicit release fence instead of AtomicInteger

2024-09-25 Thread Aleksey Shipilev
On Tue, 24 Sep 2024 20:21:48 GMT, Chen Liang wrote: > This implementation code was written in JDK 7, before storeFence was > available in JDK 8. We should update this legacy code to make it clear. I have been harboring doubts `syncAll` even works (`FIXME: NYI` is fun) with a lone barrier for a

Re: RFR: 8340812: LambdaForm customization via MethodHandle::updateForm is not thread safe

2024-09-25 Thread Aleksey Shipilev
On Tue, 24 Sep 2024 14:18:58 GMT, Tobias Hartmann wrote: > When investigating an intermittent NPE with an Oracle internal test on > AArch64, I noticed that the NPE is actually a SIGSEGV in the code emitted by > `MethodHandles::jump_to_lambda_form` when trying to load the > `MemberName::method`

Re: RFR: 8340812: LambdaForm customization via MethodHandle::updateForm is not thread safe

2024-09-24 Thread Aleksey Shipilev
On Tue, 24 Sep 2024 14:18:58 GMT, Tobias Hartmann wrote: > When investigating an intermittent NPE with an Oracle internal test on > AArch64, I noticed that the NPE is actually a SIGSEGV in the code emitted by > `MethodHandles::jump_to_lambda_form` when trying to load the > `MemberName::method`

Re: RFR: 8340717: Remove unused function declarations from java.c/java.h of the launcher

2024-09-24 Thread Aleksey Shipilev
On Tue, 24 Sep 2024 04:33:14 GMT, Jaikiran Pai wrote: > Can I please get a review of this cleanup to the launcher code to remove > declarations of unused functions? > > The `ValidateModules` function appears to be a left-over from > https://bugs.openjdk.org/browse/JDK-8194937. > > The `SetApp

Re: RFR: 8329597: C2: Intrinsify Reference.clear [v3]

2024-08-19 Thread Aleksey Shipilev
On Fri, 19 Jul 2024 15:52:14 GMT, Aleksey Shipilev wrote: >> [JDK-8240696](https://bugs.openjdk.org/browse/JDK-8240696) added the native >> method for `Reference.clear`. The original patch skipped intrinsification of >> this method, because we thought `Reference.clear` is no

Re: RFR: 8332842: Optimize empty CopyOnWriteArrayList allocations [v4]

2024-08-14 Thread Aleksey Shipilev
On Thu, 20 Jun 2024 19:17:25 GMT, jengebr wrote: >> Improve `java/util/concurrent/CopyOnWriteArrayList` by eliminating needless >> cloning of Object[0] instances. This cloning is intended to prevent callers >> from changing array contents, but many `CopyOnWriteArrayList`s are allocated >> to s

Re: RFR: 8336926: jdk/internal/util/ReferencedKeyTest.java can fail with ConcurrentModificationException

2024-08-08 Thread Aleksey Shipilev
On Wed, 7 Aug 2024 19:26:59 GMT, Roger Riggs wrote: > The original test fails intermittently, the reproducer failed consistently. > With the fix, the failure was not observed in the test or reproducer. > > In jdk.internal.util.ReferencedKeyMap.entrySet() and toString() methods, > avoid removing

Re: RFR: 8336926: jdk/internal/util/ReferencedKeyTest.java can fail with ConcurrentModificationException

2024-08-08 Thread Aleksey Shipilev
On Wed, 7 Aug 2024 19:26:59 GMT, Roger Riggs wrote: > The original test fails intermittently, the reproducer failed consistently. > With the fix, the failure was not observed in the test or reproducer. > > In jdk.internal.util.ReferencedKeyMap.entrySet() and toString() methods, > avoid removing

Re: RFR: 8324260: java/foreign/TestStubAllocFailure.java run timeout with -Xcomp

2024-07-31 Thread Aleksey Shipilev
On Wed, 31 Jul 2024 13:40:31 GMT, Jorn Vernee wrote: > This test spawns 2 forked processes, which then try to trigger a code cache > OOM in FFM hotspot code. Even without `-Xcomp`, the test is already pretty > slow, which increases dramatically with `-Xcomp`, since startup Java code has > to b

Re: RFR: 8329597: C2: Intrinsify Reference.clear [v3]

2024-07-19 Thread Aleksey Shipilev
gt; intrinsics. C2 Access also need a support for `AS_NO_KEEPALIVE` for stores. > > Additional testing: > - [x] Linux x86_64 server fastdebug, `all` > - [x] Linux AArch64 server fastdebug, `all` Aleksey Shipilev has updated the pull request incrementally with one additional commit

Re: RFR: 8329597: C2: Intrinsify Reference.clear

2024-07-17 Thread Aleksey Shipilev
On Thu, 11 Jul 2024 15:28:37 GMT, Aleksey Shipilev wrote: > [JDK-8240696](https://bugs.openjdk.org/browse/JDK-8240696) added the native > method for `Reference.clear`. The original patch skipped intrinsification of > this method, because we thought `Reference.clear` is not on a pe

Re: RFR: 8329597: C2: Intrinsify Reference.clear

2024-07-17 Thread Aleksey Shipilev
On Fri, 12 Jul 2024 13:19:31 GMT, Aleksey Shipilev wrote: >>> > The reason we did not do this before is that this is not a strong >>> > reference store. Strong reference stores with a SATB collector will keep >>> > the referent alive, which is typical

Re: RFR: 8329597: C2: Intrinsify Reference.clear [v2]

2024-07-17 Thread Aleksey Shipilev
gt; intrinsics. C2 Access also need a support for `AS_NO_KEEPALIVE` for stores. > > Additional testing: > - [x] Linux x86_64 server fastdebug, `all` > - [ ] Linux AArch64 server fastdebug, `all` Aleksey Shipilev has updated the pull request with a new target base due to a merge or a

Re: RFR: 8335922: Incorrect @Stable usage of LambdaForm$Name.index

2024-07-17 Thread Aleksey Shipilev
On Mon, 15 Jul 2024 13:39:02 GMT, Aleksey Shipilev wrote: >> Indeed, for some reason I thought the range of short is -256 to 255 instead >> of -65536 to 65535 > >> Indeed, for some reason I thought the range of short is -256 to 255 instead >> of -65536 to 65535 >

Re: RFR: 8335922: Incorrect @Stable usage of LambdaForm$Name.index [v3]

2024-07-15 Thread Aleksey Shipilev
On Mon, 15 Jul 2024 15:57:30 GMT, Chen Liang wrote: >> The `@Stable` on the `index` field is incorrect, as stable only avoids >> inlining `0`. Solution is to use bit flip on the actual index (and rename >> the field to `flippedIndex`), so we use -1 to -256 (mapping to 0 to 255) and >> 0 the de

Re: RFR: 8335922: Incorrect @Stable usage of LambdaForm$Name.index [v2]

2024-07-15 Thread Aleksey Shipilev
On Mon, 15 Jul 2024 15:47:50 GMT, Chen Liang wrote: >> src/java.base/share/classes/java/lang/invoke/LambdaFormEditor.java line 778: >> >>> 776: var newParameters = new TreeMap(new >>> Comparator<>() { >>> 777: public int compare(Name n1, Name n2) { >>> 778: r

Re: RFR: 8335922: Incorrect @Stable usage of LambdaForm$Name.index [v2]

2024-07-15 Thread Aleksey Shipilev
On Mon, 15 Jul 2024 14:09:22 GMT, Chen Liang wrote: >> The `@Stable` on the `index` field is incorrect, as stable only avoids >> inlining `0`. Solution is to use bit flip on the actual index (and rename >> the field to `flippedIndex`), so we use -1 to -256 (mapping to 0 to 255) and >> 0 the de

Re: RFR: 8332842: Optimize empty CopyOnWriteArrayList allocations [v4]

2024-07-15 Thread Aleksey Shipilev
On Thu, 20 Jun 2024 19:17:25 GMT, jengebr wrote: >> Improve `java/util/concurrent/CopyOnWriteArrayList` by eliminating needless >> cloning of Object[0] instances. This cloning is intended to prevent callers >> from changing array contents, but many `CopyOnWriteArrayList`s are allocated >> to s

Re: RFR: 8335922: Incorrect @Stable usage of LambdaForm$Name.index

2024-07-15 Thread Aleksey Shipilev
On Mon, 15 Jul 2024 12:28:51 GMT, Chen Liang wrote: > Indeed, for some reason I thought the range of short is -256 to 255 instead > of -65536 to 65535 Yeah, I thought something like this was going on; it would be a smart way to leverage that negative side in two-complement form is one value la

Re: RFR: 8335922: Incorrect @Stable usage of LambdaForm$Name.index

2024-07-15 Thread Aleksey Shipilev
On Mon, 15 Jul 2024 01:45:57 GMT, Chen Liang wrote: > The `@Stable` on the `index` field is incorrect, as stable only avoids > inlining `0`. Solution is to use bit flip on the actual index (and rename the > field to `flippedIndex`), so we use -1 to -256 (mapping to 0 to 255) and 0 > the defaul

Re: RFR: 8329597: C2: Intrinsify Reference.clear

2024-07-12 Thread Aleksey Shipilev
On Fri, 12 Jul 2024 11:57:56 GMT, Erik Österlund wrote: > The runtime use of the Access API knows how to resolve an unknown oop ref > strength using AccessBarrierSupport::resolve_unknown_oop_ref_strength. > However, we do not have support for that in the C2 backend. In fact, it does > not unde

Re: RFR: 8329597: C2: Intrinsify Reference.clear

2024-07-12 Thread Aleksey Shipilev
On Fri, 12 Jul 2024 10:16:13 GMT, Erik Österlund wrote: > The reason we did not do this before is that this is not a strong reference > store. Strong reference stores with a SATB collector will keep the referent > alive, which is typically the exact opposite of what a user wants when they > cl

Re: RFR: 8329597: C2: Intrinsify Reference.clear

2024-07-11 Thread Aleksey Shipilev
On Thu, 11 Jul 2024 15:28:37 GMT, Aleksey Shipilev wrote: > [JDK-8240696](https://bugs.openjdk.org/browse/JDK-8240696) added the native > method for `Reference.clear`. The original patch skipped intrinsification of > this method, because we thought `Reference.clear` is not on a pe

RFR: 8329597: C2: Intrinsify Reference.clear

2024-07-11 Thread Aleksey Shipilev
[JDK-8240696](https://bugs.openjdk.org/browse/JDK-8240696) added the native method for `Reference.clear`. The original patch skipped intrinsification of this method, because we thought `Reference.clear` is not on a performance sensitive path. However, it shows up prominently on simple benchmarks

Re: RFR: 8332249: Micro-optimize Method.hashCode [v2]

2024-07-11 Thread Aleksey Shipilev
On Tue, 11 Jun 2024 08:58:34 GMT, Aleksey Shipilev wrote: >> Sean Gwizdak has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request conta

Integrated: 8335274: SwitchBootstraps.ResolvedEnumLabels.resolvedEnum should be final

2024-06-28 Thread Aleksey Shipilev
On Thu, 27 Jun 2024 19:30:34 GMT, Aleksey Shipilev wrote: > I was auditing the current uses of `@Stable` before relaxing its barriers > ([JDK-8333791](https://bugs.openjdk.org/browse/JDK-8333791)), and this is an > easy spot. > > `resolvedEnum` is not `final`. So technically

Re: RFR: 8335274: SwitchBootstraps.ResolvedEnumLabels.resolvedEnum should be final

2024-06-28 Thread Aleksey Shipilev
On Thu, 27 Jun 2024 19:30:34 GMT, Aleksey Shipilev wrote: > I was auditing the current uses of `@Stable` before relaxing its barriers > ([JDK-8333791](https://bugs.openjdk.org/browse/JDK-8333791)), and this is an > easy spot. > > `resolvedEnum` is not `final`. So technically

RFR: 8335274: SwitchBootstraps.ResolvedEnumLabels.resolvedEnum should be final

2024-06-27 Thread Aleksey Shipilev
I was auditing the current uses of `@Stable` before relaxing its barriers ([JDK-8333791](https://bugs.openjdk.org/browse/JDK-8333791)), and this is an easy spot. `resolvedEnum` is not `final`. So technically publishing the object via data race can show `resolvedEnum` as `null`, which would bre

Re: RFR: 8332842: Optimize empty CopyOnWriteArrayList allocations [v3]

2024-06-19 Thread Aleksey Shipilev
On Thu, 6 Jun 2024 12:46:36 GMT, jengebr wrote: >> Improve `java/util/concurrent/CopyOnWriteArrayList` by eliminating needless >> cloning of Object[0] instances. This cloning is intended to prevent callers >> from changing array contents, but many `CopyOnWriteArrayList`s are allocated >> to si

Re: RFR: 8332249: Micro-optimize Method.hashCode [v2]

2024-06-11 Thread Aleksey Shipilev
On Mon, 3 Jun 2024 18:00:35 GMT, Sean Gwizdak wrote: >> Improve the speed of Method.hashCode by caching the hashcode on first use. >> I've seen an application where Method.hashCode is a hot path, and this is a >> fairly simple speedup. The memory overhead is low. >> >> This addresses issue >

Re: RFR: 8325984: 4 jcstress tests are failing in Tier6 4 times each

2024-06-07 Thread Aleksey Shipilev
On Wed, 5 Jun 2024 19:21:56 GMT, Jorn Vernee wrote: > These 4 tests were failing due to an incompatibility with jcstress. They were > problemlisted in past (https://bugs.openjdk.org/browse/JDK-8326062). > > Now that jcstress has been updated > (https://github.com/openjdk/jdk/pull/19332) with t

Re: RFR: 8332842: Optimize empty CopyOnWriteArrayList allocations [v3]

2024-06-06 Thread Aleksey Shipilev
On Thu, 6 Jun 2024 12:46:36 GMT, jengebr wrote: >> Improve `java/util/concurrent/CopyOnWriteArrayList` by eliminating needless >> cloning of Object[0] instances. This cloning is intended to prevent callers >> from changing array contents, but many `CopyOnWriteArrayList`s are allocated >> to si

Re: RFR: 8332842: Optimize empty CopyOnWriteArrayList allocations [v3]

2024-06-06 Thread Aleksey Shipilev
On Wed, 5 Jun 2024 14:56:26 GMT, jengebr wrote: > clone() performs a shallow copy, so there is currently no Object[] allocated > and therefore nothing to optimize. Yes, I believe so. - PR Review Comment: https://git.openjdk.org/jdk/pull/19527#discussion_r1629666551

Re: RFR: 8325984: 4 jcstress tests are failing in Tier6 4 times each

2024-06-06 Thread Aleksey Shipilev
On Wed, 5 Jun 2024 19:21:56 GMT, Jorn Vernee wrote: > These 4 tests were failing due to an incompatibility with jcstress. They were > problemlisted in past (https://bugs.openjdk.org/browse/JDK-8326062). > > Now that jcstress has been updated > (https://github.com/openjdk/jdk/pull/19332) with t

Re: RFR: 8332842: Optimize empty CopyOnWriteArrayList allocations

2024-06-03 Thread Aleksey Shipilev
On Mon, 3 Jun 2024 16:47:20 GMT, jengebr wrote: > Improve `java/util/concurrent/CopyOnWriteArrayList` by eliminating needless > cloning of Object[0] instances. This cloning is intended to prevent callers > from changing array contents, but many `CopyOnWriteArrayList`s are allocated > to size z

Re: RFR: 8332586: Avoid cloning empty arrays in java.lang.reflect.{Method,Constructor} [v6]

2024-06-03 Thread Aleksey Shipilev
On Fri, 31 May 2024 18:39:33 GMT, jengebr wrote: >> Improve `java/lang/reflect/Method.java` by eliminating needless cloning of >> Class[0] instances. This cloning is intended to prevent callers from >> changing array contents, but many Methods have zero exceptions or zero >> parameters, and

Re: RFR: 8332586: Avoid cloning empty arrays in java.lang.reflect.{Method,Constructor} [v6]

2024-05-31 Thread Aleksey Shipilev
On Fri, 31 May 2024 18:39:33 GMT, jengebr wrote: >> Improve `java/lang/reflect/Method.java` by eliminating needless cloning of >> Class[0] instances. This cloning is intended to prevent callers from >> changing array contents, but many Methods have zero exceptions or zero >> parameters, and

Re: RFR: 8332586: Avoid cloning empty arrays in java.lang.reflect.{Method,Constructor} [v5]

2024-05-31 Thread Aleksey Shipilev
On Fri, 31 May 2024 17:59:18 GMT, jengebr wrote: >> Improve `java/lang/reflect/Method.java` by eliminating needless cloning of >> Class[0] instances. This cloning is intended to prevent callers from >> changing array contents, but many Methods have zero exceptions or zero >> parameters, and

Re: RFR: 8332586: Avoid cloning empty arrays in java.lang.reflect.{Method,Constructor} [v2]

2024-05-31 Thread Aleksey Shipilev
On Fri, 31 May 2024 16:21:36 GMT, jengebr wrote: >> Improve `java/lang/reflect/Method.java` by eliminating needless cloning of >> Class[0] instances. This cloning is intended to prevent callers from >> changing array contents, but smany Methods have zero exceptions or zero >> parameters, and

Re: RFR: 8332586: Avoid cloning empty arrays in java.lang.reflect.{Method,Constructor}

2024-05-30 Thread Aleksey Shipilev
On Tue, 21 May 2024 13:49:18 GMT, jengebr wrote: > Improve `java/lang/reflect/Method.java` by eliminating needless cloning of > Class[0] instances. This cloning is intended to prevent callers from > changing array contents, but smany Methods have zero exceptions or zero > parameters, and ret

Re: RFR: 8332586: Avoid cloning empty arrays in java.lang.reflect.{Method,Constructor}

2024-05-30 Thread Aleksey Shipilev
On Wed, 22 May 2024 14:32:23 GMT, Chen Liang wrote: >> Thanks. Unfortunately the variable `cloneArray` is actually a method >> parameter, and most of the callers pass in `false` - so using this utility >> method to control cloning would actually introduce cloning in several places >> where it

Re: RFR: 8332586: Avoid cloning empty arrays in java.lang.reflect.{Method,Constructor}

2024-05-30 Thread Aleksey Shipilev
On Wed, 22 May 2024 19:48:49 GMT, Chen Liang wrote: >> I really see no reason to try and save on re-use of this one-line method for >> _everything_. In fact, I do not quite see a very compelling reason to even >> have the utility method. Sharing the code between `Method` and `Constructor` >> i

Re: RFR: 8332586: Avoid cloning empty arrays in java.lang.reflect.{Method,Constructor}

2024-05-30 Thread Aleksey Shipilev
On Tue, 21 May 2024 13:49:18 GMT, jengebr wrote: > Improve `java/lang/reflect/Method.java` by eliminating needless cloning of > Class[0] instances. This cloning is intended to prevent callers from > changing array contents, but smany Methods have zero exceptions or zero > parameters, and ret

Re: RFR: 8327247: C2 uses up to 2GB of RAM to compile complex string concat in extreme cases [v3]

2024-04-26 Thread Aleksey Shipilev
On Fri, 26 Apr 2024 08:45:45 GMT, Claes Redestad wrote: >> Splitting out the ASM-based version from #18690 to push that first under the >> JBS (to help backporting). Keeping #18690 open to rebase and follow-up on >> this as a subtask. See discussion in that #18690 for more details, >> discussi

Re: RFR: 8327247: C2 uses up to 2GB of RAM to compile complex string concat in extreme cases [v7]

2024-04-24 Thread Aleksey Shipilev
On Wed, 24 Apr 2024 10:01:47 GMT, Claes Redestad wrote: > > I really wish this change was not done with ClassFile API, but with a > > simple bundled ASM, so it would be easily backportable, if we decide to. It > > does not look like CFA buys us a lot here readability/complexity wise: > > [d99b

Re: RFR: 8327247: C2 uses up to 2GB of RAM to compile complex string concat in extreme cases [v6]

2024-04-23 Thread Aleksey Shipilev
On Thu, 18 Apr 2024 14:50:30 GMT, Claes Redestad wrote: >> This patch suggests a workaround to an issue with huge SCF MH expression >> trees taking excessive JIT compilation resources by reviving (part of) the >> simple bytecode-generating strategy that was originally available as an >> all-or

Re: RFR: 8325621: Improve jspawnhelper version checks [v2]

2024-03-14 Thread Aleksey Shipilev
On Tue, 12 Mar 2024 18:42:48 GMT, Erik Joelsson wrote: >> Chad Rakoczy has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Code cleanup > > I'm fine with just using VERSION_FEATURE. I think it's a simple and > straightforward enough simplif

Re: RFR: 8325621: Improve jspawnhelper version checks [v4]

2024-03-13 Thread Aleksey Shipilev
On Wed, 13 Mar 2024 17:11:25 GMT, Chad Rakoczy wrote: >> Fix for [8325621](https://bugs.openjdk.org/browse/JDK-8325621) >> >> Updates jspawnhelper to check that JDK version and jspawnhelper version are >> the same. Updates test to include check for version. Also tested manually by >> replacing

Re: RFR: 8325621: Improve jspawnhelper version checks [v3]

2024-03-13 Thread Aleksey Shipilev
On Tue, 12 Mar 2024 19:22:25 GMT, Chad Rakoczy wrote: >> Fix for [8325621](https://bugs.openjdk.org/browse/JDK-8325621) >> >> Updates jspawnhelper to check that JDK version and jspawnhelper version are >> the same. Updates test to include check for version. Also tested manually by >> replacing

Re: RFR: 8325621: Improve jspawnhelper version checks [v3]

2024-03-13 Thread Aleksey Shipilev
On Tue, 12 Mar 2024 23:44:45 GMT, Magnus Ihse Bursie wrote: >> Chad Rakoczy has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - Print to stdout instead of stderr >> - Compare version using VERSION_STRING > > make/modules/java.base/Launche

Re: RFR: 8327998: Enable java/lang/ProcessBuilder/JspawnhelperProtocol.java on Mac

2024-03-13 Thread Aleksey Shipilev
On Tue, 12 Mar 2024 21:52:31 GMT, Elif Aslan wrote: > This change enables to run JspawnhelperProtocol.java on MacOS. > > In addition to GHA , the test has been run on macos and linux. > > > Test report is stored in > build/macosx-x86_64-server-fastdebug/test-results/jtreg_test_jdk_java_lang_P

Re: RFR: 8325621: Improve jspawnhelper version checks [v2]

2024-03-11 Thread Aleksey Shipilev
On Mon, 11 Mar 2024 19:12:33 GMT, Chad Rakoczy wrote: >> Fix for [8325621](https://bugs.openjdk.org/browse/JDK-8325621) >> >> Updates jspawnhelper to check that JDK version and jspawnhelper version are >> the same. Updates test to include check for version. Also tested manually by >> replacing

Re: RFR: 8325621: Improve jspawnhelper version checks

2024-03-11 Thread Aleksey Shipilev
On Mon, 11 Mar 2024 18:56:21 GMT, Bernd wrote: > with a protocol version you don’t have to care about micro versions and also > it is more tolerant about the usual cpu updates which do not introduce > incompatibilities most of the time. I think we can only reasonably guarantee that jspawnhelpe

Re: RFR: 8325621: Improve jspawnhelper version checks

2024-03-11 Thread Aleksey Shipilev
On Mon, 11 Mar 2024 18:16:53 GMT, Erik Joelsson wrote: > If you really want to require an exact match for jspawnhelper, then these 4 > numbers aren't necessarily enough, but a rather arbitrarily chosen > approximation. I think for our purposes, a version number quadruplet is enough to provide

Re: RFR: 8327675: jspawnhelper should be built on all unix platforms

2024-03-08 Thread Aleksey Shipilev
On Fri, 8 Mar 2024 10:17:08 GMT, Magnus Ihse Bursie wrote: > We should match the building of jspawnhelper in > make/modules/java.base/Launcher.gmk with the usage for all unix platforms in > src/java.base/unix/classes/java/lang/ProcessImpl.java. > > Granted, the list of OSes in the makefile amo

Re: RFR: 8325567: jspawnhelper without args fails with segfault [v7]

2024-03-08 Thread Aleksey Shipilev
On Fri, 8 Mar 2024 07:35:27 GMT, Magnus Ihse Bursie wrote: >> test/jdk/java/lang/ProcessBuilder/JspawnhelperWarnings.java line 29: >> >>> 27: * @test >>> 28: * @bug 8325567 >>> 29: * @requires (os.family == "linux") | (os.family == "aix") >> >> Unless I'm mistaken, jspawn helper is used on M

Re: RFR: 8325567: jspawnhelper without args fails with segfault [v7]

2024-03-08 Thread Aleksey Shipilev
On Thu, 7 Mar 2024 20:04:13 GMT, Vladimir Petko wrote: > I wonder if it would make sense to add a test with a correct argument format? I would say that is what current jspawnhelper tests already test, and it is also tested implicitly with `Process` tests. Let's keep this test for testing that

Re: RFR: 8325567: jspawnhelper without args fails with segfault

2024-03-07 Thread Aleksey Shipilev
On Tue, 5 Mar 2024 17:56:21 GMT, Roger Riggs wrote: >> This change is intended to address the segmentation fault issue that occurs >> when jspawnhelper is called without arguments,. >> There is a new test added to verify the behavior in such cases. >> >> `[ec2-user@ip-172-16-0-10 jdk]$ make CO

Re: RFR: 8325567: jspawnhelper without args fails with segfault [v7]

2024-03-07 Thread Aleksey Shipilev
On Thu, 7 Mar 2024 17:13:12 GMT, Elif Aslan wrote: >> This change is intended to address the segmentation fault issue that occurs >> when jspawnhelper is called without arguments,. >> There is a new test added to verify the behavior in such cases. >> >> `[ec2-user@ip-172-16-0-10 jdk]$ make CON

Re: RFR: 8325567: jspawnhelper without args fails with segfault [v3]

2024-03-07 Thread Aleksey Shipilev
On Thu, 7 Mar 2024 16:29:12 GMT, Elif Aslan wrote: >> This change is intended to address the segmentation fault issue that occurs >> when jspawnhelper is called without arguments,. >> There is a new test added to verify the behavior in such cases. >> >> `[ec2-user@ip-172-16-0-10 jdk]$ make CON

Re: RFR: 8325567: jspawnhelper without args fails with segfault [v4]

2024-03-07 Thread Aleksey Shipilev
On Thu, 7 Mar 2024 16:33:11 GMT, Elif Aslan wrote: >> This change is intended to address the segmentation fault issue that occurs >> when jspawnhelper is called without arguments,. >> There is a new test added to verify the behavior in such cases. >> >> `[ec2-user@ip-172-16-0-10 jdk]$ make CON

Re: RFR: 8325567: jspawnhelper without args fails with segfault

2024-03-06 Thread Aleksey Shipilev
On Tue, 5 Mar 2024 20:34:47 GMT, Vladimir Petko wrote: > > The change in jspawnhelper looks good. > > I think it would be simpler to have a separate > > `jdk/java/lang/ProcessBuilder/JspawnhelperMisuse.java` test that simply > > invokes the `jspawnhelper` and verifies the proper message is prin

Re: RFR: 8325567: jspawnhelper without args fails with segfault

2024-03-05 Thread Aleksey Shipilev
On Mon, 4 Mar 2024 21:19:26 GMT, Elif Aslan wrote: > This change is intended to address the segmentation fault issue that occurs > when jspawnhelper is called without arguments, and it includes an updated > test to verify the behavior in such cases. > > Existing tests passes since it does not

Re: RFR: 8326461: tools/jlink/CheckExecutable.java fail after JDK-8325342

2024-02-22 Thread Aleksey Shipilev
On Thu, 22 Feb 2024 07:23:04 GMT, SendaoYan wrote: > Before JDK-8325342(commit id:0bcece995840777db660811e4b20bb018e90439b), all > the files in build/linux-x86_64-server-release/images/jdk/bin are executable: > > ![image](https://github.com/openjdk/jdk/assets/24123821/13f0eae2-7125-4d09-8793-8a

Re: RFR: 8326370: Remove redundant and misplaced micros from StringBuffers

2024-02-21 Thread Aleksey Shipilev
On Tue, 20 Feb 2024 20:58:50 GMT, Claes Redestad wrote: > Some microbenchmarks in org.openjdk.bench.java.lang.StringBuffers seem > out-of-place (not testing `StringBuffer`), redundant (covered by other tests > like StringSubstring or the various String concatenation tests), or both. > This cle

Re: RFR: 8325730: StringBuilder.toString allocation for the empty String [v3]

2024-02-20 Thread Aleksey Shipilev
On Tue, 20 Feb 2024 18:38:48 GMT, Claes Redestad wrote: >> JDK-8282429 accidentally removed an optimization (JDK-8240094) that ensured >> StringBuilder/StringBuffer::toString returns `""` when the builders are >> empty. >> >> >> Name Cnt Base Error Test Er

Re: RFR: 8325730: StringBuilder.toString allocation for the empty String [v2]

2024-02-20 Thread Aleksey Shipilev
On Tue, 20 Feb 2024 18:04:18 GMT, Claes Redestad wrote: >> src/java.base/share/classes/java/lang/StringBuilder.java line 478: >> >>> 476: } >>> 477: // Create a copy, don't share the array >>> 478: return new String(this, null); >> >> Ok, this got me a bit confused, but

Re: RFR: 8325730: StringBuilder.toString allocation for the empty String [v2]

2024-02-20 Thread Aleksey Shipilev
On Tue, 20 Feb 2024 18:00:42 GMT, Claes Redestad wrote: >> test/micro/org/openjdk/bench/java/lang/StringBuffers.java line 49: >> >>> 47: >>> 48: @Benchmark >>> 49: public String emptyToString() { >> >> Do we actually need to remove the `substring` test here? > > I couldn't figure out w

Re: RFR: 8325730: StringBuilder.toString allocation for the empty String

2024-02-20 Thread Aleksey Shipilev
On Tue, 20 Feb 2024 16:32:54 GMT, Claes Redestad wrote: > JDK-8282429 accidentally removed an optimization (JDK-8240094) that ensured > StringBuilder/StringBuffer::toString returns `""` when the builders are empty. > > > Name Cnt Base Error Test Error Uni

Re: RFR: 8325576: java/lang/ProcessHandle/InfoTest.java fails on systems with coreutils with --enable-single-binary [v2]

2024-02-13 Thread Aleksey Shipilev
On Tue, 13 Feb 2024 18:15:59 GMT, Dan Lutker wrote: >> Just ignore the binary name and only check for argv1 is imho enough. > > I couldn't think if a better way to detect what the host system had and > adding this seemed inline with the busybox check. > > The choice of running `sleep` seems li

Re: RFR: 8325576: java/lang/ProcessHandle/InfoTest.java fails on systems with coreutils with --enable-single-binary

2024-02-13 Thread Aleksey Shipilev
On Fri, 9 Feb 2024 23:40:00 GMT, Dan Lutker wrote: > Ran the test on AmazonLinux 2 which has multiple binaries from coreutils > package and no coreutils executable as well as AmazonLinux 2023 that uses > `--enable-single-binary` test/jdk/java/lang/ProcessHandle/InfoTest.java line 321: > 319:

Re: [jdk22] RFR: 8324858: [vectorapi] Bounds checking issues when accessing memory segments

2024-02-07 Thread Aleksey Shipilev
On Tue, 6 Feb 2024 16:50:10 GMT, Paul Sandoz wrote: > This pull request contains a backport of commit > [1ae85138](https://github.com/openjdk/jdk/commit/1ae851387f881263ccc6aeace5afdd0f49d41d33) > from the [openjdk/jdk](https://git.openjdk.org/jdk) repository. > > The commit being backported w

Re: RFR: JDK-8324930: java/lang/StringBuilder problem with concurrent jtreg runs

2024-01-30 Thread Aleksey Shipilev
On Tue, 30 Jan 2024 09:08:28 GMT, Matthias Baesken wrote: > On some Windows machines we see sometimes OOM errors because of high resource > (memory/swap) consumption. This is especially seen when the jtreg runs have > higher concurrency. A solution is to put the java/lang/StringBuilder tests in

Integrated: 8323717: Introduce test keyword for tests that need external dependencies

2024-01-25 Thread Aleksey Shipilev
On Mon, 15 Jan 2024 10:48:23 GMT, Aleksey Shipilev wrote: > Some jtreg tests require resolvable external dependencies. This resolution is > delegated to JIB, which is not used in vanilla OpenJDK testing. It would be > convenient to add a keyword that marks tests that require these

Re: RFR: 8323717: Introduce test keyword for tests that need external dependencies

2024-01-25 Thread Aleksey Shipilev
On Mon, 15 Jan 2024 10:48:23 GMT, Aleksey Shipilev wrote: > Some jtreg tests require resolvable external dependencies. This resolution is > delegated to JIB, which is not used in vanilla OpenJDK testing. It would be > convenient to add a keyword that marks tests that require these

Re: RFR: 8324433: Introduce a way to determine if an expression is evaluated as a constant by the Jit compiler [v6]

2024-01-25 Thread Aleksey Shipilev
On Thu, 25 Jan 2024 14:48:16 GMT, Maurizio Cimadamore wrote: > I don't 100% buy the `MethodHandleImpl` analogy. In that case the check is > not simply used to save a branch, but to spare spinning of a completely new > lambda form. Doing this to save a few intructions would not likely to worth

Re: RFR: 8323717: Introduce test keyword for tests that need external dependencies

2024-01-25 Thread Aleksey Shipilev
On Wed, 24 Jan 2024 21:28:29 GMT, Leonid Mesnik wrote: >> Some jtreg tests require resolvable external dependencies. This resolution >> is delegated to JIB, which is not used in vanilla OpenJDK testing. It would >> be convenient to add a keyword that marks tests that require these external >>

Re: RFR: 8324433: Introduce a way to determine if an expression is evaluated as a constant by the Jit compiler [v6]

2024-01-24 Thread Aleksey Shipilev
On Wed, 24 Jan 2024 18:51:27 GMT, Maurizio Cimadamore wrote: > Naive question: the right way to use this would be almost invariably be like > this: > > ``` > if (isCompileConstant(foo) && fooHasCertainStaticProperties(foo)) { > // fast-path > } > // slow path > ``` > > Right? Yes, I thi

Re: RFR: 8324433: Introduce a way to determine if an expression is evaluated as a constant by the Jit compiler [v6]

2024-01-24 Thread Aleksey Shipilev
On Wed, 24 Jan 2024 10:33:05 GMT, Quan Anh Mai wrote: >> Hi, >> >> This patch introduces `JitCompiler::isConstantExpression` which can be used >> to statically determine whether an expression has been constant-folded by >> the Jit compiler, leading to more constant-folding opportunities. For

Re: RFR: 8324433: Introduce a way to determine if an expression is evaluated as a constant by the Jit compiler [v5]

2024-01-24 Thread Aleksey Shipilev
On Wed, 24 Jan 2024 07:15:12 GMT, Quan Anh Mai wrote: >> This seems really weird to me for Java code. The method doesn't get the >> original "expression" it only gets the value of that expression after it has >> been evaluated. Is there some kind of weird "magic" happening here? > > @dholmes-or

Re: RFR: 8324647: Invalid test group of lib-test after JDK-8323515

2024-01-24 Thread Aleksey Shipilev
On Wed, 24 Jan 2024 15:20:37 GMT, Jie Fu wrote: > This patch tries to fix the invalid test group definition of lib-test. > Please review. > Thanks. Ooof. I wonder how that happened. This would not show up before we try to run the actual libtest tests. What is extra wild is that GHA reports gree

Re: RFR: 8324433: Introduce a way to determine if an expression is evaluated as a constant by the Jit compiler [v5]

2024-01-24 Thread Aleksey Shipilev
On Tue, 23 Jan 2024 22:49:49 GMT, Quan Anh Mai wrote: >> src/java.base/share/classes/jdk/internal/misc/JitCompiler.java line 32: >> >>> 30: * Just-in-time-compiler-related queries >>> 31: */ >>> 32: public class JitCompiler { >> >> An alternative name and location is `jdk.internal.vm.Constant

Re: RFR: 8324433: Introduce a way to determine if an expression is evaluated as a constant by the Jit compiler [v5]

2024-01-24 Thread Aleksey Shipilev
On Tue, 23 Jan 2024 22:41:44 GMT, Quan Anh Mai wrote: >> src/java.base/share/classes/jdk/internal/misc/JitCompiler.java line 119: >> >>> 117: * @see #isCompileConstant(boolean) >>> 118: */ >>> 119: @IntrinsicCandidate >> >> Note how the Java entry for MH intrinsic we have replaced

Re: RFR: 8324433: Introduce a way to determine if an expression is evaluated as a constant by the Jit compiler [v3]

2024-01-23 Thread Aleksey Shipilev
On Tue, 23 Jan 2024 16:01:05 GMT, Aleksey Shipilev wrote: >> Would it be possible to list further examples where this might be used? >> Asking because I'm wondering about the usability and maintainability of >> if-then-else code. > >> Would it be possible to

Re: RFR: 8324433: Introduce a way to determine if an expression is evaluated as a constant by the Jit compiler [v5]

2024-01-23 Thread Aleksey Shipilev
On Tue, 23 Jan 2024 17:21:47 GMT, Quan Anh Mai wrote: >> Hi, >> >> This patch introduces `JitCompiler::isConstantExpression` which can be used >> to statically determine whether an expression has been constant-folded by >> the Jit compiler, leading to more constant-folding opportunities. For

Re: RFR: 8323515: Create test alias "all" for all test roots [v3]

2024-01-23 Thread Aleksey Shipilev
On Tue, 16 Jan 2024 09:01:35 GMT, Aleksey Shipilev wrote: >> Since recent work to improve `tier4` performance, we actually test >> `tier{1,2,3,4}` often, which includes all the tests in current tree. It >> would be more convenient to just have the `all` test group/alias, so

Integrated: 8323515: Create test alias "all" for all test roots

2024-01-23 Thread Aleksey Shipilev
On Mon, 15 Jan 2024 11:05:09 GMT, Aleksey Shipilev wrote: > Since recent work to improve `tier4` performance, we actually test > `tier{1,2,3,4}` often, which includes all the tests in current tree. It would > be more convenient to just have the `all` test group/alias, so that

Re: RFR: 8324433: Introduce a way to determine if an expression is evaluated as a constant by the Jit compiler [v3]

2024-01-23 Thread Aleksey Shipilev
On Tue, 23 Jan 2024 15:52:29 GMT, Alan Bateman wrote: > Would it be possible to list further examples where this might be used? > Asking because I'm wondering about the usability and maintainability of > if-then-else code. A similar thing is already used in JDK: https://github.com/openjdk/jdk

  1   2   3   4   5   >