On Tue, 8 Nov 2022 04:45:17 GMT, David Holmes <dhol...@openjdk.org> wrote:
> This is a nice simplification of the VM side of the code! I do have to wonder > why this was implemented the way it was rather than doing the simple upcall > as you now do, but I suspect it was just performance, I don't think JVMTI GetThreadGroupChildren was ever performance critical, it's just that historically JVMTI functions haven't done many upcalls because of the potential side effects of events generated when executing Java code. I think the horse has bolted on this already, e.g. the JVMTI functions added in Java 9 all require calling into Java code. So I think it is a good simplification but a reminder that the set of "Universal errors" in the JVMTI spec may not be sufficient to cover the possible exceptions. As you point out, the current patch maps all errors/exceptions to JVMTI_ERROR_OUT_OF_MEMORY. It's the most likely reason for it to fail but it might stack overflow or something else. I note that JvmtiEnv::AddToSystemClassLoaderSearch maps errors/exceptions to JVMTI_ERROR_INTERNAL. Maybe we need an issue in JBS to track examine this issue as it's not specific to GetThreadGroupChildren. ------------- PR: https://git.openjdk.org/jdk/pull/11033