On Thu, 16 Feb 2023 16:59:05 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) 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 > > Patricio Chilano Mateo has updated the pull request incrementally with two > additional commits since the last revision: > > - restore state should be only for VirtualThreads > - add suspend check test/hotspot/jtreg/serviceability/jvmti/vthread/BoundVThreadTest/libBoundVThreadTest.cpp line 35: > 33: static int vthread_start_count = 0; > 34: static int vthread_end_count = 0; > 35: static bool passed = JNI_TRUE; Nit: It is better to rename: passed => status. ------------- PR: https://git.openjdk.org/jdk/pull/12512
