Re: RFR: 8303814: getLastErrorString should avoid charset conversions
On Wed, 8 Mar 2023 11:30:27 GMT, Daniel Jeliński wrote:
> This patch modifies the `getLastErrorString` method to return a `jstring`.
> Thanks to that we can avoid unnecessary back and forth conversions between
> Unicode and other charsets on Windows.
>
> Other changes include:
> - the Windows implementation of `getLastErrorString` no longer checks
> `errno`. I verified all uses of the method and confirmed that `errno` is not
> used anywhere.
> - While at it, I found and fixed a few calls to
> `JNU_ThrowIOExceptionWithLastError` that were done in context where
> `LastError` was not set.
> - jdk.hotspot.agent was modified to use `JNU_ThrowByNameWithLastError` and
> `JNU_ThrowByName` instead of `getLastErrorString`; the code is expected to
> have identical behavior.
> - zip_util was modified to return static messages instead of generated ones.
> The generated messages were not observed anywhere, because they were replaced
> by a static message in ZIP_Open, which is the only method used by other
> native code.
> - `getLastErrorString` is no longer exported by libjava.
>
> Tier1-3 tests continue to pass.
>
> No new automated regression test; testing this requires installing a language
> pack that cannot be displayed in the current code page.
> Tested this manually by installing Chinese language pack on English Windows
> 11, selecting Chinese language, then checking if the message on exception
> thrown by `InetAddress.getByName("nonexistent.local");` starts with
> `"不知道这样的主机。"` (or
> `"\u4e0d\u77e5\u9053\u8fd9\u6837\u7684\u4e3b\u673a\u3002"`). Without the
> change, the exception message started with a row of question marks.
I'm approving the SA changes. Thanks for the testing.
-
Marked as reviewed by cjplummer (Reviewer).
PR: https://git.openjdk.org/jdk/pull/12922
Re: RFR: 8303908: Add missing check in VTMS_transition_disable_for_all() for suspend mode
On Thu, 9 Mar 2023 18:55:06 GMT, Patricio Chilano Mateo
wrote:
> Please review this small fix. A suspender is a JvmtiVTMSTransitionDisabler
> monopolist, meaning VTMS_transition_disable_for_all() should not return while
> there is any active jvmtiVTMSTransitionDisabler. The code though is checking
> for active "all-disablers" but it's missing the check for active "single
> disablers".
> I attached a simple reproducer to the bug which I used to test the patch. Not
> sure if it was worth adding a test so the patch contains just the fix.
>
> Thanks,
> Patricio
src/hotspot/share/prims/jvmtiThreadState.cpp line 372:
> 370: java_lang_Thread::dec_VTMS_transition_disable_count(vth());
> 371: Atomic::dec(&_VTMS_transition_disable_for_one_count);
> 372: if (_VTMS_transition_disable_for_one_count == 0 || _is_SR) {
Sorry I don't understand why this `_is_SR` check was removed. I admit I can't
really figure out what this field means anyway, but there is nothing in the
issue description that suggests this also needs changing - and it is now
different to `VTMS_transition_enable_for_all`.
-
PR: https://git.openjdk.org/jdk/pull/12956
Re: RFR: 8303908: Add missing check in VTMS_transition_disable_for_all() for suspend mode
On Thu, 9 Mar 2023 18:55:06 GMT, Patricio Chilano Mateo wrote: > Please review this small fix. A suspender is a JvmtiVTMSTransitionDisabler > monopolist, meaning VTMS_transition_disable_for_all() should not return while > there is any active jvmtiVTMSTransitionDisabler. The code though is checking > for active "all-disablers" but it's missing the check for active "single > disablers". > I attached a simple reproducer to the bug which I used to test the patch. Not > sure if it was worth adding a test so the patch contains just the fix. > > Thanks, > Patricio This looks good. Thank you for catching and taking care about it! Serguei - Marked as reviewed by sspitsyn (Reviewer). PR: https://git.openjdk.org/jdk/pull/12956
Re: RFR: 8291555: Implement alternative fast-locking scheme [v16]
On Thu, 9 Mar 2023 21:08:16 GMT, Roman Kennke wrote: >> This change adds a fast-locking scheme as an alternative to the current >> stack-locking implementation. It retains the advantages of stack-locking >> (namely fast locking in uncontended code-paths), while avoiding the overload >> of the mark word. That overloading causes massive problems with Lilliput, >> because it means we have to check and deal with this situation when trying >> to access the mark-word. And because of the very racy nature, this turns out >> to be very complex and would involve a variant of the inflation protocol to >> ensure that the object header is stable. (The current implementation of >> setting/fetching the i-hash provides a glimpse into the complexity). >> >> What the original stack-locking does is basically to push a stack-lock onto >> the stack which consists only of the displaced header, and CAS a pointer to >> this stack location into the object header (the lowest two header bits being >> 00 indicate 'stack-locked'). The pointer into the stack can then be used to >> identify which thread currently owns the lock. >> >> This change basically reverses stack-locking: It still CASes the lowest two >> header bits to 00 to indicate 'fast-locked' but does *not* overload the >> upper bits with a stack-pointer. Instead, it pushes the object-reference to >> a thread-local lock-stack. This is a new structure which is basically a >> small array of oops that is associated with each thread. Experience shows >> that this array typcially remains very small (3-5 elements). Using this lock >> stack, it is possible to query which threads own which locks. Most >> importantly, the most common question 'does the current thread own me?' is >> very quickly answered by doing a quick scan of the array. More complex >> queries like 'which thread owns X?' are not performed in very >> performance-critical paths (usually in code like JVMTI or deadlock >> detection) where it is ok to do more complex operations (and we already do). >> The lock-stack is also a new set of GC roots, and would be scanned during >> thread scanning, possibly concurrently, via the normal protocols. >> >> The lock-stack is grown when needed. This means that we need to check for >> potential overflow before attempting locking. When that is the case, locking >> fast-paths would call into the runtime to grow the stack and handle the >> locking. Compiled fast-paths (C1 and C2 on x86_64 and aarch64) do this check >> on method entry to avoid (possibly lots) of such checks at locking sites. >> >> In contrast to stack-locking, fast-locking does *not* support recursive >> locking (yet). When that happens, the fast-lock gets inflated to a full >> monitor. It is not clear if it is worth to add support for recursive >> fast-locking. >> >> One trouble is that when a contending thread arrives at a fast-locked >> object, it must inflate the fast-lock to a full monitor. Normally, we need >> to know the current owning thread, and record that in the monitor, so that >> the contending thread can wait for the current owner to properly exit the >> monitor. However, fast-locking doesn't have this information. What we do >> instead is to record a special marker ANONYMOUS_OWNER. When the thread that >> currently holds the lock arrives at monitorexit, and observes >> ANONYMOUS_OWNER, it knows it must be itself, fixes the owner to be itself, >> and then properly exits the monitor, and thus handing over to the contending >> thread. >> >> As an alternative, I considered to remove stack-locking altogether, and only >> use heavy monitors. In most workloads this did not show measurable >> regressions. However, in a few workloads, I have observed severe >> regressions. All of them have been using old synchronized Java collections >> (Vector, Stack), StringBuffer or similar code. The combination of two >> conditions leads to regressions without stack- or fast-locking: 1. The >> workload synchronizes on uncontended locks (e.g. single-threaded use of >> Vector or StringBuffer) and 2. The workload churns such locks. IOW, >> uncontended use of Vector, StringBuffer, etc as such is ok, but creating >> lots of such single-use, single-threaded-locked objects leads to massive >> ObjectMonitor churn, which can lead to a significant performance impact. But >> alas, such code exists, and we probably don't want to punish it if we can >> avoid it. >> >> This change enables to simplify (and speed-up!) a lot of code: >> >> - The inflation protocol is no longer necessary: we can directly CAS the >> (tagged) ObjectMonitor pointer to the object header. >> - Accessing the hashcode could now be done in the fastpath always, if the >> hashcode has been installed. Fast-locked headers can be used directly, for >> monitor-locked objects we can easily reach-through to the displaced header. >> This is safe because Java threads participate in monitor deflation protocol.
Re: RFR: 8291555: Implement alternative fast-locking scheme [v15]
On Wed, 8 Mar 2023 18:25:15 GMT, Roman Kennke wrote: >> This change adds a fast-locking scheme as an alternative to the current >> stack-locking implementation. It retains the advantages of stack-locking >> (namely fast locking in uncontended code-paths), while avoiding the overload >> of the mark word. That overloading causes massive problems with Lilliput, >> because it means we have to check and deal with this situation when trying >> to access the mark-word. And because of the very racy nature, this turns out >> to be very complex and would involve a variant of the inflation protocol to >> ensure that the object header is stable. (The current implementation of >> setting/fetching the i-hash provides a glimpse into the complexity). >> >> What the original stack-locking does is basically to push a stack-lock onto >> the stack which consists only of the displaced header, and CAS a pointer to >> this stack location into the object header (the lowest two header bits being >> 00 indicate 'stack-locked'). The pointer into the stack can then be used to >> identify which thread currently owns the lock. >> >> This change basically reverses stack-locking: It still CASes the lowest two >> header bits to 00 to indicate 'fast-locked' but does *not* overload the >> upper bits with a stack-pointer. Instead, it pushes the object-reference to >> a thread-local lock-stack. This is a new structure which is basically a >> small array of oops that is associated with each thread. Experience shows >> that this array typcially remains very small (3-5 elements). Using this lock >> stack, it is possible to query which threads own which locks. Most >> importantly, the most common question 'does the current thread own me?' is >> very quickly answered by doing a quick scan of the array. More complex >> queries like 'which thread owns X?' are not performed in very >> performance-critical paths (usually in code like JVMTI or deadlock >> detection) where it is ok to do more complex operations (and we already do). >> The lock-stack is also a new set of GC roots, and would be scanned during >> thread scanning, possibly concurrently, via the normal protocols. >> >> The lock-stack is grown when needed. This means that we need to check for >> potential overflow before attempting locking. When that is the case, locking >> fast-paths would call into the runtime to grow the stack and handle the >> locking. Compiled fast-paths (C1 and C2 on x86_64 and aarch64) do this check >> on method entry to avoid (possibly lots) of such checks at locking sites. >> >> In contrast to stack-locking, fast-locking does *not* support recursive >> locking (yet). When that happens, the fast-lock gets inflated to a full >> monitor. It is not clear if it is worth to add support for recursive >> fast-locking. >> >> One trouble is that when a contending thread arrives at a fast-locked >> object, it must inflate the fast-lock to a full monitor. Normally, we need >> to know the current owning thread, and record that in the monitor, so that >> the contending thread can wait for the current owner to properly exit the >> monitor. However, fast-locking doesn't have this information. What we do >> instead is to record a special marker ANONYMOUS_OWNER. When the thread that >> currently holds the lock arrives at monitorexit, and observes >> ANONYMOUS_OWNER, it knows it must be itself, fixes the owner to be itself, >> and then properly exits the monitor, and thus handing over to the contending >> thread. >> >> As an alternative, I considered to remove stack-locking altogether, and only >> use heavy monitors. In most workloads this did not show measurable >> regressions. However, in a few workloads, I have observed severe >> regressions. All of them have been using old synchronized Java collections >> (Vector, Stack), StringBuffer or similar code. The combination of two >> conditions leads to regressions without stack- or fast-locking: 1. The >> workload synchronizes on uncontended locks (e.g. single-threaded use of >> Vector or StringBuffer) and 2. The workload churns such locks. IOW, >> uncontended use of Vector, StringBuffer, etc as such is ok, but creating >> lots of such single-use, single-threaded-locked objects leads to massive >> ObjectMonitor churn, which can lead to a significant performance impact. But >> alas, such code exists, and we probably don't want to punish it if we can >> avoid it. >> >> This change enables to simplify (and speed-up!) a lot of code: >> >> - The inflation protocol is no longer necessary: we can directly CAS the >> (tagged) ObjectMonitor pointer to the object header. >> - Accessing the hashcode could now be done in the fastpath always, if the >> hashcode has been installed. Fast-locked headers can be used directly, for >> monitor-locked objects we can easily reach-through to the displaced header. >> This is safe because Java threads participate in monitor deflation protocol.
Re: RFR: 8291555: Implement alternative fast-locking scheme [v16]
On Thu, 9 Mar 2023 21:08:16 GMT, Roman Kennke wrote: >> This change adds a fast-locking scheme as an alternative to the current >> stack-locking implementation. It retains the advantages of stack-locking >> (namely fast locking in uncontended code-paths), while avoiding the overload >> of the mark word. That overloading causes massive problems with Lilliput, >> because it means we have to check and deal with this situation when trying >> to access the mark-word. And because of the very racy nature, this turns out >> to be very complex and would involve a variant of the inflation protocol to >> ensure that the object header is stable. (The current implementation of >> setting/fetching the i-hash provides a glimpse into the complexity). >> >> What the original stack-locking does is basically to push a stack-lock onto >> the stack which consists only of the displaced header, and CAS a pointer to >> this stack location into the object header (the lowest two header bits being >> 00 indicate 'stack-locked'). The pointer into the stack can then be used to >> identify which thread currently owns the lock. >> >> This change basically reverses stack-locking: It still CASes the lowest two >> header bits to 00 to indicate 'fast-locked' but does *not* overload the >> upper bits with a stack-pointer. Instead, it pushes the object-reference to >> a thread-local lock-stack. This is a new structure which is basically a >> small array of oops that is associated with each thread. Experience shows >> that this array typcially remains very small (3-5 elements). Using this lock >> stack, it is possible to query which threads own which locks. Most >> importantly, the most common question 'does the current thread own me?' is >> very quickly answered by doing a quick scan of the array. More complex >> queries like 'which thread owns X?' are not performed in very >> performance-critical paths (usually in code like JVMTI or deadlock >> detection) where it is ok to do more complex operations (and we already do). >> The lock-stack is also a new set of GC roots, and would be scanned during >> thread scanning, possibly concurrently, via the normal protocols. >> >> The lock-stack is grown when needed. This means that we need to check for >> potential overflow before attempting locking. When that is the case, locking >> fast-paths would call into the runtime to grow the stack and handle the >> locking. Compiled fast-paths (C1 and C2 on x86_64 and aarch64) do this check >> on method entry to avoid (possibly lots) of such checks at locking sites. >> >> In contrast to stack-locking, fast-locking does *not* support recursive >> locking (yet). When that happens, the fast-lock gets inflated to a full >> monitor. It is not clear if it is worth to add support for recursive >> fast-locking. >> >> One trouble is that when a contending thread arrives at a fast-locked >> object, it must inflate the fast-lock to a full monitor. Normally, we need >> to know the current owning thread, and record that in the monitor, so that >> the contending thread can wait for the current owner to properly exit the >> monitor. However, fast-locking doesn't have this information. What we do >> instead is to record a special marker ANONYMOUS_OWNER. When the thread that >> currently holds the lock arrives at monitorexit, and observes >> ANONYMOUS_OWNER, it knows it must be itself, fixes the owner to be itself, >> and then properly exits the monitor, and thus handing over to the contending >> thread. >> >> As an alternative, I considered to remove stack-locking altogether, and only >> use heavy monitors. In most workloads this did not show measurable >> regressions. However, in a few workloads, I have observed severe >> regressions. All of them have been using old synchronized Java collections >> (Vector, Stack), StringBuffer or similar code. The combination of two >> conditions leads to regressions without stack- or fast-locking: 1. The >> workload synchronizes on uncontended locks (e.g. single-threaded use of >> Vector or StringBuffer) and 2. The workload churns such locks. IOW, >> uncontended use of Vector, StringBuffer, etc as such is ok, but creating >> lots of such single-use, single-threaded-locked objects leads to massive >> ObjectMonitor churn, which can lead to a significant performance impact. But >> alas, such code exists, and we probably don't want to punish it if we can >> avoid it. >> >> This change enables to simplify (and speed-up!) a lot of code: >> >> - The inflation protocol is no longer necessary: we can directly CAS the >> (tagged) ObjectMonitor pointer to the object header. >> - Accessing the hashcode could now be done in the fastpath always, if the >> hashcode has been installed. Fast-locked headers can be used directly, for >> monitor-locked objects we can easily reach-through to the displaced header. >> This is safe because Java threads participate in monitor deflation protocol.
Integrated: 8302779: HelidonAppTest.java fails with "assert(_cb == CodeCache::find_blob(pc())) failed: Must be the same" or SIGSEGV
On Tue, 7 Mar 2023 22:14:39 GMT, Patricio Chilano Mateo wrote: > Please review the following fix. The Method instance representing > Continuation.enterSpecial() is replaced by a new Method during redefinition > of the Continuation class. The already existing nmethod for it is not used, > but a new one will be generated the first time enterSpecial() is resolved > after redefinition. This means we could have more than one nmethod > representing enterSpecial(), in particular, one generated before redefinition > took place, and one after it. Now, when walking the stack, if we found a > return barrier pc (as in Continuation::is_return_barrier_entry()) and we want > to keep walking the physical stack then we know the sender will be the > enterSpecial frame so we create it by calling ContinuationEntry::to_frame(). > This method assumes there can only be one nmethod associated with > enterSpecial() so we hit an assert later on. See the bug for more details of > the crash. > > As I mentioned in the bug we don't need to rely on this assumption since we > can re-read the updated value from _enter_special. But reading both > _enter_special and _return_pc means we would need some kind of > synchronization since to_frame() could be called concurrently with > set_enter_code(). To avoid that we could just read _return_pc and calculate > the blob from it each time, but I'm also assuming that overhead is undesired > and that's why the static variable was introduced. Alternatively > _enter_special could be read and _return_pc could be derived from it (by > adding an extra field in the nmethod class). But if we go this route I think > we would need to do a small fix on thaw too. After redefinition and before a > new call to resolve enterSpecial(), the last thaw call for some virtual > thread would create an entry frame with an old _return_pc (see > ThawBase::new_entry_frame() and ThawBase::patch_return()). I'm not sure about > the lifecycle of the old CodeBlob but it seems it could have bee n already removed if enterSpecial was not found while traversing everybody's stack. Maybe there are more issues. > > The simple solution implemented here is just to disallow redefinition of the > Continuation class altogether. Another less restrictive option would be to > keep the already generated enterSpecial nmethod, if there is one. I can also > investigate one of the routes mentioned previously if desired. > > I tested the fix with the simple reproducer I added to the bug and also with > the previously crashing HelidonAppTest.java test. > > Thanks, > Patricio This pull request has now been integrated. Changeset: 8b740b46 Author:Patricio Chilano Mateo URL: https://git.openjdk.org/jdk/commit/8b740b46091c853c7cb66c361deda6dfbb2cedc8 Stats: 4 lines in 1 file changed: 4 ins; 0 del; 0 mod 8302779: HelidonAppTest.java fails with "assert(_cb == CodeCache::find_blob(pc())) failed: Must be the same" or SIGSEGV Reviewed-by: coleenp, sspitsyn - PR: https://git.openjdk.org/jdk/pull/12911
Re: RFR: 8302779: HelidonAppTest.java fails with "assert(_cb == CodeCache::find_blob(pc())) failed: Must be the same" or SIGSEGV
On Thu, 9 Mar 2023 22:24:00 GMT, Serguei Spitsyn wrote: > Looks good. Thank you for taking care about it! Serguei, Thanks > Thanks for the review Serguei! - PR: https://git.openjdk.org/jdk/pull/12911
Re: RFR: 8302779: HelidonAppTest.java fails with "assert(_cb == CodeCache::find_blob(pc())) failed: Must be the same" or SIGSEGV
On Tue, 7 Mar 2023 22:14:39 GMT, Patricio Chilano Mateo wrote: > Please review the following fix. The Method instance representing > Continuation.enterSpecial() is replaced by a new Method during redefinition > of the Continuation class. The already existing nmethod for it is not used, > but a new one will be generated the first time enterSpecial() is resolved > after redefinition. This means we could have more than one nmethod > representing enterSpecial(), in particular, one generated before redefinition > took place, and one after it. Now, when walking the stack, if we found a > return barrier pc (as in Continuation::is_return_barrier_entry()) and we want > to keep walking the physical stack then we know the sender will be the > enterSpecial frame so we create it by calling ContinuationEntry::to_frame(). > This method assumes there can only be one nmethod associated with > enterSpecial() so we hit an assert later on. See the bug for more details of > the crash. > > As I mentioned in the bug we don't need to rely on this assumption since we > can re-read the updated value from _enter_special. But reading both > _enter_special and _return_pc means we would need some kind of > synchronization since to_frame() could be called concurrently with > set_enter_code(). To avoid that we could just read _return_pc and calculate > the blob from it each time, but I'm also assuming that overhead is undesired > and that's why the static variable was introduced. Alternatively > _enter_special could be read and _return_pc could be derived from it (by > adding an extra field in the nmethod class). But if we go this route I think > we would need to do a small fix on thaw too. After redefinition and before a > new call to resolve enterSpecial(), the last thaw call for some virtual > thread would create an entry frame with an old _return_pc (see > ThawBase::new_entry_frame() and ThawBase::patch_return()). I'm not sure about > the lifecycle of the old CodeBlob but it seems it could have bee n already removed if enterSpecial was not found while traversing everybody's stack. Maybe there are more issues. > > The simple solution implemented here is just to disallow redefinition of the > Continuation class altogether. Another less restrictive option would be to > keep the already generated enterSpecial nmethod, if there is one. I can also > investigate one of the routes mentioned previously if desired. > > I tested the fix with the simple reproducer I added to the bug and also with > the previously crashing HelidonAppTest.java test. > > Thanks, > Patricio Looks good. Thank you for taking care about it! Serguei, Thanks - Marked as reviewed by sspitsyn (Reviewer). PR: https://git.openjdk.org/jdk/pull/12911
Re: RFR: 8291555: Implement alternative fast-locking scheme [v16]
On Thu, 9 Mar 2023 21:08:16 GMT, Roman Kennke wrote: >> This change adds a fast-locking scheme as an alternative to the current >> stack-locking implementation. It retains the advantages of stack-locking >> (namely fast locking in uncontended code-paths), while avoiding the overload >> of the mark word. That overloading causes massive problems with Lilliput, >> because it means we have to check and deal with this situation when trying >> to access the mark-word. And because of the very racy nature, this turns out >> to be very complex and would involve a variant of the inflation protocol to >> ensure that the object header is stable. (The current implementation of >> setting/fetching the i-hash provides a glimpse into the complexity). >> >> What the original stack-locking does is basically to push a stack-lock onto >> the stack which consists only of the displaced header, and CAS a pointer to >> this stack location into the object header (the lowest two header bits being >> 00 indicate 'stack-locked'). The pointer into the stack can then be used to >> identify which thread currently owns the lock. >> >> This change basically reverses stack-locking: It still CASes the lowest two >> header bits to 00 to indicate 'fast-locked' but does *not* overload the >> upper bits with a stack-pointer. Instead, it pushes the object-reference to >> a thread-local lock-stack. This is a new structure which is basically a >> small array of oops that is associated with each thread. Experience shows >> that this array typcially remains very small (3-5 elements). Using this lock >> stack, it is possible to query which threads own which locks. Most >> importantly, the most common question 'does the current thread own me?' is >> very quickly answered by doing a quick scan of the array. More complex >> queries like 'which thread owns X?' are not performed in very >> performance-critical paths (usually in code like JVMTI or deadlock >> detection) where it is ok to do more complex operations (and we already do). >> The lock-stack is also a new set of GC roots, and would be scanned during >> thread scanning, possibly concurrently, via the normal protocols. >> >> The lock-stack is grown when needed. This means that we need to check for >> potential overflow before attempting locking. When that is the case, locking >> fast-paths would call into the runtime to grow the stack and handle the >> locking. Compiled fast-paths (C1 and C2 on x86_64 and aarch64) do this check >> on method entry to avoid (possibly lots) of such checks at locking sites. >> >> In contrast to stack-locking, fast-locking does *not* support recursive >> locking (yet). When that happens, the fast-lock gets inflated to a full >> monitor. It is not clear if it is worth to add support for recursive >> fast-locking. >> >> One trouble is that when a contending thread arrives at a fast-locked >> object, it must inflate the fast-lock to a full monitor. Normally, we need >> to know the current owning thread, and record that in the monitor, so that >> the contending thread can wait for the current owner to properly exit the >> monitor. However, fast-locking doesn't have this information. What we do >> instead is to record a special marker ANONYMOUS_OWNER. When the thread that >> currently holds the lock arrives at monitorexit, and observes >> ANONYMOUS_OWNER, it knows it must be itself, fixes the owner to be itself, >> and then properly exits the monitor, and thus handing over to the contending >> thread. >> >> As an alternative, I considered to remove stack-locking altogether, and only >> use heavy monitors. In most workloads this did not show measurable >> regressions. However, in a few workloads, I have observed severe >> regressions. All of them have been using old synchronized Java collections >> (Vector, Stack), StringBuffer or similar code. The combination of two >> conditions leads to regressions without stack- or fast-locking: 1. The >> workload synchronizes on uncontended locks (e.g. single-threaded use of >> Vector or StringBuffer) and 2. The workload churns such locks. IOW, >> uncontended use of Vector, StringBuffer, etc as such is ok, but creating >> lots of such single-use, single-threaded-locked objects leads to massive >> ObjectMonitor churn, which can lead to a significant performance impact. But >> alas, such code exists, and we probably don't want to punish it if we can >> avoid it. >> >> This change enables to simplify (and speed-up!) a lot of code: >> >> - The inflation protocol is no longer necessary: we can directly CAS the >> (tagged) ObjectMonitor pointer to the object header. >> - Accessing the hashcode could now be done in the fastpath always, if the >> hashcode has been installed. Fast-locked headers can be used directly, for >> monitor-locked objects we can easily reach-through to the displaced header. >> This is safe because Java threads participate in monitor deflation protocol.
Integrated: 8303924: ProblemList serviceability/sa/UniqueVtableTest.java on Linux
On Thu, 9 Mar 2023 21:36:32 GMT, Alex Menkov wrote: > The test fails intermittently on linux-x64-debug and linux-aarch64-debug This pull request has now been integrated. Changeset: e930b63a Author:Alex Menkov URL: https://git.openjdk.org/jdk/commit/e930b63a8f166502740bca45e3d022f69fc04b53 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod 8303924: ProblemList serviceability/sa/UniqueVtableTest.java on Linux Reviewed-by: dcubed - PR: https://git.openjdk.org/jdk/pull/12962
Integrated: 8303609: ProblemList serviceability/sa/TestSysProps.java with ZGC
On Wed, 8 Mar 2023 22:40:56 GMT, Chris Plummer wrote: > Although it takes both ZGC and -Xcomp to cause the test to fail, we have no > way to problem list for just that combination, so I'm choosing the problem > list with just ZGC since it is the main cause of the failure. I've only seen > this issue on windows-x64, but there are clearly failures on linux and macos > in mach5, so I'm problem listing for all platforms. This pull request has now been integrated. Changeset: af0ca78a Author:Chris Plummer URL: https://git.openjdk.org/jdk/commit/af0ca78a8f8108fd81dcdfaa6b8a43a940942633 Stats: 2 lines in 1 file changed: 2 ins; 0 del; 0 mod 8303609: ProblemList serviceability/sa/TestSysProps.java with ZGC Reviewed-by: dcubed - PR: https://git.openjdk.org/jdk/pull/12935
Integrated: 8289765: JDI EventSet/resume/resume008 failed with "ERROR: suspendCounts don't match for : VirtualThread-unparker"
On Fri, 3 Mar 2023 18:16:25 GMT, Chris Plummer wrote: > The test failure is caused by the arrival of unexpected ThreadStartEvents, > which mess up the debugger side. The events are for threads we normally only > see getting created when using virtual threads, such as carrier threads and > the VirtualThread-unparker thread. Theoretically this issue could happen > without virtual threads due to other VM threads starting up such as > Common-Cleaner, but we haven't seen it fail for that reason yet. > > The test is testing proper thread suspension for ThreadStartEvent using each > of the 3 suspension policy types. The debuggee creates a sequence of 3 > debuggee threads, each one's timing coordinated with some complicated > synchronization with the debugger using breakpoints and the setting of fields > in the debuggee (and careful placement of suspend/resume in the debugger). > The ThreadStartRequests that the debugger sets up always use a "count filter" > of 1, which means the requests are good for delivering exactly 1 > ThreadStartEvent, and any that come after the first will get filtered out. So > when an an unexpected ThreadStartEvent arrives for something like a carrier > thread, this prematurely moves the debugger on to the next step, and the > synchronization with the debuggee gets messed up. > > The first step in fixing this test was to remove the count filter, so the > request can handle any number of ThreadStartEvents. > > The next step was then fixing the test library code in EventHandler.java so > it would filter out any undesired ThreadStartEvents, so the test just ends up > getting one event, and always for the thread it is expecting. There are a few > parts to this. One is improving EventFilters.filter() to filter out more > threads that tend to be created during VM startup, including carrier threads > and the VirtualThread-unparker thread. > > It was necessary to add some calls EventFilters.filter() from EventHandler. > This was done by adding a ThreadStartEvent listener for the "spurious" thread > starts (those the test debuggee does not create). This listener is added by > waitForRequestedEventCommon(), which is indirectly called by the test when is > calls waitForRequestedEventSet(). > > There is a also 2nd place where the ThreadStartEvent listener for "spurious" > threads is needed. It is now also installed with the default listeners that > are always in place. It is needed when the test is not actually waiting for a > ThreadStartEvent, but is waiting for a BreakpointEvent. > waitForRequestedEventCommon() is not used in this case (so none of its > listeners are installed), but the default listeners are always in place and > can be used to filter these ThreadStartEvents. Note this filter will also be > in place when calling waitForRequestedEventCommon(), but we can't realy on it > when waitForRequestedEventCommon() is used for ThreadStartEvents because the > spurious ThreadStartEvent will be seen and returned before we ever get to the > default filter. So we actually end up with this ThreadStartEvent listener > installed twice during waitForRequestedEventCommon(). > > I did a bit of cleanup on the test, mostly renaming of threads and > ThreadStartRequests so they are easier to match up with the iteration # we > use in both the debuggee and debugger (0, 1, and 2). The only real change in > the test itself is removing the filter count, and verifying that the > ThreadStartEvent is for the expected thread. This pull request has now been integrated. Changeset: 8b0eb729 Author:Chris Plummer URL: https://git.openjdk.org/jdk/commit/8b0eb7299a5d0e142453ed5c7a17308077e27993 Stats: 97 lines in 4 files changed: 73 ins; 1 del; 23 mod 8289765: JDI EventSet/resume/resume008 failed with "ERROR: suspendCounts don't match for : VirtualThread-unparker" Reviewed-by: sspitsyn, kevinw - PR: https://git.openjdk.org/jdk/pull/12861
Re: RFR: 8289765: JDI EventSet/resume/resume008 failed with "ERROR: suspendCounts don't match for : VirtualThread-unparker" [v2]
On Wed, 8 Mar 2023 22:39:55 GMT, Chris Plummer wrote: >> The test failure is caused by the arrival of unexpected ThreadStartEvents, >> which mess up the debugger side. The events are for threads we normally only >> see getting created when using virtual threads, such as carrier threads and >> the VirtualThread-unparker thread. Theoretically this issue could happen >> without virtual threads due to other VM threads starting up such as >> Common-Cleaner, but we haven't seen it fail for that reason yet. >> >> The test is testing proper thread suspension for ThreadStartEvent using each >> of the 3 suspension policy types. The debuggee creates a sequence of 3 >> debuggee threads, each one's timing coordinated with some complicated >> synchronization with the debugger using breakpoints and the setting of >> fields in the debuggee (and careful placement of suspend/resume in the >> debugger). The ThreadStartRequests that the debugger sets up always use a >> "count filter" of 1, which means the requests are good for delivering >> exactly 1 ThreadStartEvent, and any that come after the first will get >> filtered out. So when an an unexpected ThreadStartEvent arrives for >> something like a carrier thread, this prematurely moves the debugger on to >> the next step, and the synchronization with the debuggee gets messed up. >> >> The first step in fixing this test was to remove the count filter, so the >> request can handle any number of ThreadStartEvents. >> >> The next step was then fixing the test library code in EventHandler.java so >> it would filter out any undesired ThreadStartEvents, so the test just ends >> up getting one event, and always for the thread it is expecting. There are a >> few parts to this. One is improving EventFilters.filter() to filter out more >> threads that tend to be created during VM startup, including carrier threads >> and the VirtualThread-unparker thread. >> >> It was necessary to add some calls EventFilters.filter() from EventHandler. >> This was done by adding a ThreadStartEvent listener for the "spurious" >> thread starts (those the test debuggee does not create). This listener is >> added by waitForRequestedEventCommon(), which is indirectly called by the >> test when is calls waitForRequestedEventSet(). >> >> There is a also 2nd place where the ThreadStartEvent listener for "spurious" >> threads is needed. It is now also installed with the default listeners that >> are always in place. It is needed when the test is not actually waiting for >> a ThreadStartEvent, but is waiting for a BreakpointEvent. >> waitForRequestedEventCommon() is not used in this case (so none of its >> listeners are installed), but the default listeners are always in place and >> can be used to filter these ThreadStartEvents. Note this filter will also be >> in place when calling waitForRequestedEventCommon(), but we can't realy on >> it when waitForRequestedEventCommon() is used for ThreadStartEvents because >> the spurious ThreadStartEvent will be seen and returned before we ever get >> to the default filter. So we actually end up with this ThreadStartEvent >> listener installed twice during waitForRequestedEventCommon(). >> >> I did a bit of cleanup on the test, mostly renaming of threads and >> ThreadStartRequests so they are easier to match up with the iteration # we >> use in both the debuggee and debugger (0, 1, and 2). The only real change in >> the test itself is removing the filter count, and verifying that the >> ThreadStartEvent is for the expected thread. > > Chris Plummer has updated the pull request incrementally with one additional > commit since the last revision: > > Fix a couple of typos Thanks Kevin and Serguei! - PR: https://git.openjdk.org/jdk/pull/12861
Re: RFR: 8303609: ProblemList serviceability/sa/TestSysProps.java with ZGC
On Wed, 8 Mar 2023 22:40:56 GMT, Chris Plummer wrote: > Although it takes both ZGC and -Xcomp to cause the test to fail, we have no > way to problem list for just that combination, so I'm choosing the problem > list with just ZGC since it is the main cause of the failure. I've only seen > this issue on windows-x64, but there are clearly failures on linux and macos > in mach5, so I'm problem listing for all platforms. Thanks Dan! - PR: https://git.openjdk.org/jdk/pull/12935
RFR: 8303924: ProblemList serviceability/sa/UniqueVtableTest.java on Linux
The test fails intermittently on linux-x64-debug and linux-aarch64-debug - Commit messages: - problemlist UniqueVtableTest on linux Changes: https://git.openjdk.org/jdk/pull/12962/files Webrev: https://webrevs.openjdk.org/?repo=jdk=12962=00 Issue: https://bugs.openjdk.org/browse/JDK-8303924 Stats: 1 line in 1 file changed: 1 ins; 0 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/12962.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12962/head:pull/12962 PR: https://git.openjdk.org/jdk/pull/12962
Re: RFR: 8303924: ProblemList serviceability/sa/UniqueVtableTest.java on Linux
On Thu, 9 Mar 2023 21:36:32 GMT, Alex Menkov wrote: > The test fails intermittently on linux-x64-debug and linux-aarch64-debug Thumbs up. This is a trivial fix. - Marked as reviewed by dcubed (Reviewer). PR: https://git.openjdk.org/jdk/pull/12962
Re: RFR: 8301995: Move invokedynamic resolution information out of ConstantPoolCacheEntry [v2]
> The current structure used to store the resolution information for > invokedynamic, ConstantPoolCacheEntry, is difficult to interpret due to its > ambigious fields f1 and f2. This structure can hold information for fields, > methods, and invokedynamics and each of its fields can hold different types > of values depending on the entry. > > This enhancement proposes a new structure to exclusively contain > invokedynamic information in a manner that is easy to interpret and easy to > extend. Resolved invokedynamic entries will be stored in an array in the > constant pool cache and the operand of the invokedynamic bytecode will be > rewritten to be the index into this array. > > Any areas that previously accessed invokedynamic data from > ConstantPoolCacheEntry will be replaced with accesses to this new array and > structure. Verified with tier1-9 tests. > > The PPC was provided by @reinrich and the RISCV port was provided by > @DingliZhang and @zifeihan. > > This change supports the following platforms: x86, aarch64, PPC, and RISCV Matias Saavedra Silva has updated the pull request incrementally with one additional commit since the last revision: Interpreter optimization and comments - Changes: - all: https://git.openjdk.org/jdk/pull/12778/files - new: https://git.openjdk.org/jdk/pull/12778/files/829517d6..c2d87e59 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=12778=01 - incr: https://webrevs.openjdk.org/?repo=jdk=12778=00-01 Stats: 47 lines in 10 files changed: 11 ins; 13 del; 23 mod Patch: https://git.openjdk.org/jdk/pull/12778.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12778/head:pull/12778 PR: https://git.openjdk.org/jdk/pull/12778
Re: RFR: 8292818: replace 96-bit representation for field metadata with variable-sized streams
On Wed, 8 Mar 2023 16:05:57 GMT, Coleen Phillimore wrote: >> Please review this change re-implementing the FieldInfo data structure. >> >> The FieldInfo array is an old data structure storing fields metadata. It has >> poor extension capabilities, a complex management code because of lack of >> strong typing and semantic overloading, and a poor memory efficiency. >> >> The new implementation uses a compressed stream to store those metadata, >> achieving better memory density and providing flexible extensibility, while >> exposing a strongly typed set of data when uncompressed. The stream is >> compressed using the unsigned5 encoding, which alreay present in the JDK >> (because of pack200) and the JVM (because JIT compulers use it to comrpess >> debugging information). >> >> More technical details are available in the CR: >> https://bugs.openjdk.org/browse/JDK-8292818 >> >> Those changes include a re-organisation of fields' flags, splitting the >> previous heterogeneous AccessFlags field into three distincts flag >> categories: immutable flags from the class file, immutable fields defined by >> the JVM, and finally mutable flags defined by the JVM. >> >> The SA, CI, and JVMCI, which all used to access the old FieldInfo array, >> have been updated too to deal with the new FieldInfo format. >> >> Tested with mach5, tier 1 to 7. >> >> Thank you. > > src/hotspot/share/classfile/fieldLayoutBuilder.cpp line 554: > >> 552: FieldInfo ctrl = _field_info->at(0); >> 553: FieldGroup* group = nullptr; >> 554: FieldInfo tfi = *it; > > What's the 't' in tfi? Maybe a longer variable name would be helpful here. At some point there was a TempFieldInfo type, hence the name. Renamed to fieldinfo. > src/hotspot/share/classfile/javaClasses.cpp line 871: > >> 869: // a new UNSIGNED5 stream, and substitute it to the old FieldInfo >> stream. >> 870: >> 871: int java_fields; > > Can you put InstanceKlass* ik = InstanceKlass::cast(k); here and use that so > there's only one cast? Sure, done. - PR: https://git.openjdk.org/jdk/pull/12855
Re: RFR: 8291555: Implement alternative fast-locking scheme [v16]
> This change adds a fast-locking scheme as an alternative to the current > stack-locking implementation. It retains the advantages of stack-locking > (namely fast locking in uncontended code-paths), while avoiding the overload > of the mark word. That overloading causes massive problems with Lilliput, > because it means we have to check and deal with this situation when trying to > access the mark-word. And because of the very racy nature, this turns out to > be very complex and would involve a variant of the inflation protocol to > ensure that the object header is stable. (The current implementation of > setting/fetching the i-hash provides a glimpse into the complexity). > > What the original stack-locking does is basically to push a stack-lock onto > the stack which consists only of the displaced header, and CAS a pointer to > this stack location into the object header (the lowest two header bits being > 00 indicate 'stack-locked'). The pointer into the stack can then be used to > identify which thread currently owns the lock. > > This change basically reverses stack-locking: It still CASes the lowest two > header bits to 00 to indicate 'fast-locked' but does *not* overload the upper > bits with a stack-pointer. Instead, it pushes the object-reference to a > thread-local lock-stack. This is a new structure which is basically a small > array of oops that is associated with each thread. Experience shows that this > array typcially remains very small (3-5 elements). Using this lock stack, it > is possible to query which threads own which locks. Most importantly, the > most common question 'does the current thread own me?' is very quickly > answered by doing a quick scan of the array. More complex queries like 'which > thread owns X?' are not performed in very performance-critical paths (usually > in code like JVMTI or deadlock detection) where it is ok to do more complex > operations (and we already do). The lock-stack is also a new set of GC roots, > and would be scanned during thread scanning, possibly concurrently, via the > normal p rotocols. > > The lock-stack is grown when needed. This means that we need to check for > potential overflow before attempting locking. When that is the case, locking > fast-paths would call into the runtime to grow the stack and handle the > locking. Compiled fast-paths (C1 and C2 on x86_64 and aarch64) do this check > on method entry to avoid (possibly lots) of such checks at locking sites. > > In contrast to stack-locking, fast-locking does *not* support recursive > locking (yet). When that happens, the fast-lock gets inflated to a full > monitor. It is not clear if it is worth to add support for recursive > fast-locking. > > One trouble is that when a contending thread arrives at a fast-locked object, > it must inflate the fast-lock to a full monitor. Normally, we need to know > the current owning thread, and record that in the monitor, so that the > contending thread can wait for the current owner to properly exit the > monitor. However, fast-locking doesn't have this information. What we do > instead is to record a special marker ANONYMOUS_OWNER. When the thread that > currently holds the lock arrives at monitorexit, and observes > ANONYMOUS_OWNER, it knows it must be itself, fixes the owner to be itself, > and then properly exits the monitor, and thus handing over to the contending > thread. > > As an alternative, I considered to remove stack-locking altogether, and only > use heavy monitors. In most workloads this did not show measurable > regressions. However, in a few workloads, I have observed severe regressions. > All of them have been using old synchronized Java collections (Vector, > Stack), StringBuffer or similar code. The combination of two conditions leads > to regressions without stack- or fast-locking: 1. The workload synchronizes > on uncontended locks (e.g. single-threaded use of Vector or StringBuffer) and > 2. The workload churns such locks. IOW, uncontended use of Vector, > StringBuffer, etc as such is ok, but creating lots of such single-use, > single-threaded-locked objects leads to massive ObjectMonitor churn, which > can lead to a significant performance impact. But alas, such code exists, and > we probably don't want to punish it if we can avoid it. > > This change enables to simplify (and speed-up!) a lot of code: > > - The inflation protocol is no longer necessary: we can directly CAS the > (tagged) ObjectMonitor pointer to the object header. > - Accessing the hashcode could now be done in the fastpath always, if the > hashcode has been installed. Fast-locked headers can be used directly, for > monitor-locked objects we can easily reach-through to the displaced header. > This is safe because Java threads participate in monitor deflation protocol. > This would be implemented in a separate PR > > > Testing: > - [x] tier1 x86_64 x aarch64 x +UseFastLocking > - [x] tier2
Re: RFR: 8292818: replace 96-bit representation for field metadata with variable-sized streams
On Wed, 8 Mar 2023 15:50:05 GMT, Coleen Phillimore wrote: >> Please review this change re-implementing the FieldInfo data structure. >> >> The FieldInfo array is an old data structure storing fields metadata. It has >> poor extension capabilities, a complex management code because of lack of >> strong typing and semantic overloading, and a poor memory efficiency. >> >> The new implementation uses a compressed stream to store those metadata, >> achieving better memory density and providing flexible extensibility, while >> exposing a strongly typed set of data when uncompressed. The stream is >> compressed using the unsigned5 encoding, which alreay present in the JDK >> (because of pack200) and the JVM (because JIT compulers use it to comrpess >> debugging information). >> >> More technical details are available in the CR: >> https://bugs.openjdk.org/browse/JDK-8292818 >> >> Those changes include a re-organisation of fields' flags, splitting the >> previous heterogeneous AccessFlags field into three distincts flag >> categories: immutable flags from the class file, immutable fields defined by >> the JVM, and finally mutable flags defined by the JVM. >> >> The SA, CI, and JVMCI, which all used to access the old FieldInfo array, >> have been updated too to deal with the new FieldInfo format. >> >> Tested with mach5, tier 1 to 7. >> >> Thank you. > > src/hotspot/share/classfile/classFileParser.cpp line 1608: > >> 1606: fflags.update_injected(true); >> 1607: AccessFlags aflags; >> 1608: FieldInfo fi(aflags, (u2)(injected[n].name_index), >> (u2)(injected[n].signature_index), 0, fflags); > > I don't know why there's a cast here until I read more. If the FieldInfo > name_index and signature_index fields are only u2 sized, could you pass this > as an int and then in the constructor assert that the value doesn't overflow > u2 instead? The type of name_index and signature_index is const vmSymbolID, because they names and signatures of injected fields do not come from a constant pool, but from the vmSymbol array. - PR: https://git.openjdk.org/jdk/pull/12855
Re: RFR: 8292818: replace 96-bit representation for field metadata with variable-sized streams
On Wed, 8 Mar 2023 15:46:12 GMT, Coleen Phillimore wrote: >> Please review this change re-implementing the FieldInfo data structure. >> >> The FieldInfo array is an old data structure storing fields metadata. It has >> poor extension capabilities, a complex management code because of lack of >> strong typing and semantic overloading, and a poor memory efficiency. >> >> The new implementation uses a compressed stream to store those metadata, >> achieving better memory density and providing flexible extensibility, while >> exposing a strongly typed set of data when uncompressed. The stream is >> compressed using the unsigned5 encoding, which alreay present in the JDK >> (because of pack200) and the JVM (because JIT compulers use it to comrpess >> debugging information). >> >> More technical details are available in the CR: >> https://bugs.openjdk.org/browse/JDK-8292818 >> >> Those changes include a re-organisation of fields' flags, splitting the >> previous heterogeneous AccessFlags field into three distincts flag >> categories: immutable flags from the class file, immutable fields defined by >> the JVM, and finally mutable flags defined by the JVM. >> >> The SA, CI, and JVMCI, which all used to access the old FieldInfo array, >> have been updated too to deal with the new FieldInfo format. >> >> Tested with mach5, tier 1 to 7. >> >> Thank you. > > src/hotspot/share/classfile/classFileParser.cpp line 1491: > >> 1489: _temp_field_info = new GrowableArray(total_fields); >> 1490: >> 1491: ResourceMark rm(THREAD); > > Is the ResourceMark ok here or should it go before allocating > _temp_field_info ? _temp_field_info must survive after ClassFileParser::parse_fields() has returned, so definitively after the allocation of _temp_field_info. That being said, I don't see any reason to have a ResourceMark here, probably a remain of some debugging/tracing code. I'll remove it. - PR: https://git.openjdk.org/jdk/pull/12855
Re: RFR: 8301995: Move invokedynamic resolution information out of ConstantPoolCacheEntry
On Thu, 9 Mar 2023 16:44:21 GMT, Richard Reingruber wrote: >> This looks cool. > >> This change should be in a further RFE though (and you can explain it there >> so we can maybe use it in the other platforms too). > > Ok. > >> Does it affect performance when generating the template interpreter? > > I didn't think it would affect performance if the interpreter is not printed. > I have not measured it though. > >> Do you need to have hsdis in the LD_LIBRARY_PATH environment variable to use >> this? I see it's already used by default in one place. > > Yes you do. It is not working with the AbstractDisassembler which produces a > hex dump of the machine code. I was searching in the wrong directory, which is why I didn't find this and apparently I reviewed this change in 2018. We can leave this change here. Sorry for the noise. - PR: https://git.openjdk.org/jdk/pull/12778
Re: RFR: 8301995: Move invokedynamic resolution information out of ConstantPoolCacheEntry
On Mon, 27 Feb 2023 21:37:34 GMT, Matias Saavedra Silva
wrote:
> The current structure used to store the resolution information for
> invokedynamic, ConstantPoolCacheEntry, is difficult to interpret due to its
> ambigious fields f1 and f2. This structure can hold information for fields,
> methods, and invokedynamics and each of its fields can hold different types
> of values depending on the entry.
>
> This enhancement proposes a new structure to exclusively contain
> invokedynamic information in a manner that is easy to interpret and easy to
> extend. Resolved invokedynamic entries will be stored in an array in the
> constant pool cache and the operand of the invokedynamic bytecode will be
> rewritten to be the index into this array.
>
> Any areas that previously accessed invokedynamic data from
> ConstantPoolCacheEntry will be replaced with accesses to this new array and
> structure. Verified with tier1-9 tests.
>
> The PPC was provided by @reinrich and the RISCV port was provided by
> @DingliZhang and @zifeihan.
>
> This change supports the following platforms: x86, aarch64, PPC, and RISCV
src/hotspot/cpu/aarch64/interp_masm_aarch64.cpp line 1844:
> 1842: // Scale the index to be the entry index *
> sizeof(ResolvedInvokeDynamicInfo)
> 1843: mov(tmp, sizeof(ResolvedIndyEntry));
> 1844: mul(index, index, tmp);
On 64bits platform, sizeof(ResolvedIndyEntry) is 16, a power of two, so shift
instruction could be used instead of a multiply instructions (with an assert in
case the size of ResolvedIndyEntry is changed).
src/hotspot/cpu/x86/interp_masm_x86.cpp line 2075:
> 2073: movptr(cache, Address(rbp, frame::interpreter_frame_cache_offset *
> wordSize));
> 2074: movptr(cache, Address(cache,
> in_bytes(ConstantPoolCache::invokedynamic_entries_offset(;
> 2075: imull(index, index, sizeof(ResolvedIndyEntry)); // Scale the index to
> be the entry index * sizeof(ResolvedInvokeDynamicInfo)
A shift instruction could be used when sizeof(ResolvedIndyEntry) is a power of
two. It is on x86_64 platforms but not on x86_32 platforms (both are using this
file).
Suggested change:
if (is_power_of_2(sizeof(ResolvedIndyEntry))) {
shll(index, log2i(sizeof(ResolvedIndyEntry)));
} else {
imull(index, index, sizeof(ResolvedIndyEntry)); // Scale the index to be
the entry index * sizeof(ResolvedInvokeDynamicInfo)
}
src/hotspot/cpu/x86/templateTable_x86.cpp line 2747:
> 2745: address entry = CAST_FROM_FN_PTR(address,
> InterpreterRuntime::resolve_from_cache);
> 2746: __ movl(method, code); // this is essentially
> Bytecodes::_invokedynamic
> 2747: __ call_VM(noreg, entry, method); // Example uses temp = rbx. In this
> case rbx is method
The comment is confusing and seems to need an update. The register 'method' is
used, but its content is not the method anymore, it is the bytecode.
src/hotspot/cpu/x86/templateTable_x86.cpp line 2770:
> 2768: // since the parameter_size includes it.
> 2769: __ push(rbx);
> 2770: __ mov(rbx, index);
Why is the index (rdx) copied to rbx instead of using the index (rdx) register
directly to call load_resolved_reference_at_index() ? The method doesn't modify
the content of the register.
src/hotspot/share/interpreter/bootstrapInfo.cpp line 67:
> 65: assert(_indy_index != -1, "");
> 66: // Check if method is not null
> 67: if ( _pool->resolved_indy_entry_at(_indy_index)->method() != nullptr) {
_pool->resolved_reference_from_indy(_indy_index) is repeated 5 times. Using a
local variable would make the code easier to read.
-
PR: https://git.openjdk.org/jdk/pull/12778
RFR: 8303908: Add missing check in VTMS_transition_disable_for_all() for suspend mode
The message from this sender included one or more files which could not be scanned for virus detection; do not open these files unless you are certain of the sender's intent. -- Please review this small fix. A suspender is a JvmtiVTMSTransitionDisabler monopolist, meaning VTMS_transition_disable_for_all() should not return while there is any active jvmtiVTMSTransitionDisabler. The code though is checking for active "all-disablers" but it's missing the check for active "single disablers". I attached a simple reproducer to the bug which I used to test the patch. Not sure if it was worth adding a test so the patch contains just the fix. Thanks, Patricio - Commit messages: - v1 Changes: https://git.openjdk.org/jdk/pull/12956/files Webrev: https://webrevs.openjdk.org/?repo=jdk=12956=00 Issue: https://bugs.openjdk.org/browse/JDK-8303908 Stats: 3 lines in 1 file changed: 1 ins; 0 del; 2 mod Patch: https://git.openjdk.org/jdk/pull/12956.diff Fetch: git fetch https://git.openjdk.org/jdk pull/12956/head:pull/12956 PR: https://git.openjdk.org/jdk/pull/12956
Integrated: 8303489: Add a test to verify classes in vmStruct have unique vtables
On Thu, 2 Mar 2023 02:41:12 GMT, Alex Menkov wrote: > Unique vtables for classes in vmStruct data is a requirement for SA to > correctly detect hotspot classes. > The fix adds test to verify this requirement. > > The test fails as expected on Windows if VM is built without RTTI (see > JDK-8302817) This pull request has now been integrated. Changeset: f9aadb94 Author:Alex Menkov URL: https://git.openjdk.org/jdk/commit/f9aadb943cb90382a631a5cafd0624d4e8a47789 Stats: 180 lines in 2 files changed: 178 ins; 0 del; 2 mod 8303489: Add a test to verify classes in vmStruct have unique vtables Reviewed-by: cjplummer, sspitsyn - PR: https://git.openjdk.org/jdk/pull/12820
Re: RFR: 8291555: Implement alternative fast-locking scheme [v15]
On Wed, 8 Mar 2023 18:25:15 GMT, Roman Kennke wrote: >> This change adds a fast-locking scheme as an alternative to the current >> stack-locking implementation. It retains the advantages of stack-locking >> (namely fast locking in uncontended code-paths), while avoiding the overload >> of the mark word. That overloading causes massive problems with Lilliput, >> because it means we have to check and deal with this situation when trying >> to access the mark-word. And because of the very racy nature, this turns out >> to be very complex and would involve a variant of the inflation protocol to >> ensure that the object header is stable. (The current implementation of >> setting/fetching the i-hash provides a glimpse into the complexity). >> >> What the original stack-locking does is basically to push a stack-lock onto >> the stack which consists only of the displaced header, and CAS a pointer to >> this stack location into the object header (the lowest two header bits being >> 00 indicate 'stack-locked'). The pointer into the stack can then be used to >> identify which thread currently owns the lock. >> >> This change basically reverses stack-locking: It still CASes the lowest two >> header bits to 00 to indicate 'fast-locked' but does *not* overload the >> upper bits with a stack-pointer. Instead, it pushes the object-reference to >> a thread-local lock-stack. This is a new structure which is basically a >> small array of oops that is associated with each thread. Experience shows >> that this array typcially remains very small (3-5 elements). Using this lock >> stack, it is possible to query which threads own which locks. Most >> importantly, the most common question 'does the current thread own me?' is >> very quickly answered by doing a quick scan of the array. More complex >> queries like 'which thread owns X?' are not performed in very >> performance-critical paths (usually in code like JVMTI or deadlock >> detection) where it is ok to do more complex operations (and we already do). >> The lock-stack is also a new set of GC roots, and would be scanned during >> thread scanning, possibly concurrently, via the normal protocols. >> >> The lock-stack is grown when needed. This means that we need to check for >> potential overflow before attempting locking. When that is the case, locking >> fast-paths would call into the runtime to grow the stack and handle the >> locking. Compiled fast-paths (C1 and C2 on x86_64 and aarch64) do this check >> on method entry to avoid (possibly lots) of such checks at locking sites. >> >> In contrast to stack-locking, fast-locking does *not* support recursive >> locking (yet). When that happens, the fast-lock gets inflated to a full >> monitor. It is not clear if it is worth to add support for recursive >> fast-locking. >> >> One trouble is that when a contending thread arrives at a fast-locked >> object, it must inflate the fast-lock to a full monitor. Normally, we need >> to know the current owning thread, and record that in the monitor, so that >> the contending thread can wait for the current owner to properly exit the >> monitor. However, fast-locking doesn't have this information. What we do >> instead is to record a special marker ANONYMOUS_OWNER. When the thread that >> currently holds the lock arrives at monitorexit, and observes >> ANONYMOUS_OWNER, it knows it must be itself, fixes the owner to be itself, >> and then properly exits the monitor, and thus handing over to the contending >> thread. >> >> As an alternative, I considered to remove stack-locking altogether, and only >> use heavy monitors. In most workloads this did not show measurable >> regressions. However, in a few workloads, I have observed severe >> regressions. All of them have been using old synchronized Java collections >> (Vector, Stack), StringBuffer or similar code. The combination of two >> conditions leads to regressions without stack- or fast-locking: 1. The >> workload synchronizes on uncontended locks (e.g. single-threaded use of >> Vector or StringBuffer) and 2. The workload churns such locks. IOW, >> uncontended use of Vector, StringBuffer, etc as such is ok, but creating >> lots of such single-use, single-threaded-locked objects leads to massive >> ObjectMonitor churn, which can lead to a significant performance impact. But >> alas, such code exists, and we probably don't want to punish it if we can >> avoid it. >> >> This change enables to simplify (and speed-up!) a lot of code: >> >> - The inflation protocol is no longer necessary: we can directly CAS the >> (tagged) ObjectMonitor pointer to the object header. >> - Accessing the hashcode could now be done in the fastpath always, if the >> hashcode has been installed. Fast-locked headers can be used directly, for >> monitor-locked objects we can easily reach-through to the displaced header. >> This is safe because Java threads participate in monitor deflation protocol.
Re: RFR: 8291555: Implement alternative fast-locking scheme [v5]
On Mon, 30 Jan 2023 14:30:41 GMT, Roman Kennke wrote:
>> src/hotspot/share/runtime/synchronizer.cpp line 1336:
>>
>>> 1334: // Success! Return inflated monitor.
>>> 1335: if (own) {
>>> 1336: assert(current->is_Java_thread(), "must be: checked in
>>> is_lock_owned()");
>>
>> `is_lock_owned()` currently does this:
>>
>>
>> static bool is_lock_owned(Thread* thread, oop obj) {
>> assert(UseFastLocking, "only call this with fast-locking enabled");
>> return thread->is_Java_thread() ?
>> reinterpret_cast(thread)->lock_stack().contains(obj) : false;
>> }
>>
>>
>> so I would not say "checked in is_locked_owned()" since `is_locked_owned()`
>> does
>> not enforce that the caller is a JavaThread.
>
> If it's not a Java thread, `is_lock_owned()` returns `false`, and we wouldn't
> end up in the `if (own)` branch.
Okay, I get it. `is_lock_owned()` only return `true` when called by a
JavaThread and if that JavaThread owns the monitor.
-
PR: https://git.openjdk.org/jdk/pull/10907
Re: RFR: 8303814: getLastErrorString should avoid charset conversions
On Wed, 8 Mar 2023 11:30:27 GMT, Daniel Jeliński wrote:
> This patch modifies the `getLastErrorString` method to return a `jstring`.
> Thanks to that we can avoid unnecessary back and forth conversions between
> Unicode and other charsets on Windows.
>
> Other changes include:
> - the Windows implementation of `getLastErrorString` no longer checks
> `errno`. I verified all uses of the method and confirmed that `errno` is not
> used anywhere.
> - While at it, I found and fixed a few calls to
> `JNU_ThrowIOExceptionWithLastError` that were done in context where
> `LastError` was not set.
> - jdk.hotspot.agent was modified to use `JNU_ThrowByNameWithLastError` and
> `JNU_ThrowByName` instead of `getLastErrorString`; the code is expected to
> have identical behavior.
> - zip_util was modified to return static messages instead of generated ones.
> The generated messages were not observed anywhere, because they were replaced
> by a static message in ZIP_Open, which is the only method used by other
> native code.
> - `getLastErrorString` is no longer exported by libjava.
>
> Tier1-3 tests continue to pass.
>
> No new automated regression test; testing this requires installing a language
> pack that cannot be displayed in the current code page.
> Tested this manually by installing Chinese language pack on English Windows
> 11, selecting Chinese language, then checking if the message on exception
> thrown by `InetAddress.getByName("nonexistent.local");` starts with
> `"不知道这样的主机。"` (or
> `"\u4e0d\u77e5\u9053\u8fd9\u6837\u7684\u4e3b\u673a\u3002"`). Without the
> change, the exception message started with a row of question marks.
Looks good (w/ some minor comments)
src/java.base/share/native/libzip/zip_util.c line 767:
> 765: * or NULL if an error occurred. If a zip error occurred then *pmsg will
> 766: * be set to the error message text if pmsg != 0. Otherwise, *pmsg will
> be
> 767: * set to NULL. Caller doesn't need to free the error message.
I'd put some more context here why the caller does not need to free. (as it is
a static text)
src/java.base/windows/native/libjava/jni_util_md.c line 80:
> 78: 0,
> 79: buf,
> 80: sizeof(buf) / sizeof(WCHAR),
Maybe better to #define the size 256 so that this division is not needed.
src/java.base/windows/native/libnio/ch/FileDispatcherImpl.c line 208:
> 206:
> 207: if (result == 0) {
> 208: JNU_ThrowIOExceptionWithLastError(env, "Write failed");
Could be replaced with `JNU_ThrowIOException`?
src/java.base/windows/native/libnio/ch/FileDispatcherImpl.c line 260:
> 258:
> 259: if (result == 0) {
> 260: JNU_ThrowIOExceptionWithLastError(env, "Write failed");
Same as above
src/java.base/windows/native/libnio/ch/FileDispatcherImpl.c line 299:
> 297:
> 298: if (result == 0) {
> 299: JNU_ThrowIOExceptionWithLastError(env, "Write failed");
Same here
-
PR: https://git.openjdk.org/jdk/pull/12922
Re: RFR: 8257967: JFR: Events for loaded agents [v9]
> Greetings,
>
> We are adding support to let JFR report on Agents.
>
> Design
>
> An Agent is a library that uses any instrumentation or profiling APIs. Most
> agents are started and initialized on the command line, but agents can also
> be loaded dynamically during runtime. Because command line agents initialize
> during the VM startup sequence, they add to the overall startup time latency
> in getting the VM ready. The events will report on the time the agent took to
> initialize.
>
> A JavaAgent is an agent written in the Java programming language, using the
> APIs in the package
> [java.lang.instrument](https://docs.oracle.com/en/java/javase/19/docs/api/java.instrument/java/lang/instrument/package-summary.html)
>
> A JavaAgent is sometimes called a JPLIS agent, where the acronym JPLIS stands
> for Java Programming Language Instrumentation Services.
>
> To report on JavaAgents, JFR will add the new event type jdk.JavaAgent and
> events will look similar to these two examples:
>
> // Command line
> jdk.JavaAgent {
> startTime = 12:31:19.789 (2023-03-08)
> name = "JavaAgent.jar"
> options = "foo=bar"
> dynamic = false
> initialization = 12:31:15.574 (2023-03-08)
> initializationTime = 172 ms
> }
>
> // Dynamic load
> jdk.JavaAgent {
> startTime = 12:31:31.158 (2023-03-08)
> name = "JavaAgent.jar"
> options = "bar=baz"
> dynamic = true
> initialization = 12:31:31.037 (2023-03-08)
> initializationTime = 64,1 ms
> }
>
> The jdk.JavaAgent event type is a JFR periodic event that iterates over
> running Java agents.
>
> For a JavaAgent event, the agent's name will be the specific .jar file
> containing the instrumentation code. The options will be the specific options
> passed to the .jar file as part of launching the agent, for example, on the
> command line: -javaagent: JavaAgent.jar=foo=bar.
>
> The "dynamic" field denotes if the agent was loaded via the command line
> (dynamic = false) or dynamically (dynamic = true)
>
> "initialization" is the timestamp the JVM invoked the initialization method,
> and "initializationTime" is the duration of executing the initialization
> method.
>
> "startTime" represents the time the JFR framework issued the periodic event;
> hence "initialization" will be earlier than "startTime".
>
> An agent can also be written in a native programming language using the [JVM
> Tools Interface
> (JVMTI)](https://docs.oracle.com/en/java/javase/19/docs/specs/jvmti.html).
> This kind of agent, sometimes called a native agent, is a platform-specific
> binary, sometimes referred to as a library, but here it means a .so or .dll
> file.
>
> To report on native agents, JFR will add the new event type jdk.NativeAgent
> and events will look similar to this example:
>
> jdk.NativeAgent {
> startTime = 12:31:40.398 (2023-03-08)
> name = "jdwp"
> options = "transport=dt_socket,server=y,address=any,onjcmd=y"
> dynamic = false
> initialization = 12:31:36.142 (2023-03-08)
> initializationTime = 0,00184 ms
> path =
> "c:\ade\github\openjdk\jdk\build\windows-x86_64-server-slowdebug\jdk\bin\jdwp.dll"
> }
>
> The layout of the event type is very similar to the jdk.JavaAgent event, but
> here the path to the native library is reported.
>
> The initialization of a native agent is performed by invoking an
> agent-specified callback routine. The "initialization" is when the JVM sent
> or would have sent the JVMTI VMInit event to a specified callback.
> "initializationTime" is the duration to execute that specific callback. If no
> callback is specified for the JVMTI VMInit event, the "initializationTime"
> will be 0.
>
> Implementation
>
> There has not existed a reification of a JavaAgent directly in the JVM, as
> these are built on top of the JDK native library, "instrument", using a
> many-to-one mapping. At the level of the JVM, the only representation of
> agents after startup is through JvmtiEnv's, which agents request from the JVM
> during startup and initialization — as such, mapping which JvmtiEnv belongs
> to what JavaAgent was not possible before.
>
> Using implementation details of how the JDK native library "instrument"
> interacts with the JVM, we can build this mapping to track what JvmtiEnv's
> "belong" to what JavaAgent. This mapping now lets us report the Java-relevant
> context (name, options) and measure the time it takes for the JavaAgent to
> initialize.
>
> When implementing this capability, it was necessary to refactor the code used
> to represent agents, AgentLibrary. The previous implementation was located
> primarily in arguments.cpp, and threads.cpp but also jvmtiExport.cpp.
>
> The refactoring isolates the relevant logic into two new modules,
> prims/agent.hpp and prims/agentList.hpp. Breaking out this code from their
> older places will help reduce the sizes of oversized arguments.cpp and
> threads.cpp.
>
> The previous two lists that maintained "agents" (JVMTI)
Re: RFR: 8301995: Move invokedynamic resolution information out of ConstantPoolCacheEntry
On Thu, 9 Mar 2023 16:01:21 GMT, Coleen Phillimore wrote: >> This change should be in a further RFE though (and you can explain it there >> so we can maybe use it in the other platforms too). Does it affect >> performance when generating the template interpreter? Do you need to have >> hsdis in the LD_LIBRARY_PATH environment variable to use this? I see it's >> already used by default in one place. > > This looks cool. > This change should be in a further RFE though (and you can explain it there > so we can maybe use it in the other platforms too). Ok. > Does it affect performance when generating the template interpreter? I didn't think it would affect performance if the interpreter is not printed. I have not measured it though. > Do you need to have hsdis in the LD_LIBRARY_PATH environment variable to use > this? I see it's already used by default in one place. Yes you do. It is not working with the AbstractDisassembler which produces a hex dump of the machine code. - PR: https://git.openjdk.org/jdk/pull/12778
Re: RFR: 8301995: Move invokedynamic resolution information out of ConstantPoolCacheEntry
On Thu, 9 Mar 2023 16:00:53 GMT, Coleen Phillimore wrote: >> Yes this is really useful when debugging the template interpreter. It >> annotates the disassembly with the generator source code. It helped tracking >> down a bug in the ppc part oft this pr. Other platforms have it too. >> >> Example: >> >> invokedynamic 186 invokedynamic [0x3fff80075a00, 0x3fff80075dc8] >> 968 bytes >> >> >> 0x3fff80075a00: std r17,0(r15) ;;@FILE: >> src/hotspot/cpu/ppc/templateInterpreterGenerator_ppc.cpp >> ;; 2185: aep = >> __ pc(); __ push_ptr(); __ b(L); >> 0x3fff80075a04: addir15,r15,-8 >> 0x3fff80075a08: b 0x3fff80075a40 ;; 2185: aep = >> __ pc(); __ push_ptr(); __ b(L); >> 0x3fff80075a0c: stfsf15,0(r15) ;; 2186: fep = >> __ pc(); __ push_f();__ b(L); >> 0x3fff80075a10: addir15,r15,-8 >> 0x3fff80075a14: b 0x3fff80075a40 ;; 2186: fep = >> __ pc(); __ push_f();__ b(L); >> 0x3fff80075a18: stfdf15,-8(r15) ;; 2187: dep = >> __ pc(); __ push_d();__ b(L); >> 0x3fff80075a1c: addir15,r15,-16 >> 0x3fff80075a20: b 0x3fff80075a40 ;; 2187: dep = >> __ pc(); __ push_d();__ b(L); >> 0x3fff80075a24: li r0,0;; 2188: lep = >> __ pc(); __ push_l();__ b(L); >> 0x3fff80075a28: std r0,0(r15) >> 0x3fff80075a2c: std r17,-8(r15) >> 0x3fff80075a30: addir15,r15,-16 >> 0x3fff80075a34: b 0x3fff80075a40 ;; 2188: lep = >> __ pc(); __ push_l();__ b(L); >> 0x3fff80075a38: stw r17,0(r15) ;; 2189: __ >> align(32, 12, 24); // align L >> ;; 2191: iep = >> __ pc(); __ push_i(); >> 0x3fff80075a3c: addir15,r15,-8 >> 0x3fff80075a40: li r21,1 ;; 2192: vep = >> __ pc(); >> ;; 2193: __ >> bind(L); >> ;;@FILE: >> src/hotspot/share/interpreter/templateInterpreterGenerator.cpp >> ;; 366: __ >> verify_FPU(1, t->tos_in()); >> ;;@FILE: >> src/hotspot/cpu/ppc/templateTable_ppc_64.cpp >> ;; 2293: __ >> load_resolved_indy_entry(cache, index); >> 0x3fff80075a44: lwaxr21,r14,r21 >> 0x3fff80075a48: nandr21,r21,r21 >> 0x3fff80075a4c: ld r31,40(r27) >> 0x3fff80075a50: rldicr r21,r21,4,59 >> 0x3fff80075a54: addir21,r21,8 >> 0x3fff80075a58: add r31,r31,r21 >> 0x3fff80075a5c: ld r22,0(r31) ;; 2294: __ >> ld_ptr(method, in_bytes(ResolvedIndyEntry::method_offset()), cache); >> 0x3fff80075a60: cmpdi r22,0 ;; 2297: __ >> cmpdi(CCR0, method, 0); >> 0x3fff80075a64: bne-0x3fff80075b94 ;; 2298: __ >> bne(CCR0, resolved);,bo=0b00100[no_hint] >> 0x3fff80075a68: li r4,186 ;; 2304: __ >> li(R4_ARG2, code); >> 0x3fff80075a6c: ld r11,0(r1) ;; 2305: __ >> call_VM(noreg, entry, R4_ARG2, true); > > This change should be in a further RFE though (and you can explain it there > so we can maybe use it in the other platforms too). Does it affect > performance when generating the template interpreter? Do you need to have > hsdis in the LD_LIBRARY_PATH environment variable to use this? I see it's > already used by default in one place. This looks cool. - PR: https://git.openjdk.org/jdk/pull/12778
Re: RFR: 8301995: Move invokedynamic resolution information out of ConstantPoolCacheEntry
On Tue, 7 Mar 2023 15:04:29 GMT, Richard Reingruber wrote: >> src/hotspot/cpu/ppc/templateTable_ppc_64.cpp line 53: >> >>> 51: >>> 52: #undef __ >>> 53: #define __ Disassembler::hook(__FILE__, >>> __LINE__, _masm)-> >> >> What is this? Is this something useful for debugging the template >> interpreter? Probably doesn't belong with this change but might be nice to >> have (?) @reinrich > > Yes this is really useful when debugging the template interpreter. It > annotates the disassembly with the generator source code. It helped tracking > down a bug in the ppc part oft this pr. Other platforms have it too. > > Example: > > invokedynamic 186 invokedynamic [0x3fff80075a00, 0x3fff80075dc8] > 968 bytes > > > 0x3fff80075a00: std r17,0(r15) ;;@FILE: > src/hotspot/cpu/ppc/templateInterpreterGenerator_ppc.cpp > ;; 2185: aep = > __ pc(); __ push_ptr(); __ b(L); > 0x3fff80075a04: addir15,r15,-8 > 0x3fff80075a08: b 0x3fff80075a40 ;; 2185: aep = > __ pc(); __ push_ptr(); __ b(L); > 0x3fff80075a0c: stfsf15,0(r15) ;; 2186: fep = > __ pc(); __ push_f();__ b(L); > 0x3fff80075a10: addir15,r15,-8 > 0x3fff80075a14: b 0x3fff80075a40 ;; 2186: fep = > __ pc(); __ push_f();__ b(L); > 0x3fff80075a18: stfdf15,-8(r15) ;; 2187: dep = > __ pc(); __ push_d();__ b(L); > 0x3fff80075a1c: addir15,r15,-16 > 0x3fff80075a20: b 0x3fff80075a40 ;; 2187: dep = > __ pc(); __ push_d();__ b(L); > 0x3fff80075a24: li r0,0;; 2188: lep = > __ pc(); __ push_l();__ b(L); > 0x3fff80075a28: std r0,0(r15) > 0x3fff80075a2c: std r17,-8(r15) > 0x3fff80075a30: addir15,r15,-16 > 0x3fff80075a34: b 0x3fff80075a40 ;; 2188: lep = > __ pc(); __ push_l();__ b(L); > 0x3fff80075a38: stw r17,0(r15) ;; 2189: __ > align(32, 12, 24); // align L > ;; 2191: iep = > __ pc(); __ push_i(); > 0x3fff80075a3c: addir15,r15,-8 > 0x3fff80075a40: li r21,1 ;; 2192: vep = > __ pc(); > ;; 2193: __ > bind(L); > ;;@FILE: > src/hotspot/share/interpreter/templateInterpreterGenerator.cpp > ;; 366: __ > verify_FPU(1, t->tos_in()); > ;;@FILE: > src/hotspot/cpu/ppc/templateTable_ppc_64.cpp > ;; 2293: __ > load_resolved_indy_entry(cache, index); > 0x3fff80075a44: lwaxr21,r14,r21 > 0x3fff80075a48: nandr21,r21,r21 > 0x3fff80075a4c: ld r31,40(r27) > 0x3fff80075a50: rldicr r21,r21,4,59 > 0x3fff80075a54: addir21,r21,8 > 0x3fff80075a58: add r31,r31,r21 > 0x3fff80075a5c: ld r22,0(r31) ;; 2294: __ > ld_ptr(method, in_bytes(ResolvedIndyEntry::method_offset()), cache); > 0x3fff80075a60: cmpdi r22,0 ;; 2297: __ > cmpdi(CCR0, method, 0); > 0x3fff80075a64: bne-0x3fff80075b94 ;; 2298: __ > bne(CCR0, resolved);,bo=0b00100[no_hint] > 0x3fff80075a68: li r4,186 ;; 2304: __ > li(R4_ARG2, code); > 0x3fff80075a6c: ld r11,0(r1) ;; 2305: __ > call_VM(noreg, entry, R4_ARG2, true); This change should be in a further RFE though (and you can explain it there so we can maybe use it in the other platforms too). Does it affect performance when generating the template interpreter? Do you need to have hsdis in the LD_LIBRARY_PATH environment variable to use this? I see it's already used by default in one place. - PR: https://git.openjdk.org/jdk/pull/12778
Re: RFR: 8301995: Move invokedynamic resolution information out of ConstantPoolCacheEntry
On Tue, 7 Mar 2023 19:25:58 GMT, Matias Saavedra Silva
wrote:
>> src/hotspot/share/oops/cpCache.cpp line 727:
>>
>>> 725: set_reference_map(nullptr);
>>> 726: #if INCLUDE_CDS
>>> 727: if (_initial_entries != nullptr) {
>>
>> @iklam with moving invokedynamic entries out, do you still need to save
>> initialized entries ? Does invokehandle need this? (Should have separate
>> RFE if more cleanup is possible)
>
> This along with the previous comment about `_invokedynamic_references_map`
> would probably be better suited for their own RFE. I think the scope of this
> PR should be limited to the indy structure and its implementation, so any
> changes related to invokehandle can be traced more easily.
ok, that's fine.
-
PR: https://git.openjdk.org/jdk/pull/12778
Integrated: 8303702: Provide ThreadFactory to create platform/virtual threads for com/sun/jdi tests
On Mon, 6 Mar 2023 23:47:09 GMT, Leonid Mesnik wrote: > Provide a way to start debugee threads as platform or virtual for debugee in > com/sun/jdi tests. This pull request has now been integrated. Changeset: cdcf5c1e Author:Leonid Mesnik URL: https://git.openjdk.org/jdk/commit/cdcf5c1ed89505b6bf688fb255b493be4bbb13d2 Stats: 116 lines in 13 files changed: 20 ins; 30 del; 66 mod 8303702: Provide ThreadFactory to create platform/virtual threads for com/sun/jdi tests Reviewed-by: cjplummer, sspitsyn - PR: https://git.openjdk.org/jdk/pull/12894
Re: RFR: 8303702: Provide ThreadFactory to create platform/virtual threads for com/sun/jdi tests [v3]
On Tue, 7 Mar 2023 19:00:25 GMT, Leonid Mesnik wrote: >> Provide a way to start debugee threads as platform or virtual for debugee in >> com/sun/jdi tests. > > Leonid Mesnik has updated the pull request incrementally with one additional > commit since the last revision: > > fixed JdbStopThreadidTest to support new change. I've updated all tests with additional threads which I found, - PR: https://git.openjdk.org/jdk/pull/12894
Re: RFR: 8257967: JFR: Events for loaded agents [v8]
> Greetings,
>
> We are adding support to let JFR report on Agents.
>
> Design
>
> An Agent is a library that uses any instrumentation or profiling APIs. Most
> agents are started and initialized on the command line, but agents can also
> be loaded dynamically during runtime. Because command line agents initialize
> during the VM startup sequence, they add to the overall startup time latency
> in getting the VM ready. The events will report on the time the agent took to
> initialize.
>
> A JavaAgent is an agent written in the Java programming language, using the
> APIs in the package
> [java.lang.instrument](https://docs.oracle.com/en/java/javase/19/docs/api/java.instrument/java/lang/instrument/package-summary.html)
>
> A JavaAgent is sometimes called a JPLIS agent, where the acronym JPLIS stands
> for Java Programming Language Instrumentation Services.
>
> To report on JavaAgents, JFR will add the new event type jdk.JavaAgent and
> events will look similar to these two examples:
>
> // Command line
> jdk.JavaAgent {
> startTime = 12:31:19.789 (2023-03-08)
> name = "JavaAgent.jar"
> options = "foo=bar"
> dynamic = false
> initialization = 12:31:15.574 (2023-03-08)
> initializationTime = 172 ms
> }
>
> // Dynamic load
> jdk.JavaAgent {
> startTime = 12:31:31.158 (2023-03-08)
> name = "JavaAgent.jar"
> options = "bar=baz"
> dynamic = true
> initialization = 12:31:31.037 (2023-03-08)
> initializationTime = 64,1 ms
> }
>
> The jdk.JavaAgent event type is a JFR periodic event that iterates over
> running Java agents.
>
> For a JavaAgent event, the agent's name will be the specific .jar file
> containing the instrumentation code. The options will be the specific options
> passed to the .jar file as part of launching the agent, for example, on the
> command line: -javaagent: JavaAgent.jar=foo=bar.
>
> The "dynamic" field denotes if the agent was loaded via the command line
> (dynamic = false) or dynamically (dynamic = true)
>
> "initialization" is the timestamp the JVM invoked the initialization method,
> and "initializationTime" is the duration of executing the initialization
> method.
>
> "startTime" represents the time the JFR framework issued the periodic event;
> hence "initialization" will be earlier than "startTime".
>
> An agent can also be written in a native programming language using the [JVM
> Tools Interface
> (JVMTI)](https://docs.oracle.com/en/java/javase/19/docs/specs/jvmti.html).
> This kind of agent, sometimes called a native agent, is a platform-specific
> binary, sometimes referred to as a library, but here it means a .so or .dll
> file.
>
> To report on native agents, JFR will add the new event type jdk.NativeAgent
> and events will look similar to this example:
>
> jdk.NativeAgent {
> startTime = 12:31:40.398 (2023-03-08)
> name = "jdwp"
> options = "transport=dt_socket,server=y,address=any,onjcmd=y"
> dynamic = false
> initialization = 12:31:36.142 (2023-03-08)
> initializationTime = 0,00184 ms
> path =
> "c:\ade\github\openjdk\jdk\build\windows-x86_64-server-slowdebug\jdk\bin\jdwp.dll"
> }
>
> The layout of the event type is very similar to the jdk.JavaAgent event, but
> here the path to the native library is reported.
>
> The initialization of a native agent is performed by invoking an
> agent-specified callback routine. The "initialization" is when the JVM sent
> or would have sent the JVMTI VMInit event to a specified callback.
> "initializationTime" is the duration to execute that specific callback. If no
> callback is specified for the JVMTI VMInit event, the "initializationTime"
> will be 0.
>
> Implementation
>
> There has not existed a reification of a JavaAgent directly in the JVM, as
> these are built on top of the JDK native library, "instrument", using a
> many-to-one mapping. At the level of the JVM, the only representation of
> agents after startup is through JvmtiEnv's, which agents request from the JVM
> during startup and initialization — as such, mapping which JvmtiEnv belongs
> to what JavaAgent was not possible before.
>
> Using implementation details of how the JDK native library "instrument"
> interacts with the JVM, we can build this mapping to track what JvmtiEnv's
> "belong" to what JavaAgent. This mapping now lets us report the Java-relevant
> context (name, options) and measure the time it takes for the JavaAgent to
> initialize.
>
> When implementing this capability, it was necessary to refactor the code used
> to represent agents, AgentLibrary. The previous implementation was located
> primarily in arguments.cpp, and threads.cpp but also jvmtiExport.cpp.
>
> The refactoring isolates the relevant logic into two new modules,
> prims/agent.hpp and prims/agentList.hpp. Breaking out this code from their
> older places will help reduce the sizes of oversized arguments.cpp and
> threads.cpp.
>
> The previous two lists that maintained "agents" (JVMTI)
Re: RFR: 8257967: JFR: Events for loaded agents [v7]
> Greetings,
>
> We are adding support to let JFR report on Agents.
>
> Design
>
> An Agent is a library that uses any instrumentation or profiling APIs. Most
> agents are started and initialized on the command line, but agents can also
> be loaded dynamically during runtime. Because command line agents initialize
> during the VM startup sequence, they add to the overall startup time latency
> in getting the VM ready. The events will report on the time the agent took to
> initialize.
>
> A JavaAgent is an agent written in the Java programming language, using the
> APIs in the package
> [java.lang.instrument](https://docs.oracle.com/en/java/javase/19/docs/api/java.instrument/java/lang/instrument/package-summary.html)
>
> A JavaAgent is sometimes called a JPLIS agent, where the acronym JPLIS stands
> for Java Programming Language Instrumentation Services.
>
> To report on JavaAgents, JFR will add the new event type jdk.JavaAgent and
> events will look similar to these two examples:
>
> // Command line
> jdk.JavaAgent {
> startTime = 12:31:19.789 (2023-03-08)
> name = "JavaAgent.jar"
> options = "foo=bar"
> dynamic = false
> initialization = 12:31:15.574 (2023-03-08)
> initializationTime = 172 ms
> }
>
> // Dynamic load
> jdk.JavaAgent {
> startTime = 12:31:31.158 (2023-03-08)
> name = "JavaAgent.jar"
> options = "bar=baz"
> dynamic = true
> initialization = 12:31:31.037 (2023-03-08)
> initializationTime = 64,1 ms
> }
>
> The jdk.JavaAgent event type is a JFR periodic event that iterates over
> running Java agents.
>
> For a JavaAgent event, the agent's name will be the specific .jar file
> containing the instrumentation code. The options will be the specific options
> passed to the .jar file as part of launching the agent, for example, on the
> command line: -javaagent: JavaAgent.jar=foo=bar.
>
> The "dynamic" field denotes if the agent was loaded via the command line
> (dynamic = false) or dynamically (dynamic = true)
>
> "initialization" is the timestamp the JVM invoked the initialization method,
> and "initializationTime" is the duration of executing the initialization
> method.
>
> "startTime" represents the time the JFR framework issued the periodic event;
> hence "initialization" will be earlier than "startTime".
>
> An agent can also be written in a native programming language using the [JVM
> Tools Interface
> (JVMTI)](https://docs.oracle.com/en/java/javase/19/docs/specs/jvmti.html).
> This kind of agent, sometimes called a native agent, is a platform-specific
> binary, sometimes referred to as a library, but here it means a .so or .dll
> file.
>
> To report on native agents, JFR will add the new event type jdk.NativeAgent
> and events will look similar to this example:
>
> jdk.NativeAgent {
> startTime = 12:31:40.398 (2023-03-08)
> name = "jdwp"
> options = "transport=dt_socket,server=y,address=any,onjcmd=y"
> dynamic = false
> initialization = 12:31:36.142 (2023-03-08)
> initializationTime = 0,00184 ms
> path =
> "c:\ade\github\openjdk\jdk\build\windows-x86_64-server-slowdebug\jdk\bin\jdwp.dll"
> }
>
> The layout of the event type is very similar to the jdk.JavaAgent event, but
> here the path to the native library is reported.
>
> The initialization of a native agent is performed by invoking an
> agent-specified callback routine. The "initialization" is when the JVM sent
> or would have sent the JVMTI VMInit event to a specified callback.
> "initializationTime" is the duration to execute that specific callback. If no
> callback is specified for the JVMTI VMInit event, the "initializationTime"
> will be 0.
>
> Implementation
>
> There has not existed a reification of a JavaAgent directly in the JVM, as
> these are built on top of the JDK native library, "instrument", using a
> many-to-one mapping. At the level of the JVM, the only representation of
> agents after startup is through JvmtiEnv's, which agents request from the JVM
> during startup and initialization — as such, mapping which JvmtiEnv belongs
> to what JavaAgent was not possible before.
>
> Using implementation details of how the JDK native library "instrument"
> interacts with the JVM, we can build this mapping to track what JvmtiEnv's
> "belong" to what JavaAgent. This mapping now lets us report the Java-relevant
> context (name, options) and measure the time it takes for the JavaAgent to
> initialize.
>
> When implementing this capability, it was necessary to refactor the code used
> to represent agents, AgentLibrary. The previous implementation was located
> primarily in arguments.cpp, and threads.cpp but also jvmtiExport.cpp.
>
> The refactoring isolates the relevant logic into two new modules,
> prims/agent.hpp and prims/agentList.hpp. Breaking out this code from their
> older places will help reduce the sizes of oversized arguments.cpp and
> threads.cpp.
>
> The previous two lists that maintained "agents" (JVMTI)
Re: RFR: 8257967: JFR: Events for loaded agents [v6]
> Greetings,
>
> We are adding support to let JFR report on Agents.
>
> Design
>
> An Agent is a library that uses any instrumentation or profiling APIs. Most
> agents are started and initialized on the command line, but agents can also
> be loaded dynamically during runtime. Because command line agents initialize
> during the VM startup sequence, they add to the overall startup time latency
> in getting the VM ready. The events will report on the time the agent took to
> initialize.
>
> A JavaAgent is an agent written in the Java programming language, using the
> APIs in the package
> [java.lang.instrument](https://docs.oracle.com/en/java/javase/19/docs/api/java.instrument/java/lang/instrument/package-summary.html)
>
> A JavaAgent is sometimes called a JPLIS agent, where the acronym JPLIS stands
> for Java Programming Language Instrumentation Services.
>
> To report on JavaAgents, JFR will add the new event type jdk.JavaAgent and
> events will look similar to these two examples:
>
> // Command line
> jdk.JavaAgent {
> startTime = 12:31:19.789 (2023-03-08)
> name = "JavaAgent.jar"
> options = "foo=bar"
> dynamic = false
> initialization = 12:31:15.574 (2023-03-08)
> initializationTime = 172 ms
> }
>
> // Dynamic load
> jdk.JavaAgent {
> startTime = 12:31:31.158 (2023-03-08)
> name = "JavaAgent.jar"
> options = "bar=baz"
> dynamic = true
> initialization = 12:31:31.037 (2023-03-08)
> initializationTime = 64,1 ms
> }
>
> The jdk.JavaAgent event type is a JFR periodic event that iterates over
> running Java agents.
>
> For a JavaAgent event, the agent's name will be the specific .jar file
> containing the instrumentation code. The options will be the specific options
> passed to the .jar file as part of launching the agent, for example, on the
> command line: -javaagent: JavaAgent.jar=foo=bar.
>
> The "dynamic" field denotes if the agent was loaded via the command line
> (dynamic = false) or dynamically (dynamic = true)
>
> "initialization" is the timestamp the JVM invoked the initialization method,
> and "initializationTime" is the duration of executing the initialization
> method.
>
> "startTime" represents the time the JFR framework issued the periodic event;
> hence "initialization" will be earlier than "startTime".
>
> An agent can also be written in a native programming language using the [JVM
> Tools Interface
> (JVMTI)](https://docs.oracle.com/en/java/javase/19/docs/specs/jvmti.html).
> This kind of agent, sometimes called a native agent, is a platform-specific
> binary, sometimes referred to as a library, but here it means a .so or .dll
> file.
>
> To report on native agents, JFR will add the new event type jdk.NativeAgent
> and events will look similar to this example:
>
> jdk.NativeAgent {
> startTime = 12:31:40.398 (2023-03-08)
> name = "jdwp"
> options = "transport=dt_socket,server=y,address=any,onjcmd=y"
> dynamic = false
> initialization = 12:31:36.142 (2023-03-08)
> initializationTime = 0,00184 ms
> path =
> "c:\ade\github\openjdk\jdk\build\windows-x86_64-server-slowdebug\jdk\bin\jdwp.dll"
> }
>
> The layout of the event type is very similar to the jdk.JavaAgent event, but
> here the path to the native library is reported.
>
> The initialization of a native agent is performed by invoking an
> agent-specified callback routine. The "initialization" is when the JVM sent
> or would have sent the JVMTI VMInit event to a specified callback.
> "initializationTime" is the duration to execute that specific callback. If no
> callback is specified for the JVMTI VMInit event, the "initializationTime"
> will be 0.
>
> Implementation
>
> There has not existed a reification of a JavaAgent directly in the JVM, as
> these are built on top of the JDK native library, "instrument", using a
> many-to-one mapping. At the level of the JVM, the only representation of
> agents after startup is through JvmtiEnv's, which agents request from the JVM
> during startup and initialization — as such, mapping which JvmtiEnv belongs
> to what JavaAgent was not possible before.
>
> Using implementation details of how the JDK native library "instrument"
> interacts with the JVM, we can build this mapping to track what JvmtiEnv's
> "belong" to what JavaAgent. This mapping now lets us report the Java-relevant
> context (name, options) and measure the time it takes for the JavaAgent to
> initialize.
>
> When implementing this capability, it was necessary to refactor the code used
> to represent agents, AgentLibrary. The previous implementation was located
> primarily in arguments.cpp, and threads.cpp but also jvmtiExport.cpp.
>
> The refactoring isolates the relevant logic into two new modules,
> prims/agent.hpp and prims/agentList.hpp. Breaking out this code from their
> older places will help reduce the sizes of oversized arguments.cpp and
> threads.cpp.
>
> The previous two lists that maintained "agents" (JVMTI)
Re: RFR: 8257967: JFR: Events for loaded agents [v5]
> Greetings,
>
> We are adding support to let JFR report on Agents.
>
> Design
>
> An Agent is a library that uses any instrumentation or profiling APIs. Most
> agents are started and initialized on the command line, but agents can also
> be loaded dynamically during runtime. Because command line agents initialize
> during the VM startup sequence, they add to the overall startup time latency
> in getting the VM ready. The events will report on the time the agent took to
> initialize.
>
> A JavaAgent is an agent written in the Java programming language, using the
> APIs in the package
> [java.lang.instrument](https://docs.oracle.com/en/java/javase/19/docs/api/java.instrument/java/lang/instrument/package-summary.html)
>
> A JavaAgent is sometimes called a JPLIS agent, where the acronym JPLIS stands
> for Java Programming Language Instrumentation Services.
>
> To report on JavaAgents, JFR will add the new event type jdk.JavaAgent and
> events will look similar to these two examples:
>
> // Command line
> jdk.JavaAgent {
> startTime = 12:31:19.789 (2023-03-08)
> name = "JavaAgent.jar"
> options = "foo=bar"
> dynamic = false
> initialization = 12:31:15.574 (2023-03-08)
> initializationTime = 172 ms
> }
>
> // Dynamic load
> jdk.JavaAgent {
> startTime = 12:31:31.158 (2023-03-08)
> name = "JavaAgent.jar"
> options = "bar=baz"
> dynamic = true
> initialization = 12:31:31.037 (2023-03-08)
> initializationTime = 64,1 ms
> }
>
> The jdk.JavaAgent event type is a JFR periodic event that iterates over
> running Java agents.
>
> For a JavaAgent event, the agent's name will be the specific .jar file
> containing the instrumentation code. The options will be the specific options
> passed to the .jar file as part of launching the agent, for example, on the
> command line: -javaagent: JavaAgent.jar=foo=bar.
>
> The "dynamic" field denotes if the agent was loaded via the command line
> (dynamic = false) or dynamically (dynamic = true)
>
> "initialization" is the timestamp the JVM invoked the initialization method,
> and "initializationTime" is the duration of executing the initialization
> method.
>
> "startTime" represents the time the JFR framework issued the periodic event;
> hence "initialization" will be earlier than "startTime".
>
> An agent can also be written in a native programming language using the [JVM
> Tools Interface
> (JVMTI)](https://docs.oracle.com/en/java/javase/19/docs/specs/jvmti.html).
> This kind of agent, sometimes called a native agent, is a platform-specific
> binary, sometimes referred to as a library, but here it means a .so or .dll
> file.
>
> To report on native agents, JFR will add the new event type jdk.NativeAgent
> and events will look similar to this example:
>
> jdk.NativeAgent {
> startTime = 12:31:40.398 (2023-03-08)
> name = "jdwp"
> options = "transport=dt_socket,server=y,address=any,onjcmd=y"
> dynamic = false
> initialization = 12:31:36.142 (2023-03-08)
> initializationTime = 0,00184 ms
> path =
> "c:\ade\github\openjdk\jdk\build\windows-x86_64-server-slowdebug\jdk\bin\jdwp.dll"
> }
>
> The layout of the event type is very similar to the jdk.JavaAgent event, but
> here the path to the native library is reported.
>
> The initialization of a native agent is performed by invoking an
> agent-specified callback routine. The "initialization" is when the JVM sent
> or would have sent the JVMTI VMInit event to a specified callback.
> "initializationTime" is the duration to execute that specific callback. If no
> callback is specified for the JVMTI VMInit event, the "initializationTime"
> will be 0.
>
> Implementation
>
> There has not existed a reification of a JavaAgent directly in the JVM, as
> these are built on top of the JDK native library, "instrument", using a
> many-to-one mapping. At the level of the JVM, the only representation of
> agents after startup is through JvmtiEnv's, which agents request from the JVM
> during startup and initialization — as such, mapping which JvmtiEnv belongs
> to what JavaAgent was not possible before.
>
> Using implementation details of how the JDK native library "instrument"
> interacts with the JVM, we can build this mapping to track what JvmtiEnv's
> "belong" to what JavaAgent. This mapping now lets us report the Java-relevant
> context (name, options) and measure the time it takes for the JavaAgent to
> initialize.
>
> When implementing this capability, it was necessary to refactor the code used
> to represent agents, AgentLibrary. The previous implementation was located
> primarily in arguments.cpp, and threads.cpp but also jvmtiExport.cpp.
>
> The refactoring isolates the relevant logic into two new modules,
> prims/agent.hpp and prims/agentList.hpp. Breaking out this code from their
> older places will help reduce the sizes of oversized arguments.cpp and
> threads.cpp.
>
> The previous two lists that maintained "agents" (JVMTI)
Re: RFR: 8257967: JFR: Events for loaded agents [v4]
On Thu, 9 Mar 2023 09:36:28 GMT, Andrew Dinn wrote: > Yes, I appreciate that `dynamic` can be derived from `initializationMethod` > -- and vice versa. However, I was approaching this semantically from the > opposite end. To me the primary characteristic that the user would be > interested in is whether the agent was loaded dynamically or on the command > line (whatever the type of agent). The corresponding fact, for a Java agent, > that it is entered, respectively, via method agentMain or preMain is a > derived (implementation) detail. Is there a reason to mention that detail? That's a good point. The overall intent was to map what method was measured during initialization. That included native agent callbacks as well. It may be an unnecessary implementation detail and may restrict the possibility of growth. It is probably a better design abstraction to leave out the specific method. I dropped the callback function for the native agent, but we should also do the same for JavaAgents. - PR: https://git.openjdk.org/jdk/pull/12923
Re: RFR: 8257967: JFR: Events for loaded agents [v4]
The message from this sender included one or more files
which could not be scanned for virus detection; do not
open these files unless you are certain of the sender's intent.
--
On Wed, 8 Mar 2023 18:56:55 GMT, Markus Grönlund wrote:
>> Greetings,
>>
>> We are adding support to let JFR report on Agents.
>>
>> Design
>>
>> An Agent is a library that uses any instrumentation or profiling APIs. Most
>> agents are started and initialized on the command line, but agents can also
>> be loaded dynamically during runtime. Because command line agents initialize
>> during the VM startup sequence, they add to the overall startup time latency
>> in getting the VM ready. The events will report on the time the agent took
>> to initialize.
>>
>> A JavaAgent is an agent written in the Java programming language, using the
>> APIs in the package
>> [java.lang.instrument](https://docs.oracle.com/en/java/javase/19/docs/api/java.instrument/java/lang/instrument/package-summary.html)
>>
>> A JavaAgent is sometimes called a JPLIS agent, where the acronym JPLIS
>> stands for Java Programming Language Instrumentation Services.
>>
>> To report on JavaAgents, JFR will add the new event type jdk.JavaAgent and
>> events will look similar to these two examples:
>>
>> // Command line
>> jdk.JavaAgent {
>> startTime = 12:31:19.789 (2023-03-08)
>> name = "JavaAgent.jar"
>> options = "foo=bar"
>> initialization = 12:31:15.574 (2023-03-08)
>> initializationTime = 172 ms
>> initializationMethod = "premain"
>> }
>> // Dynamic load
>> jdk.JavaAgent {
>> startTime = 12:31:31.158 (2023-03-08)
>> name = "JavaAgent.jar"
>> options = "bar=baz"
>> initialization = 12:31:31.037 (2023-03-08)
>> initializationTime = 64,1 ms
>> initializationMethod = "agentmain"
>> }
>>
>> The jdk.JavaAgent event type is a JFR periodic event that iterates over
>> running Java agents.
>>
>> For a JavaAgent event, the agent's name will be the specific .jar file
>> containing the instrumentation code. The options will be the specific
>> options passed to the .jar file as part of launching the agent, for example,
>> on the command line: -javaagent: JavaAgent.jar=foo=bar
>>
>> The event will also detail which initialization method was invoked by the
>> JVM, "premain" for command line agents, and "agentmain" for agents loaded
>> dynamically.
>>
>> "initialization" is the timestamp the JVM invoked the initialization method,
>> and "initializationTime" is the duration of executing the initialization
>> method.
>>
>> "startTime" represents the time the JFR framework issued the periodic event;
>> hence "initialization" will be earlier than "startTime".
>>
>> An agent can also be written in a native programming language using the [JVM
>> Tools Interface
>> (JVMTI)](https://docs.oracle.com/en/java/javase/19/docs/specs/jvmti.html).
>> This kind of agent, sometimes called a native agent, is a platform-specific
>> binary, sometimes referred to as a library, but here it means a .so or .dll
>> file.
>>
>> To report on native agents, JFR will add the new event type jdk.NativeAgent
>> and events will look similar to this example:
>>
>> jdk.NativeAgent {
>> startTime = 12:31:40.398 (2023-03-08)
>> name = "jdwp"
>> options = "transport=dt_socket,server=y,address=any,onjcmd=y"
>> path =
>> "c:\ade\github\openjdk\jdk\build\windows-x86_64-server-slowdebug\jdk\bin\jdwp.dll"
>> dynamic = false
>> initialization = 12:31:36.142 (2023-03-08)
>> initializationTime = 0,00184 ms
>> }
>>
>> The layout of the event type is very similar to the jdk.JavaAgent event, but
>> here the path to the native library is reported, and there is also a
>> denotation if the agent was loaded via the command line (dynamic = false) or
>> dynamically (dynamic = true).
>>
>> The initialization of a native agent is performed by invoking an
>> agent-specified callback routine which is not detailed in the event (for
>> now). The "initialization" is when the JVM sent or would have sent the JVMTI
>> VMInit event to a specified callback. "initializationTime" is the duration
>> to execute that specific callback. If no callback is specified for the JVMTI
>> VMInit event, the "initializationTime" will be 0.
>>
>> Implementation
>>
>> There has not existed a reification of a JavaAgent directly in the JVM, as
>> these are built on top of the JDK native library, "instrument", using a
>> many-to-one mapping. At the level of the JVM, the only representation of
>> agents after startup is through JvmtiEnv's, which agents request from the
>> JVM during startup and initialization — as such, mapping which JvmtiEnv
>> belongs to what JavaAgent was not possible before.
>>
>> Using implementation details of how the JDK native library "instrument"
>> interacts with the JVM, we can build this mapping to track what JvmtiEnv's
>> "belong" to what JavaAgent. This mapping now
Re: RFR: 8257967: JFR: Events for loaded agents [v4]
On Thu, 9 Mar 2023 00:23:39 GMT, David Holmes wrote: > > No need to load any JFR classes. > > I thought JFR was all Java-based these days. But if no Java involved then > that is good. Ehh, no. Far from it. > > No change to startup logic. > > I flagged a change in my comment above. Thanks, pls see my reply. - PR: https://git.openjdk.org/jdk/pull/12923
Re: RFR: 8257967: JFR: Events for loaded agents [v4]
The message from this sender included one or more files
which could not be scanned for virus detection; do not
open these files unless you are certain of the sender's intent.
--
On Wed, 8 Mar 2023 22:56:31 GMT, David Holmes wrote:
>> Markus Grönlund has updated the pull request incrementally with two
>> additional commits since the last revision:
>>
>> - remove JVMPI
>> - cleanup
>
> src/hotspot/share/runtime/threads.cpp line 338:
>
>> 336: if (EagerXrunInit && Arguments::init_libraries_at_startup()) {
>> 337: create_vm_init_libraries();
>> 338: }
>
> Not obvious where this went. Changes to the initialization order can be very
> problematic.
Thanks, David. Two calls to launch XRun agents are invoked during startup, and
they depend on the EagerXrunInit option. The !EagerXrunInit case is already
located in create_vm(), but the EagerXrunInit was located as the first entry in
initialize_java_lang_classes(), which I thought was tucked away a bit
unnecessarily.
I hoisted the EagerXrunInit from initialize_java_lang_classes() into to
create_vm(). It's now the call just before initialize_java_lang_classes().
This made it clearer, i.e. to have both calls located directly in create_vm().
-
PR: https://git.openjdk.org/jdk/pull/12923
Re: RFR: 8257967: JFR: Events for loaded agents [v4]
On Wed, 8 Mar 2023 23:28:52 GMT, Markus Grönlund wrote: >> Markus Grönlund has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - remove JVMPI >> - cleanup > > No need to load any JFR classes. No change to startup logic. > @mgronlun Why mark Java agents as command-line or dynamic using > `initializationMethod = "premain"/"agentMain"` and mark native agents using > `dynamic = true/false`? Why not use `dynamic` for both? Hi Andrew, that's a good question. I thought it could be derived in the JavaAgent case, because there are only two entry points, "premain" implies static and "agentmain" implies dynamic. For the native case, there is no information about the callback (I had it, but it depends on symbols), so the dynamic field is made explicit. It can also be added to the JavaAgent if that makes it clearer. - PR: https://git.openjdk.org/jdk/pull/12923
Re: RFR: 8257967: JFR: Events for loaded agents [v4]
The message from this sender included one or more files which could not be scanned for virus detection; do not open these files unless you are certain of the sender's intent. -- On Wed, 8 Mar 2023 23:28:52 GMT, Markus Grönlund wrote: >> Markus Grönlund has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - remove JVMPI >> - cleanup > > No need to load any JFR classes. No change to startup logic. @mgronlun Why mark Java agents as command-line or dynamic using `initializationMethod = "premain"/"agentMain"` and mark native agents using `dynamic = true/false`? Why not use `dynamic` for both? - PR: https://git.openjdk.org/jdk/pull/12923
Re: RFR: 8303814: getLastErrorString should avoid charset conversions
On Wed, 8 Mar 2023 23:23:33 GMT, Chris Plummer wrote:
>> This patch modifies the `getLastErrorString` method to return a `jstring`.
>> Thanks to that we can avoid unnecessary back and forth conversions between
>> Unicode and other charsets on Windows.
>>
>> Other changes include:
>> - the Windows implementation of `getLastErrorString` no longer checks
>> `errno`. I verified all uses of the method and confirmed that `errno` is not
>> used anywhere.
>> - While at it, I found and fixed a few calls to
>> `JNU_ThrowIOExceptionWithLastError` that were done in context where
>> `LastError` was not set.
>> - jdk.hotspot.agent was modified to use `JNU_ThrowByNameWithLastError` and
>> `JNU_ThrowByName` instead of `getLastErrorString`; the code is expected to
>> have identical behavior.
>> - zip_util was modified to return static messages instead of generated ones.
>> The generated messages were not observed anywhere, because they were
>> replaced by a static message in ZIP_Open, which is the only method used by
>> other native code.
>> - `getLastErrorString` is no longer exported by libjava.
>>
>> Tier1-3 tests continue to pass.
>>
>> No new automated regression test; testing this requires installing a
>> language pack that cannot be displayed in the current code page.
>> Tested this manually by installing Chinese language pack on English Windows
>> 11, selecting Chinese language, then checking if the message on exception
>> thrown by `InetAddress.getByName("nonexistent.local");` starts with
>> `"不知道这样的主机。"` (or
>> `"\u4e0d\u77e5\u9053\u8fd9\u6837\u7684\u4e3b\u673a\u3002"`). Without the
>> change, the exception message started with a row of question marks.
>
> We don't have a test for the SA changes you made. The best way to test it is
> with clhsdb. Run the following against a JVM pid:
>
> `$ jhsdb clhsdb --pid `
>
> Use "jstack -v" to get a native pc from a frame, and then try `disassemble`
> on the address. It most likely will produce an exception since it can't find
> hsdis, which is actually what we want to be testing in this case:
>
>
> hsdb> disassemble 0x7f38b371fca0
> Error: sun.jvm.hotspot.debugger.DebuggerException: hsdis-amd64.so: cannot
> open shared object file: No such file or directory
>
>
> You'll need to test separately on Windows and any unix platform.
Thanks @plummercj for the instructions. Here's the results:
Linux, with this change applied:
hsdb> disassemble 0x7f3484558da0
Error: sun.jvm.hotspot.debugger.DebuggerException: hsdis-amd64.so: cannot open
shared object file: No such file or directory
Windows, EN, with the change:
hsdb> disassemble 0x0107d4dae0c6
Error: sun.jvm.hotspot.debugger.DebuggerException: The specified module could
not be found
Windows, misconfigured CN, without the change:
hsdb> disassemble 0x0200d60de0b4
Error: sun.jvm.hotspot.debugger.DebuggerException: ?
Windows, misconfigured CN, with the change:
hsdb> disassemble 0x01fab996e0b4
Error: sun.jvm.hotspot.debugger.DebuggerException: 找不到指定的模块。
(note: I had to run `chcp 65001` in the console, otherwise the exception would
still display incorrectly)
-
PR: https://git.openjdk.org/jdk/pull/12922
