On 12/16/19 8:13 AM, Robbin Ehn wrote:
Hi Coleen, in VM_RedefineClasses::doit:
This updates the breakpoints:
MetadataOnStackMark md_on_stack(/*walk_all_metadata*/true,
/*redefinition_walk*/true);
And this removes breakpoints:
for (int i = 0; i < _class_count; i++) {
redefine_single_class(_class_defs[i].klass, _scratch_classes[i],
thread);
}
So we skip updating, since we do remove them after we updated them.
But you are the expert here. Let me know if there is something I missed.
No, you are correct. The JVMTI spec says that the breakpoints are all
deleted. I'm remembering code that sets/clears breakpoints that has to
walk emcp methods, and set them there also. But redefinition does clear
them.
If the old Method* is still executing or referenced somehow, the other
metadata walking would find it anyway. So maybe this was never needed.
OopHandle just adds more code.
It doesn't. And if we want to make all native memory never point
directly to oops and point to oopStorage instead, having some
encapsulation makes that easier. It also makes it so that we don't have
to stare at oop* in data structures and wonder if we're going to miss
the mumble-fratz access and decorators that we need. ie:
http://cr.openjdk.java.net/~rehn/8235912/v1/webrev/src/hotspot/share/prims/jvmtiImpl.cpp.udiff.html
+ NativeAccess<>::oop_store(_class_holder, class_holder_oop);
This should probably be:
41 NativeAccess<IS_DEST_UNINITIALIZED>::oop_store(handle, obj);
You can leave out using OopHandle. I have a patch to add the missing
functionality and add it to your code. Actually, I was looking to see
how much OopHandle is used to see if it's helping anything and there is
a lot of code using it. Most of it is to hide oop* in ClassLoaderData.
This change otherwise looks great.
Thanks,
Coleen
Thanks for having a look, Robbin
On 12/16/19 1:32 PM, [email protected] wrote:
I have to think about this. Could there be breakpoints in old emcp
methods that we do not remove? The metadata_do function is trying
to keep old Methods from being deleted while there are still
references to them.
http://cr.openjdk.java.net/~rehn/8235912/v1/webrev/src/hotspot/share/prims/jvmtiImpl.hpp.udiff.html
+ oop* _class_holder; // keeps _method memory from being deallocated
We created the class OopHandle to encapsulate strong oopStorage
references, although it's missing oop_store. Can you use that?
Coleen
On 12/16/19 4:47 AM, Robbin Ehn wrote:
Hi all, please review.
From issue, https://bugs.openjdk.java.net/browse/JDK-8235912:
JvmtiBreakpoints are walked via VMThread oops_do (the breakpoint is
in a vm operation) before they are installed in the safeopint and
after they have been installed, walked with
JvmtiCurrentBreakpoints::oops_do().
By putting the class holder inside oopStorage there is no need for
this.
JvmtiCurrentBreakpoints::metadata_do is not needed because redefine
classes actually removes the breakpoints before updating them (so
there is no breakpoints to update).
We can just remove metadata_do.
I also removed some unused code.
Changeset:
http://cr.openjdk.java.net/~rehn/8235912/v1/webrev/
Passes several runs of nsk jvmti/jdi and t1-7.
Thanks, Robbin
