<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<br>
<br>
<div class="moz-cite-prefix">On 12/16/19 8:13 AM, Robbin Ehn wrote:<br>
</div>
<blockquote type="cite"
cite="mid:e803bda4-c57c-6eec-45e7-c2a6028d3ae6@oracle.com">Hi
Coleen, in VM_RedefineClasses::doit:
<br>
<br>
This updates the breakpoints:
<br>
 MetadataOnStackMark md_on_stack(/*walk_all_metadata*/true,
/*redefinition_walk*/true);
<br>
<br>
And this removes breakpoints:
<br>
 for (int i = 0; i < _class_count; i++) {
<br>
   redefine_single_class(_class_defs[i].klass,
_scratch_classes[i], thread);
<br>
 }
<br>
<br>
So we skip updating, since we do remove them after we updated
them.
<br>
But you are the expert here. Let me know if there is something I
missed.
<br>
<br>
</blockquote>
<br>
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.<br>
<br>
If the old Method* is still executing or referenced somehow, the
other metadata walking would find it anyway. So maybe this was
never needed.<br>
<br>
<blockquote type="cite"
cite="mid:e803bda4-c57c-6eec-45e7-c2a6028d3ae6@oracle.com">OopHandle
just adds more code.
<br>
<br>
</blockquote>
<br>
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:<br>
<br>
<a
href="http://cr.openjdk.java.net/~rehn/8235912/v1/webrev/src/hotspot/share/prims/jvmtiImpl.cpp.udiff.html">http://cr.openjdk.java.net/~rehn/8235912/v1/webrev/src/hotspot/share/prims/jvmtiImpl.cpp.udiff.html</a><br>
<br>
<pre style="color: rgb(0, 0, 0); font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration-style: initial; text-decoration-color: initial;"><span class="new" style="color: blue; font-weight: normal;">+ NativeAccess<>::oop_store(_class_holder, class_holder_oop);</span>
</pre>
<br class="Apple-interchange-newline">
This should probably be:<br class="Apple-interchange-newline">
<pre style="color: rgb(0, 0, 0); font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; text-decoration-style: initial; text-decoration-color: initial;"> 41 NativeAccess<IS_DEST_UNINITIALIZED>::oop_store(handle, obj);
</pre>
<br class="Apple-interchange-newline">
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.<br>
<br>
This change otherwise looks great.<br>
Thanks,<br>
Coleen<br>
<br>
<br>
<blockquote type="cite"
cite="mid:e803bda4-c57c-6eec-45e7-c2a6028d3ae6@oracle.com">Thanks
for having a look, Robbin
<br>
<br>
On 12/16/19 1:32 PM, <a class="moz-txt-link-abbreviated" href="mailto:coleen.phillimore@oracle.com">coleen.phillimore@oracle.com</a> wrote:
<br>
<blockquote type="cite">
<br>
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.
<br>
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~rehn/8235912/v1/webrev/src/hotspot/share/prims/jvmtiImpl.hpp.udiff.html">http://cr.openjdk.java.net/~rehn/8235912/v1/webrev/src/hotspot/share/prims/jvmtiImpl.hpp.udiff.html</a>
<br>
<br>
+ oop* _class_holder; // keeps _method memory from being
deallocated
<br>
<br>
<br>
We created the class OopHandle to encapsulate strong oopStorage
references, although it's missing oop_store. Can you use that?
<br>
</blockquote>
<br>
<br>
<blockquote type="cite">
<br>
Coleen
<br>
<br>
On 12/16/19 4:47 AM, Robbin Ehn wrote:
<br>
<blockquote type="cite">Hi all, please review.
<br>
<br>
From issue, <a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8235912">https://bugs.openjdk.java.net/browse/JDK-8235912</a>:
<br>
<br>
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().
<br>
By putting the class holder inside oopStorage there is no need
for this.
<br>
<br>
JvmtiCurrentBreakpoints::metadata_do is not needed because
redefine classes actually removes the breakpoints before
updating them (so there is no breakpoints to update).
<br>
We can just remove metadata_do.
<br>
<br>
<br>
I also removed some unused code.
<br>
<br>
Changeset:
<br>
<a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~rehn/8235912/v1/webrev/">http://cr.openjdk.java.net/~rehn/8235912/v1/webrev/</a>
<br>
<br>
Passes several runs of nsk jvmti/jdi and t1-7.
<br>
<br>
Thanks, Robbin
<br>
</blockquote>
<br>
</blockquote>
</blockquote>
<br>
</body>
</html>