On Fri, 10 Feb 2023 14:50:36 GMT, Patricio Chilano Mateo <pchilanom...@openjdk.org> wrote:
> Please review the following change. Some of the JVMTI functions had to be > modified since they are not supported by the specs on virtual threads > (StopThread, RunAgentThread, GetCurrentThreadCpuTime, GetThreadCpuTime, > PopFrame, ForceEarlyReturnXXX) or shouldn't include virtual threads in the > results (GetAllThreads, GetAllStackTraces, GetThreadGroupChildren). Others > were modified because they should work on virtual thread but they weren't > (SuspendAllVirtualThreads/ResumeAllVirtualThreads). Also support for > VirtualThreadStart/VirtualThreadEnd events was added. > > There were a total of 71 tests under serviceability/jvmti/ that had the > vm.continuations requirement. Of those, 24 where under > serviceability/jvmti/vthread/. I was able to remove that requirement from 50 > tests. Most of those tests were already passing for the alternative vthread > implementation just by removing the check in jni_GetEnv for VMContinuations. > From the other 21 tests, 20 still fail with the changes either because they > actually test continuations or because of different assumptions in the tests > that don't hold true for the alternative vthread implementation. So for those > I left the vm.continuations requirement untouched (from those 20 tests there > are actually 4 tests from the thread/GetStackTrace/ family that are passing > because of a bug in the test, but I can fix that in another RFE). The other > remaining test is vthread/GetSetLocalTest/GetSetLocalTest.java which I see is > problemlisted. > > Regarding variable _is_bound_vthread, although it's handy as a cache to avoid > repeating the check, I mainly added it to avoid transitioning for > GetCurrentThreadCpuTime (we are native and cannot access oops). > > I added new test BoundVThreadTest.java which just checks for the > unsupported/supported behavior mentioned previously. For some extra basic > coverage I also added a new run with -XX:-VMContinuations on tests inside the > serviceability/jvmti/vthread/ directory that don't require vm.continuations > anymore. I could also add that for all the other tests in > serviceability/jvmti/ for which I removed the vm.continuations requirement. > > I run the patch through mach5 tiers 1-6 plus some extra local runs on the > relevant tests. > > Thanks, > Patricio src/hotspot/share/classfile/vmClassMacros.hpp line 92: > 90: do_klass(Thread_Constants_klass, > java_lang_Thread_Constants ) \ > 91: do_klass(ThreadGroup_klass, > java_lang_ThreadGroup ) \ > 92: do_klass(BasicVirtualThread_klass, > java_lang_BaseVirtualThread ) \ It should be BaseVirtualThread_klass rather BasicVirtualThread_klass (probably my fault when adding the alt implementation). ------------- PR: https://git.openjdk.org/jdk/pull/12512
