Re: RFR: 8321971: Improve the user name detection logic in perfMemory get_user_name_slow [v3]
> Can I please get a review of this change which proposes to improve the code > in `get_user_name_slow` function, which is used to identify the target JVM > owner's user name? This addresses https://bugs.openjdk.org/browse/JDK-8321971. > > As noted in that JBS issue, in its current form, the nested loop ends up > iterating over the directory contents of `hsperfdata_xxx` directory and then > for each iteration it checks if the name of the entry matches the pid. This > iteration shouldn't be needed and instead one could look for a file named > `` within that directory. > > No new test has been added, given the nature of this change. Existing tier1, > tier2, tier3 and svc_tools tests pass with this change on Linux, Windows and > macosx. Jaikiran Pai has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision: - remove redundant if block - merge latest from master branch - David's review comments - reduce if blocks and release the array outside if block - David's review comment - punctuation - 8321971: Improve the user name detection logic in perfMemory get_user_name_slow - Changes: - all: https://git.openjdk.org/jdk/pull/17104/files - new: https://git.openjdk.org/jdk/pull/17104/files/cfd50d79..243d9817 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=17104&range=02 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=17104&range=01-02 Stats: 3660 lines in 265 files changed: 2177 ins; 586 del; 897 mod Patch: https://git.openjdk.org/jdk/pull/17104.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/17104/head:pull/17104 PR: https://git.openjdk.org/jdk/pull/17104
Re: RFR: 8311218: fatal error: stuck in JvmtiVTMSTransitionDisabler::VTMS_transition_disable [v8]
On Mon, 18 Dec 2023 17:09:59 GMT, Serguei Spitsyn wrote:
>> This fix is for JDK 23 but the intention is to back port it to 22 in RDP-1
>> time frame.
>> It is fixing a deadlock issue between `VirtualThread` class critical
>> sections with the `interruptLock` (in methods: `unpark()`, `interrupt()`,
>> `getAndClearInterrupt()`, `threadState()`, `toString()`),
>> `JvmtiVTMSTransitionDisabler` and JVMTI `Suspend/Resume` mechanisms.
>> The deadlocking scenario is well described by Patricio in a bug report
>> comment.
>> In simple words, a virtual thread should not be suspended during
>> 'interruptLock' critical sections.
>>
>> The fix is to record that a virtual thread is in a critical section
>> (`JavaThread`'s `_in_critical_section` bit) by notifying the VM/JVMTI about
>> begin/end of critical section.
>> This bit is used in `HandshakeState::get_op_for_self()` to filter out any
>> `HandshakeOperation` if a target `JavaThread` is in a critical section.
>>
>> Some of new notifications with `notifyJvmtiSync()` method is on a
>> performance critical path. It is why this method has been intrincified.
>>
>> New test was developed by Patricio:
>> `test/hotspot/jtreg/serviceability/jvmti/vthread/SuspendWithInterruptLock`
>> The test is very nice as it reliably in 100% reproduces the deadlock without
>> the fix.
>> The test is never failing with this fix.
>>
>> Testing:
>> - tested with newly added test:
>> `test/hotspot/jtreg/serviceability/jvmti/vthread/SuspendWithInterruptLock`
>> - tested with mach5 tiers 1-6
>
> Serguei Spitsyn has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains 10 commits:
>
> - Merge
> - review: improve an assert message
> - review: moved a couple of comments out of try blocks
> - review: moved notifyJvmtiDisableSuspend(true) out of try-block
> - review: 1) replace CriticalLock with DisableSuspend; 2) minor tweaks
> - review: (1) rename notifyJvmti method; (2) add try-final statements to
> VirtualThread methods
> - Resolved merge conflict in VirtualThread.java
> - added @summary to new test SuspendWithInterruptLock.java
> - add new test SuspendWithInterruptLock.java
> - 8311218: fatal error: stuck in
> JvmtiVTMSTransitionDisabler::VTMS_transition_disable
src/hotspot/share/prims/jvm.cpp line 4024:
> 4022: #else
> 4023: fatal("Should only be called with JVMTI enabled");
> 4024: #endif
You can't do this! The Java code knows nothing about JVM TI being
enabled/disabled and will call this function unconditionally.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17011#discussion_r1432241016
Re: RFR: 8309271: A way to align already compiled methods with compiler directives [v15]
On Thu, 14 Dec 2023 15:29:06 GMT, Dmitry Chuyko wrote:
>> Compiler Control (https://openjdk.org/jeps/165) provides method-context
>> dependent control of the JVM compilers (C1 and C2). The active directive
>> stack is built from the directive files passed with the
>> `-XX:CompilerDirectivesFile` diagnostic command-line option and the
>> Compiler.add_directives diagnostic command. It is also possible to clear all
>> directives or remove the top from the stack.
>>
>> A matching directive will be applied at method compilation time when such
>> compilation is started. If directives are added or changed, but compilation
>> does not start, then the state of compiled methods doesn't correspond to the
>> rules. This is not an error, and it happens in long running applications
>> when directives are added or removed after compilation of methods that could
>> be matched. For example, the user decides that C2 compilation needs to be
>> disabled for some method due to a compiler bug, issues such a directive but
>> this does not affect the application behavior. In such case, the target
>> application needs to be restarted, and such an operation can have high costs
>> and risks. Another goal is testing/debugging compilers.
>>
>> It would be convenient to optionally reconcile at least existing matching
>> nmethods to the current stack of compiler directives (so bypass inlined
>> methods).
>>
>> Natural way to eliminate the discrepancy between the result of compilation
>> and the broken rule is to discard the compilation result, i.e.
>> deoptimization. Prior to that we can try to re-compile the method letting
>> compile broker to perform it taking new directives stack into account.
>> Re-compilation helps to prevent hot methods from execution in the
>> interpreter.
>>
>> A new flag `-r` has beed introduced for some directives related to compile
>> commands: `Compiler.add_directives`, `Compiler.remove_directives`,
>> `Compiler.clear_directives`. The default behavior has not changed (no flag).
>> If the new flag is present, the command scans already compiled methods and
>> puts methods that have any active non-default matching compiler directives
>> to re-compilation if possible, otherwise marks them for deoptimization.
>> There is currently no distinction which directives are found. In particular,
>> this means that if there are rules for inlining into some method, it will be
>> refreshed. On the other hand, if there are rules for a method and it was
>> inlined, top-level methods won't be refreshed, but this can be achieved by
>> having rules for them.
>>
>> In addition, a new diagnostic command `Compiler.replace_directives...
>
> Dmitry Chuyko has updated the pull request with a new target base due to a
> merge or a rebase. The pull request now contains 33 commits:
>
> - Merge branch 'openjdk:master' into compiler-directives-force-update
> - Merge branch 'openjdk:master' into compiler-directives-force-update
> - Merge branch 'openjdk:master' into compiler-directives-force-update
> - Merge branch 'openjdk:master' into compiler-directives-force-update
> - Merge branch 'openjdk:master' into compiler-directives-force-update
> - Merge branch 'openjdk:master' into compiler-directives-force-update
> - Merge branch 'openjdk:master' into compiler-directives-force-update
> - Merge branch 'openjdk:master' into compiler-directives-force-update
> - Merge branch 'openjdk:master' into compiler-directives-force-update
> - Merge branch 'openjdk:master' into compiler-directives-force-update
> - ... and 23 more: https://git.openjdk.org/jdk/compare/fde5b168...44d680cd
src/hotspot/share/code/codeCache.cpp line 1409:
> 1407: while(iter.next()) {
> 1408: CompiledMethod* nm = iter.method();
> 1409: methodHandle mh(thread, nm->method());
If there are two CompiledMethods for the same Java method, will it be scheduled
for recompilation twice? Related question: if `nm` is an OSR method, does it
make sense to go directly for deoptimization rather than compiling a non-OSR
version?
src/hotspot/share/code/codeCache.cpp line 1413:
> 1411: ResourceMark rm;
> 1412: // Try the max level and let the directives be applied during the
> compilation.
> 1413: int complevel = CompLevel::CompLevel_full_optimization;
Should the highest level depend on the configuration instead of the hard-coded
constant? Perhaps, needs to be `highest_compile_level()`
src/hotspot/share/compiler/compilerDirectives.cpp line 750:
> 748: if (!dir->is_default_directive() && dir->match(method)) {
> 749: match_found = true;
> 750: break;
`match_found` is redundant: for better readability, you may just return true.
Curly braces around MutexLocker won't be needed either.
src/hotspot/share/oops/method.hpp line 820:
> 818: // Clear the flags related to compiler directives that were set by the
> compilerBroker,
> 819: // because the directives can be updated.
> 820: void c
[jdk22] RFR: 8321565: [REDO] Heap dump does not contain virtual Thread stack references
Hi all, This pull request contains a backport of commit [cf948548](https://github.com/openjdk/jdk/commit/cf948548c390c42ca63525d41a9d63ff31349c3a) from the [openjdk/jdk](https://git.openjdk.org/jdk) repository. The commit being backported was authored by Alex Menkov on 13 Dec 2023 and was reviewed by Serguei Spitsyn, Yi Yang and David Holmes. Thanks! - Commit messages: - Backport cf948548c390c42ca63525d41a9d63ff31349c3a Changes: https://git.openjdk.org/jdk22/pull/21/files Webrev: https://webrevs.openjdk.org/?repo=jdk22&pr=21&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8321565 Stats: 319 lines in 3 files changed: 152 ins; 81 del; 86 mod Patch: https://git.openjdk.org/jdk22/pull/21.diff Fetch: git fetch https://git.openjdk.org/jdk22.git pull/21/head:pull/21 PR: https://git.openjdk.org/jdk22/pull/21
Re: RFR: 8321971: Improve the user name detection logic in perfMemory get_user_name_slow [v2]
On Mon, 18 Dec 2023 10:24:06 GMT, Jaikiran Pai wrote:
>> Can I please get a review of this change which proposes to improve the code
>> in `get_user_name_slow` function, which is used to identify the target JVM
>> owner's user name? This addresses
>> https://bugs.openjdk.org/browse/JDK-8321971.
>>
>> As noted in that JBS issue, in its current form, the nested loop ends up
>> iterating over the directory contents of `hsperfdata_xxx` directory and then
>> for each iteration it checks if the name of the entry matches the pid. This
>> iteration shouldn't be needed and instead one could look for a file named
>> `` within that directory.
>>
>> No new test has been added, given the nature of this change. Existing tier1,
>> tier2, tier3 and svc_tools tests pass with this change on Linux, Windows and
>> macosx.
>
> Jaikiran Pai has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - David's review comments - reduce if blocks and release the array outside
> if block
> - David's review comment - punctuation
src/hotspot/os/posix/perfMemory_posix.cpp line 609:
> 607: if (statbuf.st_size > 0 && statbuf.st_ctime > oldest_ctime) {
> 608:
> 609: if (statbuf.st_ctime > oldest_ctime) {
This `if` expression is repeats what we already know to be true from the
previous `if` expression.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/17104#discussion_r1431895566
Re: RFR: JDK-8322237: Heap dump contains duplicate thread records for mounted virtual threads
On Tue, 19 Dec 2023 05:16:38 GMT, David Holmes wrote: >> Good point. I'll remove dependency on JVMTI. >> I don't think approximation would be good here (comparing state to >> RUNNABLE/PINNED/TIMED_PINNED or comparing carrierThread with null). >> It's racy and we have a chance to not dump unmounted vthread or dump mounted >> vthread twice. >> Maybe `is_vthread_mounted` should check if the virtual thread continuation >> has non-empty chunk. > > If that is racy then any solution is going to be racy. I assumed this was all > happening at a global safepoint, otherwise threads could be > mounting/unmounting at any time. > > I said "approximation" only because I'm unsure exactly when the thread state > gets updated in the mounting/unmounting process. I mean race between virtual thread state change and the thread stack switch (to/from carrier). Correct condition here is "dump the virtual thread if it was not dumped as mounted virtual thread". Most likely for RUNNABLE/PINNED/TIMED_PINNED we can be sure the thread is mounted, but we can get vthread in transition (PARKING, YIELDING, etc). Virtual threads may be in transition at safepoints (but they can't change mounted/unmounted state). So I think `is_vthread_mounted` can be implemented in 2 ways: 1) copy logic of JvmtiEnvBase::get_JavaThread_or_null: get JavaThread for java_lang_VirtualThread::carrier_thread(vt); if not null, check Continuation::is_continuation_mounted(java_thread, java_lang_VirtualThread::continuation(vt)) - this is to handle transition, when vthread is already unmounted, but carrierThread is not yet set to null; 2) check that java_lang_VirtualThread::continuation(vt) doesn't have non-empty chunk. AFAIU this is true for mounted vthreads. If we get it for unmounted vt, its stack trace of the thread is empty anyway, so it's ok to skip it (most likely it can happen only if thread state is NEW or TERMINATED, we already skip such vthreads). @AlanBateman could you please comment if my understanding is correct - PR Review Comment: https://git.openjdk.org/jdk/pull/17134#discussion_r1431959463
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v5]
> J2SE agent does not start and throws error when it tries to find the shared > library ibm_16_am. > After searching for ibm_16_am.so ,the jvm agent throws and error as dll_load > fails.It fails to identify the shared library ibm_16_am.a shared archive file > on AIX. > Hence we are providing a function which will additionally search for .a file > on AIX ,when the search for .so file fails. Suchismith Roy has updated the pull request incrementally with four additional commits since the last revision: - Change return type - Change dll load function signature that does dlopen - Remove AIX macros - Add wrapper function to check extension before dlopen - Changes: - all: https://git.openjdk.org/jdk/pull/16604/files - new: https://git.openjdk.org/jdk/pull/16604/files/eb09224d..cd7e0e64 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16604&range=04 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16604&range=03-04 Stats: 34 lines in 2 files changed: 22 ins; 11 del; 1 mod Patch: https://git.openjdk.org/jdk/pull/16604.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16604/head:pull/16604 PR: https://git.openjdk.org/jdk/pull/16604
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v4]
> J2SE agent does not start and throws error when it tries to find the shared > library ibm_16_am. > After searching for ibm_16_am.so ,the jvm agent throws and error as dll_load > fails.It fails to identify the shared library ibm_16_am.a shared archive file > on AIX. > Hence we are providing a function which will additionally search for .a file > on AIX ,when the search for .so file fails. Suchismith Roy has refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR. The pull request contains eight new commits since the last revision: - merge pr/16920 - change macro position - Adapt hotspot coding style - Improve comments and coding style. - Remove macro for file extension. - Move mapping function to aix specific file. - Introduce new macro for AIX archives. - Add support for .a extension in jvm agent. 1. Add support to load archive files and shared objects in jvm agent for AIX. - Changes: - all: https://git.openjdk.org/jdk/pull/16604/files - new: https://git.openjdk.org/jdk/pull/16604/files/151f6c20..eb09224d Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=16604&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=16604&range=02-03 Stats: 3 lines in 2 files changed: 0 ins; 3 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/16604.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16604/head:pull/16604 PR: https://git.openjdk.org/jdk/pull/16604
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Tue, 19 Dec 2023 16:47:33 GMT, Goetz Lindenmaier wrote: > try it! error: failed to push some refs to 'github.com:suchismith1993/jdk.git' I tried the fork instructions . However I think I need to do a force push. Will that be fine since the PR is not in draft state ? - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1863210891
Integrated: 8311218: fatal error: stuck in JvmtiVTMSTransitionDisabler::VTMS_transition_disable
On Thu, 7 Dec 2023 06:28:43 GMT, Serguei Spitsyn wrote: > This fix is for JDK 23 but the intention is to back port it to 22 in RDP-1 > time frame. > It is fixing a deadlock issue between `VirtualThread` class critical sections > with the `interruptLock` (in methods: `unpark()`, `interrupt()`, > `getAndClearInterrupt()`, `threadState()`, `toString()`), > `JvmtiVTMSTransitionDisabler` and JVMTI `Suspend/Resume` mechanisms. > The deadlocking scenario is well described by Patricio in a bug report > comment. > In simple words, a virtual thread should not be suspended during > 'interruptLock' critical sections. > > The fix is to record that a virtual thread is in a critical section > (`JavaThread`'s `_in_critical_section` bit) by notifying the VM/JVMTI about > begin/end of critical section. > This bit is used in `HandshakeState::get_op_for_self()` to filter out any > `HandshakeOperation` if a target `JavaThread` is in a critical section. > > Some of new notifications with `notifyJvmtiSync()` method is on a performance > critical path. It is why this method has been intrincified. > > New test was developed by Patricio: > `test/hotspot/jtreg/serviceability/jvmti/vthread/SuspendWithInterruptLock` > The test is very nice as it reliably in 100% reproduces the deadlock without > the fix. > The test is never failing with this fix. > > Testing: > - tested with newly added test: > `test/hotspot/jtreg/serviceability/jvmti/vthread/SuspendWithInterruptLock` > - tested with mach5 tiers 1-6 This pull request has now been integrated. Changeset: 0f8e4e0a Author:Serguei Spitsyn URL: https://git.openjdk.org/jdk/commit/0f8e4e0a81257c678e948c341a241dc0b810494f Stats: 229 lines in 15 files changed: 196 ins; 0 del; 33 mod 8311218: fatal error: stuck in JvmtiVTMSTransitionDisabler::VTMS_transition_disable Reviewed-by: lmesnik, alanb - PR: https://git.openjdk.org/jdk/pull/17011
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v3]
> J2SE agent does not start and throws error when it tries to find the shared > library ibm_16_am. > After searching for ibm_16_am.so ,the jvm agent throws and error as dll_load > fails.It fails to identify the shared library ibm_16_am.a shared archive file > on AIX. > Hence we are providing a function which will additionally search for .a file > on AIX ,when the search for .so file fails. Suchismith Roy has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains eight commits: - Merge branch 'pr/16920' into jvmagent - change macro position - Adapt hotspot coding style - Improve comments and coding style. - Remove macro for file extension. - Move mapping function to aix specific file. - Introduce new macro for AIX archives. - Add support for .a extension in jvm agent. 1. Add support to load archive files and shared objects in jvm agent for AIX. - Changes: https://git.openjdk.org/jdk/pull/16604/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16604&range=02 Stats: 13 lines in 3 files changed: 13 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/16604.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/16604/head:pull/16604 PR: https://git.openjdk.org/jdk/pull/16604
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Wed, 22 Nov 2023 16:24:24 GMT, Suchismith Roy wrote: >> J2SE agent does not start and throws error when it tries to find the shared >> library ibm_16_am. >> After searching for ibm_16_am.so ,the jvm agent throws and error as dll_load >> fails.It fails to identify the shared library ibm_16_am.a shared archive >> file on AIX. >> Hence we are providing a function which will additionally search for .a file >> on AIX ,when the search for .so file fails. > > Suchismith Roy has updated the pull request incrementally with one additional > commit since the last revision: > > change macro position try it! - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1863123599
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Tue, 19 Dec 2023 13:55:09 GMT, Suchismith Roy wrote: >>> > > >>> > >>> > >>> > @tstuefe 3rd parameter to pass the either of 2 things: >>> > ``` >>> > 1. The JvmTiAgent object "agent", so that after shifting the >>> > save_library_signature to os_aix,we can still access the agent object.-> >>> > For this i tried importing jvm/prims header file, but i get segmentation >>> > faults during build . Not sure if i am doing it the right way. >>> > >>> > 2. Pass a character buffer(and not const char*) where we copy the >>> > modified filename back to it and then use it in jvmAgent. code. >>> > ``` >>> >>> Does not sound really appealing tbh. We pile one hack atop of another. >>> >>> Please synchronize with @JoKern65 at SAP. He will rewrite the JVMTI handler >>> code, which will make this point moot. See >>> https://bugs.openjdk.org/browse/JDK-8320890. >> >> Hi @tstuefe Should i then wait for this code to be integrated and then >> rewrite the .a handling ? >> I mean this PR shall remain open then right ? >> @JoKern65 Are you even handling the .a handling case ? i would like this PR >> to stay open. Maybe i can wait for the design change that you are working on. > >> Hi @suchismith1993 , you can place this change on top of #16920 by comparing >> it with branch origin/pr/16920 instead of master. This way you might be able >> to proceed with your change. But as Thomas says you can only push if you >> have appropriate reviews. > > Hi @GoeLin I am not sure how to do that . Could you tell me in brief ? > Do I run the checkout command on the other PR and then place my change of top > of it ? > > > Hi @suchismith1993 , you can place this change on top of #16920 by > > > comparing it with branch origin/pr/16920 instead of master. This way you > > > might be able to proceed with your change. But as Thomas says you can > > > only push if you have appropriate reviews. > > > > > > Hi @GoeLin I am not sure how to do that . Could you tell me in brief ? Do I > > run the checkout command on the other PR and then place my change of top of > > it ? > > Yes, you can do that. You can also change the branch in this pr. Click edit > on the top right. Choose an alternative for "openjdk:master" I see. So If I do that, will it reflect on my command line in local machine ? I mean I need run a rebase command with origin as pr/16920 ? - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1863086746
Re: RFR: 8319115: GrowableArray: Do not initialize up to capacity
On Fri, 1 Dec 2023 07:56:04 GMT, Emanuel Peter wrote: > Before this patch, we always initialized the GrowableArray up to its > `capacity`, and not just up to `length`. This is problematic for a few > reasons: > > - It is not expected. `std::vector` also only initializes the elements up to > its size, and not to capacity. > - It requires a default-constructor for the element type. And the > default-constructor is then used to fill in the elements between length and > capacity. If the elements do any allocation themselves, then this is a waste > of resources. > - The implementation also required the copy-assignment-operator for the > element type. This is a lesser restriction. But the copy-assignment-operator > was used in cases like `append` (where placement copy-construct would be > expected), and not just in true assignment kinds of cases like `at_put`. > > For this reason, I reworked a lot of the methods to ensure that only the > "slots" up to `length` are ever initialized, and the space between `length` > and `capacity` is always garbage. > > - > > Also, before this patch, one can CHeap allocate both with `GrowableArray` and > `GrowableArrayCHeap`. This is unnecessary. It required more complex > verification in `GrowableArray` to deal with all cases. And > `GrowableArrayCHeap` is already explicitly a smaller object, and should hence > be preferred. Hence I changed all CHeap allocating cases of `GrowableArray` > to `GrowableArrayCHeap`. This also allows for a clear separation: > - `GrowableArray` only deals with arena / resource area allocation. These are > arrays that are regularly abandoned at the end of their use, rather than > deleted or even cleared. > - `GrowableArrayCHeap` only deals with CHeap allocated memory. We expect that > the destructor for it is called eventually, either when it goes out of scope > or when `delete` is explicitly called. We expect that the elements could be > allocating resources internally, and hence rely on the destructors for the > elements being called, which may free up those internally allocated resources. > > Therefore, we now only allow `GrowableArrayCHeap` to have element types with > non-trivial destructors, but `GrowableArray` checks that element types do not > have non-trivial destructors (since it is common practice to just abandon > arena / resource area allocated arrays, rather than calling the destructor or > clearing the array, which also destructs all elements). This more clearly > separates the two worlds: clean-up your own mess (CHeap) vs abandon your mess > (arena / resource area). > > - > > I also completely refactored and improved ... Filed: JDK-8322476: Remove GrowableArray C-Heap version, replace usages with GrowableArrayCHeap Will work on that first, and then come back here later. - PR Comment: https://git.openjdk.org/jdk/pull/16918#issuecomment-1863061669
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Tue, 19 Dec 2023 13:55:09 GMT, Suchismith Roy wrote: > > Hi @suchismith1993 , you can place this change on top of #16920 by > > comparing it with branch origin/pr/16920 instead of master. This way you > > might be able to proceed with your change. But as Thomas says you can only > > push if you have appropriate reviews. > > Hi @GoeLin I am not sure how to do that . Could you tell me in brief ? Do I > run the checkout command on the other PR and then place my change of top of > it ? Yes, you can do that. You can also change the branch in this pr. Click edit on the top right. Choose an alternative for "openjdk:master" - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1863051404
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Tue, 28 Nov 2023 12:59:01 GMT, Suchismith Roy wrote: >>> > >>> >>> @tstuefe 3rd parameter to pass the either of 2 things: >>> >>> 1. The JvmTiAgent object "agent", so that after shifting the >>> save_library_signature to os_aix,we can still access the agent object.-> >>> For this i tried importing jvm/prims header file, but i get segmentation >>> faults during build . Not sure if i am doing it the right way. >>> >>> 2. Pass a character buffer(and not const char*) where we copy the >>> modified filename back to it and then use it in jvmAgent. code. >> >> Does not sound really appealing tbh. We pile one hack atop of another. >> >> Please synchronize with @JoKern65 at SAP. He will rewrite the JVMTI handler >> code, which will make this point moot. See >> https://bugs.openjdk.org/browse/JDK-8320890. > >> > > >> > >> > >> > @tstuefe 3rd parameter to pass the either of 2 things: >> > ``` >> > 1. The JvmTiAgent object "agent", so that after shifting the >> > save_library_signature to os_aix,we can still access the agent object.-> >> > For this i tried importing jvm/prims header file, but i get segmentation >> > faults during build . Not sure if i am doing it the right way. >> > >> > 2. Pass a character buffer(and not const char*) where we copy the modified >> > filename back to it and then use it in jvmAgent. code. >> > ``` >> >> Does not sound really appealing tbh. We pile one hack atop of another. >> >> Please synchronize with @JoKern65 at SAP. He will rewrite the JVMTI handler >> code, which will make this point moot. See >> https://bugs.openjdk.org/browse/JDK-8320890. > > Hi @tstuefe Should i then wait for this code to be integrated and then > rewrite the .a handling ? > I mean this PR shall remain open then right ? > @JoKern65 Are you even handling the .a handling case ? i would like this PR > to stay open. Maybe i can wait for the design change that you are working on. > Hi @suchismith1993 , you can place this change on top of #16920 by comparing > it with branch origin/pr/16920 instead of master. This way you might be able > to proceed with your change. But as Thomas says you can only push if you have > appropriate reviews. Hi @GoeLin I am not sure how to do that . Could you tell me in brief ? Do I run the checkout command on the other PR and then place my change of top of it ? - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1862802768
Re: RFR: JDK-8320005 : Native library suffix impact on hotspot code in AIX [v2]
On Tue, 28 Nov 2023 12:59:01 GMT, Suchismith Roy wrote: >>> > >>> >>> @tstuefe 3rd parameter to pass the either of 2 things: >>> >>> 1. The JvmTiAgent object "agent", so that after shifting the >>> save_library_signature to os_aix,we can still access the agent object.-> >>> For this i tried importing jvm/prims header file, but i get segmentation >>> faults during build . Not sure if i am doing it the right way. >>> >>> 2. Pass a character buffer(and not const char*) where we copy the >>> modified filename back to it and then use it in jvmAgent. code. >> >> Does not sound really appealing tbh. We pile one hack atop of another. >> >> Please synchronize with @JoKern65 at SAP. He will rewrite the JVMTI handler >> code, which will make this point moot. See >> https://bugs.openjdk.org/browse/JDK-8320890. > >> > > >> > >> > >> > @tstuefe 3rd parameter to pass the either of 2 things: >> > ``` >> > 1. The JvmTiAgent object "agent", so that after shifting the >> > save_library_signature to os_aix,we can still access the agent object.-> >> > For this i tried importing jvm/prims header file, but i get segmentation >> > faults during build . Not sure if i am doing it the right way. >> > >> > 2. Pass a character buffer(and not const char*) where we copy the modified >> > filename back to it and then use it in jvmAgent. code. >> > ``` >> >> Does not sound really appealing tbh. We pile one hack atop of another. >> >> Please synchronize with @JoKern65 at SAP. He will rewrite the JVMTI handler >> code, which will make this point moot. See >> https://bugs.openjdk.org/browse/JDK-8320890. > > Hi @tstuefe Should i then wait for this code to be integrated and then > rewrite the .a handling ? > I mean this PR shall remain open then right ? > @JoKern65 Are you even handling the .a handling case ? i would like this PR > to stay open. Maybe i can wait for the design change that you are working on. Hi @suchismith1993 , you can place this change on top of #16920 by comparing it with branch origin/pr/16920 instead of master. This way you might be able to proceed with your change. But as Thomas says you can only push if you have appropriate reviews. - PR Comment: https://git.openjdk.org/jdk/pull/16604#issuecomment-1862791022
Re: RFR: JDK-8320890: [AIX] Find a better way to mimic dl handle equality [v4]
On Tue, 19 Dec 2023 12:52:23 GMT, Thomas Stuefe wrote: >> Hi @JoKern65 Is this good to integrate now ? > > @suchismith1993 > >> Once this code goes in I can push in my changes. We are targeting the fix >> for January. > > If you talk about https://github.com/openjdk/jdk/pull/16604, no, you cannot > push that even if Joachim finishes his work. > > Your patch has not even a single review, is quite controversial, and none of > the issues the reviewers have mentioned are addressed. This needs a lot more > discussion time. > > @tstuefe Sorry to tag you. Can you review the code. Once this code goes in > > I can push in my changes. > > We are targeting the fix for January. > > > Hi @JoKern65 Is this good to integrate now ? > > @suchismith1993 Please don't put pressure on patch authors and developers. > There is zero reason why this patch should be rushed. > > > Hi @suchismith1993, I'm waiting for a second review. Complex hotspot > > changes should be reviewed twice. > > Not only that, hotspot changes _need_ to be reviewed by at least two > reviewers. That is not optional. See OpenJDK bylaws. Sorry about that. The fix was critical for the adoptium builds and hence was looking to fix this soon. - PR Comment: https://git.openjdk.org/jdk/pull/16920#issuecomment-1862776678
Re: RFR: JDK-8320890: [AIX] Find a better way to mimic dl handle equality [v4]
On Tue, 19 Dec 2023 12:37:33 GMT, Suchismith Roy wrote: >> The libpath parsing code is from me, so no license problems. > > Hi @JoKern65 Is this good to integrate now ? > @suchismith1993 > > > Once this code goes in I can push in my changes. We are targeting the fix > > for January. > > If you talk about #16604, no, you cannot push that even if Joachim finishes > his work. > > Your patch has not even a single review, is quite controversial, and none of > the issues the reviewers have mentioned are addressed. This needs a lot more > discussion time. I have the patch ready based on the changes in this patch, as I take the diff and apply. But I cannot push since it will end up adding the entire file. - PR Comment: https://git.openjdk.org/jdk/pull/16920#issuecomment-1862768974
Re: RFR: JDK-8320890: [AIX] Find a better way to mimic dl handle equality [v4]
On Tue, 19 Dec 2023 12:37:33 GMT, Suchismith Roy wrote: >> The libpath parsing code is from me, so no license problems. > > Hi @JoKern65 Is this good to integrate now ? @suchismith1993 > Once this code goes in I can push in my changes. We are targeting the fix for > January. If you talk about https://github.com/openjdk/jdk/pull/16604, no, you cannot push that even if Joachim finishes his work. Your patch has not even a single review, is quite controversial, and none of the issues the reviewers have mentioned are addressed. This needs a lot more discussion time. - PR Comment: https://git.openjdk.org/jdk/pull/16920#issuecomment-1862704694
Re: RFR: JDK-8320890: [AIX] Find a better way to mimic dl handle equality [v6]
On Mon, 18 Dec 2023 13:33:46 GMT, Thomas Stuefe wrote: >> Joachim Kern has updated the pull request incrementally with one additional >> commit since the last revision: >> >> Followed Thomas proposals > > Well done. > > Releasing the mutex before asserting is not necessary; we don't pull the > handle table lock as part of error reporting. > @tstuefe Sorry to tag you. Can you review the code. Once this code goes in I > can push in my changes. We are targeting the fix for January. > Hi @JoKern65 Is this good to integrate now ? @suchismith1993 Please don't put pressure on patch authors and developers. There is zero reason why this patch should be rushed. > Hi @suchismith1993, I'm waiting for a second review. Complex hotspot changes > should be reviewed twice. Not only that, hotspot changes *need* to be reviewed by at least two reviewers. That is not optional. See OpenJDK bylaws. - PR Comment: https://git.openjdk.org/jdk/pull/16920#issuecomment-1862695052
Re: RFR: JDK-8320890: [AIX] Find a better way to mimic dl handle equality [v4]
On Tue, 19 Dec 2023 12:37:33 GMT, Suchismith Roy wrote: >> The libpath parsing code is from me, so no license problems. > > Hi @JoKern65 Is this good to integrate now ? Hi @suchismith1993, I'm waiting for a second review. Complex hotspot changes should be reviewed twice. - PR Comment: https://git.openjdk.org/jdk/pull/16920#issuecomment-1862690708
Re: RFR: JDK-8320890: [AIX] Find a better way to mimic dl handle equality [v4]
On Fri, 15 Dec 2023 11:51:43 GMT, Joachim Kern wrote: >> Joachim Kern has updated the pull request incrementally with one additional >> commit since the last revision: >> >> followed the proposals > > The libpath parsing code is from me, so no license problems. Hi @JoKern65 Is this good to integrate now ? - PR Comment: https://git.openjdk.org/jdk/pull/16920#issuecomment-1862684040
Re: RFR: 8320707: Virtual thread test updates
On Mon, 18 Dec 2023 20:37:29 GMT, Chris Plummer wrote: > I'm working on a test where I just added a CountDownLatch(1) and am wondering > if I should do the same, but I'm not sure if there is something about these > tests that is motivating the change. CountDownLatch is great for many tests. It's not as powerful as a Phaser of course but good enough and usually easy to understand quickly what is going on. However, for tests that are testing thread state then you often want to have as few dependencies as possible. In the case of CountDownlatch, the await method may park. The countDown method may have to unpark waiters for for virtual threads it means potentially parking (to queue a task) when unparking, so temporary transitions that JVMTI has to be concerned with. The other thing is keeping tests simple/consistent, it can be hard to maintain tests where one test method coordinates with one approach, another test method does something different. - PR Comment: https://git.openjdk.org/jdk/pull/17136#issuecomment-1862390382
