On Fri, 16 Apr 2021 20:30:15 GMT, Rafael Winterhalter <winterhal...@openjdk.org> wrote:
>> To allow agents the definition of auxiliary classes, an API is needed to >> allow this. Currently, this is often achieved by using `sun.misc.Unsafe` or >> `jdk.internal.misc.Unsafe` ever since the `defineClass` method was removed >> from `sun.misc.Unsafe`. > > Rafael Winterhalter has refreshed the contents of this pull request, and > previous commits have been removed. The incremental views will show > differences compared to the previous content of the PR. Setting '-javaagent' is mainly an operations problem. Many build tools do not allow to declare a test dependency this way as the life cycles are not laid out for it, the internal repository location might be machine dependent, for example, and it's difficult to point to a specific folder and file reliably. In this case, I'd say it would be easier to specify a parameter in the sense of '-Djdk.attach.allowAttachSelf=true' as it is a static value. This would however only work for build tools that initiate a new VM for running tests which is not overly attractive for simple builds due to the overhead. Of course, Maven, Gradle and similar tools could set this parameter by default for their own JVM, that part is likely overcomeable but it will certainly create confusion about how to run libraries that are omnipresent today and make the JVM ecosystem less approachable. What bothers me more is the tooling perspective for non-self-attached agents. For example, Aaeron offers a Java agent that adds plenty of debug logging to relevant lines of code. This affects method size and so forth, with Aaeron as a high-performance tool for banking and finance which is written very consciously with regards to the JIT, adding it directly was not feasible. Normally this logging is therefore thrown into a VM in retrospect, once an application starts failing and is already taken off the load balancer. For such a post-mortem, it would be rather annoying to realize that a JVM cannot be attached to with full capabilities if you forgot to set some flag. And often you did of course not consider the VM instance to fail, sometimes it takes months to get a JVM into this buggy state. This would be fairly inconvenient to face. Therefore, I really hope that the dynamic attach from 'outside' the VM will survive without imposing limits and that rather the self-attachment problem will be targeted as such. When I mention a 'jdk.test' module in the Mockito context, I am also rather hoping to improve performance compared to convenience. The problem with '-Djdk.attach.allowAttachSelf=true' is that you still need to start a new VM etc. Since Java 9, running single tests with Mockito has for example become much slower compared to Java 8. Despite the startup performance improvements in the JVM. If one could avoid the location-bound '-javaagent:...', but get the Instrumentation instance directly, I think this would render a real performance improvement in actual execution scenarios. Am Mi., 21. Apr. 2021 um 20:42 Uhr schrieb mlbridge[bot] < ***@***.***>: > *Mailing list message from Alan Bateman ***@***.***> on > core-libs-dev ***@***.***>:* > > On 20/04/2021 21:26, Rafael Winterhalter wrote: > > I have earlier proposed to offer a "jdk.test" module that > offers the Instrumentation instance via a simple API similar to Byte > Buddy's. The JVM would not load this module unless requested on the command > line. Build tools like Maven's surefire or Gradle's testrunner could then > standardize on loading this module as a convention to give access to this > test module by default such that libraries like Mockito could continue to > function out of the box without the libraries functioning on a standard VM > without extra configuration. As far as I know, mainly test libraries need > this API. This would also emphasise that Mockito and others are meant for > testing and fewer people would abuse it for production applications. People > would also have an explicit means of running a JVM for a production > application or for executing a test. > > Helping testing is good but a jdk.test module that hands out the > Instrumentation object could be problematic. Is there a reason why the > runner for Mockito and other mocking frameworks can't specify -javaagent > when launching? I would expect at least some mocking scenarios to > require load time transformations (to drop the final modifier from some > API elements for example) so important to have the transformer set > before classes are loaded. > > As for adding the API, my thought is that if the Instrumentation API were > to throw exceptions on some methods/arguments for dynamic agents in the > future, for example for retransformClasses(Object.class), this breaking > change would then simply extend to the proposed "defineClass" method. In > this sense, the Instrumentation API already assumes full power, I find it > not problematic to add the missing bit to this API even if it was > restricted in the future in the same spirit as other methods of the API > would be. > > I think it would really hard to put this genie back into a bottle. It's > way more attractive to use that than the very agent oriented > redefineModule and retransformClasses. > > I mentioned JNI as it is a well-known approach to defining a class today, > using a minimal native binding to an interface that directly calls down to > JNI's: > > jclass DefineClass(JNIEnv *env, const char *name, jobject loader, const > jbyte *buf, jsize bufLen); > > This interface can then simply be used to define any class just as I > propse, even when not writing an agent or attaching. This method makes > class definitions also already trivial for JVMTI agents compared to Java > agents. Unless restricting JNI, the defineClass method is already a low > hanging fruit, but at the cost of having to maintain a tiny bit of native > code. > > Sure, if you are using native code then you have the full power of JVM > TI and JNI available. Project Panama is exploring how to restrict access > to native code, I think too early to say how this might extend to JNI. > > -Alan > > — > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub > <https://github.com/openjdk/jdk/pull/3546#issuecomment-824277041>, or > unsubscribe > <https://github.com/notifications/unsubscribe-auth/ABCIA4HHMA67R24GWUPFVATTJ4MBRANCNFSM43BSDEGQ> > . > ------------- PR: https://git.openjdk.java.net/jdk/pull/3546
