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) 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 This pull request has now been integrated. Changeset: 83bea26d Author: Patricio Chilano Mateo <pchilanom...@openjdk.org> URL: https://git.openjdk.org/jdk/commit/83bea26df453282d46afff333e006e8f9b7fb201 Stats: 635 lines in 69 files changed: 514 ins; 77 del; 44 mod 8300575: JVMTI support when using alternative virtual thread implementation Reviewed-by: lmesnik, sspitsyn, alanb ------------- PR: https://git.openjdk.org/jdk/pull/12512
