On Tue, 12 Dec 2023 23:54:43 GMT, Leonid Mesnik <lmes...@openjdk.org> wrote:
>> Serguei Spitsyn has updated the pull request incrementally with one >> additional commit since the last revision: >> >> review: (1) rename notifyJvmti method; (2) add try-final statements to >> VirtualThread methods > > src/hotspot/share/prims/jvm.cpp line 4013: > >> 4011: // Notification from VirtualThread about entering/exiting sync >> critical section. >> 4012: // Needed to avoid deadlocks with JVMTI suspend mechanism. >> 4013: JVM_ENTRY(void, JVM_VirtualThreadCriticalLock(JNIEnv* env, jobject >> vthread, jboolean enter)) > > the jobject vthread is not used. Can't be the method made static to reduce > the number of arguments? > It is the performance-critical code, I don't know if it is optimized by C2. Good question. In general, I'd like to keep this unified with the other `notiftJvmti` methods. Let me double check how it fits together. Also, I'm not sure how is going to impact the intrinsification. > src/hotspot/share/runtime/javaThread.hpp line 320: > >> 318: bool _is_in_VTMS_transition; // thread is >> in virtual thread mount state transition >> 319: bool _is_in_tmp_VTMS_transition; // thread is >> in temporary virtual thread mount state transition >> 320: bool _is_in_critical_section; // thread is >> in a locking critical section > > might make sense to add a comment, that his variable Is changed/read only by > current thread and no sync is needed. Good suggestion, thanks. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/17011#discussion_r1426643218 PR Review Comment: https://git.openjdk.org/jdk/pull/17011#discussion_r1426643663
