On Fri, 26 Aug 2022 09:25:52 GMT, Axel Boldt-Christmas <abold...@openjdk.org> wrote:
>> Maybe this is ok, but I want to look at it more. The reason these functions >> started out in codeCache is because they looked like other functions in >> codeCache. > > The encapsulation is about defining an API for deoptimization which can > enforce invariants and be safer to use. > ```C++ > // A DeoptimizationContext provides an API for deoptimizing CompileMethods > // The contract for using a DeoptimizationContext is as follows > // The context is active from the DeoptimizationContext objects creating > // until deoptimize() is called on the object. > // While the context is active, > // * no safepoint may be taken > // * any interaction with the context object must be done under > // the Compile_lock > // * deoptimize() must be called > // While the context is inactive > // * only interaction with the DeoptimizationContext object may > // calls to enqueued() and its destruction > > The idea with the API is that the logic of how compiled methods are selected > is done in a context which wants to use the API, like class redefinition, > class dependency changes, method handles, call site changes etc. > The goal was to move all the deoptimization logic from the codeCache to > deoptimization. The JTMTI code got moved to jvmtiRedefineClasses as it had > separate JVMTI specific logic. Storing old methods, etc. But I think some of > the code that got moved from codeCache to deoptimization should/could be > moved to instanceKlass, method, dependencies etc. as well. > The reason I only moved the JVMTI code is because it had side > effects(separate from deoptimization). While all the other were only help > functions which made dependency traversal easier. > > The only reason I feel reluctance about moving the JVMTI code to > deoptimization is because it then must be old_compiled_method_table aware. > > But if you have some reason why it should be in deoptimization I would like > to know. Okay, I looked at the change to jvmtiRedefineClasses.cpp in context and I like where you moved it. ------------- PR: https://git.openjdk.org/jdk/pull/9655
