On 8/27/14, 3:33 PM, Daniel D. Daugherty wrote:
On 8/27/14 12:41 PM, serguei.spit...@oracle.com wrote:
On 8/27/14 6:54 AM, Daniel D. Daugherty wrote:
On 8/27/14 5:40 AM, serguei.spit...@oracle.com wrote:
On 8/20/14 3:45 PM, Daniel D. Daugherty wrote:
On 8/20/14 2:01 PM, Coleen Phillimore wrote:
On 8/20/14, 3:49 PM, serguei.spit...@oracle.com wrote:
If an EMCP method is not running, should we save it on a
previous version list anyway so that we can make it obsolete if
it's redefined and made obsolete?
I hope, Dan will catch me if I'm wrong...
I think, we should not.
An EMCP method can not be made obsolete if it is not running.
It should be this way otherwise we'd have to hang onto things
forever.
An EMCP method should only be made obsolete if a RedefineClasses() or
RetransformClasses() operation made it so. We should not be
leveraging
off the obsolete-ness attribute to solve a life-cycle problem.
In the pre-PGR world, we could trust GC to make a completely unused
EMCP method collectible and eventually our weak reference would go
away. Just because an EMCP method is not on a stack does not mean
that it is not used so we need a different way to determine whether
it is OK to no longer track an EMCP method.
Most likely, you are right.
But I'm not convinced yet. Sorry.
A convincing point would be a test that shows this behavior.
I understand that it is not an easy task to write such a test though.
However, such a test would serve nicely if we want a different way
to determine whether it is OK to no longer track an EMCP method.
Running the Serviceability stack of tests with the following
-XX:TraceRedefineClasses=NN flags:
// 0x00001000 | 4096 - detect calls to obsolete methods
// 0x00002000 | 8192 - fail a guarantee() in addition to
detection
will show failures of the guarantee(). I used to do this
periodically and then chase down the failures to make sure
only the "legitimate" races were happening. The reason that
we had to add these flags was to find all the places where
folks were caching methods in the VM. I think the last place
I found and fixed was in reflection.
Ok, thanks.
We threat this as buggy behavior, right?
Not necessarily. If it's because of a cached method that didn't
get updated to the new version, then that's a bug. However, if
it's because of a call that is in progress, then we have not
considered that a bug in the past.
I don't remember the exact details, but the dance is something
like this:
- jmethodID converted to methodHandle
- call to methodHandle prepared:
- methodHandle -> methodOop
- parameters marshalled
- methodOop converted to method impl
- method called
- somewhere in the middle of call sequence the
method is redefined
- jmethodID and methodHandle are updated to refer to the
new method, but we already converted to the methodOop
and the underlying method implementation for the final
stages of the call
- when we've actually called the now obsolete method,
the guarantee() mentioned above fires and we have a
false positive for the tracing code
Of course, now that we don't have methodOops, maybe this
doesn't happen anymore. I haven't done a test run with the
above flags enabled in quite some time...
Do you mean methodHandle or MethodHandle above? Or
java_lang_reflect_Method?
The methodOop in little m methodHandles are not updated to refer to the
new method, and Method* isn't either, so that really hasn't changed.
I'm not following the call sequence above. But yes, I believe we could
get into javaCalls::call() with an method, stop for a safepoint, and end
up calling an obsolete method. But that obsolete method is considered
already running at that stage because the methodHandle logic marks it as
such, so it's not considered a bug.
I ran the tests with the -XX:TraceRedefineClasses=0x2000 and didn't get
the guarantee though, but that doesn't mean much.
I'm reading these mails in reverse...
Thanks,
Coleen
Dan
Thanks,
Serguei
Dan
Thanks,
Serguei
BTW, I'm reviewing the webrev too, but probably it'd be better
to switch to updated webrev after it is ready.
Yes, this is a good idea. I figured out why I made emcp methods
obsolete, and I'm fixing that as well as Dan's comments. Thanks!
Cool! I'm looking forward to the next review.
Dan
Coleen
Thanks,
Serguei
