Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

2019-12-09 Thread Laurence Cable

inline...

On 12/6/19 6:12 PM, serguei.spit...@oracle.com wrote:

On 12/6/19 17:24, Daniel D. Daugherty wrote:

On 12/6/19 6:26 PM, serguei.spit...@oracle.com wrote:

On 12/6/19 13:52, Chris Plummer wrote:

On 12/6/19 1:18 PM, serguei.spit...@oracle.com wrote:

On 12/6/19 11:07, Chris Plummer wrote:

On 12/5/19 6:45 PM, David Holmes wrote:

Hi Serguei,

On 6/12/2019 11:31 am, serguei.spit...@oracle.com wrote:

Hi Chris and Alex,

(I've also included Dan, David and Dean to the mailing list)

We have to reach a consensus about this.


This is just part of a much broader issue with JVM TI that I 
tried to have a discussion started based on Richard Reingruber's 
proposals around Escape Analysis:


http://mail.openjdk.java.net/pipermail/serviceability-dev/2019-September/029285.html 



Unfortunately that discussion did not get much traction.
Hmm. I have the emails that precede yours above, but not that 
one. Not sure how what happened. Just read through it and it did 
give me one thought.


Consider a model where the program is designed drive behavior of 
the agent, triggering the agent to do certain things by having 
the program do certain things. Normally an agent monitors the 
application, but in this case the application is purposefully 
controlling actions performed by the agent. If code is elided 
from the program, then the agent no longer performs as expected. 
It's a kind of backwards jvmti programming model, and you may ask 
why would anyone do this. I'm not sure if there's a good reason 
for it, but should it be expected to work given how the spec is 
written?


My interpretation is that the current JVM TI PopFrame behavior 
does not break this model.
The spec says: "any changes to the arguments, which occurred in 
the called method, remain;"
As the code was eliminated by the compiler then no changes to this 
argument occurred.
So, the PopFrame behavior follows the spec. So, I think, the 
option #2 is not right. But it depends on our basic philosophy.
If the developer wants to control the agent then the program has 
to be designed to do something meaningful that is not going to be 
optimized out by the JIT compiler.
You misunderstood my point. What I'm saying is that someone might 
do something like assign to a local with the specific intent of 
having that trigger a jmvti event, with the specific intent of 
having the agent perform some expected action as a result. Think of 
it as being a trigger for the agent, not as the agent monitoring 
the app. For example, you could right a program + agent, and 
setting a specific local in the program triggers the agent to turn 
on a light, and setting some other local turns it off. Absurd, but 
possible, and maybe there are less absurd applications.


I think, I understood your point correctly.
Your point is that the code that can be eliminated (e.g. local++) is 
not that meaningless as it seems to be.
My point is that there are still other more reliable ways to trigger 
the agent.
So that relying on something that can be eliminated by JIT compilers 
is not important to support.


You are making the assumption that the agent author understands what
Java code/variables *might* be eliminated by the JIT compiler. I don't
think that's a good assumption. I might have code that does a really
complicated thing in a local variable that is only useful to the
agent itself. The JIT will see that the local variable cannot escape
the function and is not used outside the function (as far as it can
see) so it will elide the local variable and the code that was used
to generated the local result in the variable.

If that local result happens to be some computation that the agent
needed to see to do its next operation...


Thank you for sharing your point.
I'm not insisting on my assumptions here, just not sure this is more 
important than allowing optimizations.

Do you actually think this use case needs to be supported?

In general, to identify our philosophy about interaction between JIT 
compiler

code elimination and JVM TI we need to make some assumptions.


Let's temporarily put JVM TI out of scope.
Are there any assumptions when JIT compilers eliminate some code?
Is it based on some vision what code is observable?
If it was decided some code can be eliminated then is it JVM TI only 
that breaks such assumptions about observability?


If so, then such optimizations can be disabled at some level.
Then we end up debugging/profiling/monitoring, and finally, observing 
a slightly different application.

Are we Okay with this? Do we need any compromises here?
Maybe we need more flags to control the JIT compiler behavior.
I would say that "in general" (not Java specific) there is an implicit 
assumption that compiler optimization and "debugging" are diametrically 
opposed to each other, and thus one cannot assume/expect that either can 
transparently co-exist, you either optimize or you debug,
but there is a sliding scale between the two extremes, fully optimized 
and no 

Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

2019-12-06 Thread Chris Plummer

On 12/6/19 6:12 PM, serguei.spit...@oracle.com wrote:

On 12/6/19 17:24, Daniel D. Daugherty wrote:

On 12/6/19 6:26 PM, serguei.spit...@oracle.com wrote:

On 12/6/19 13:52, Chris Plummer wrote:

On 12/6/19 1:18 PM, serguei.spit...@oracle.com wrote:

On 12/6/19 11:07, Chris Plummer wrote:

On 12/5/19 6:45 PM, David Holmes wrote:

Hi Serguei,

On 6/12/2019 11:31 am, serguei.spit...@oracle.com wrote:

Hi Chris and Alex,

(I've also included Dan, David and Dean to the mailing list)

We have to reach a consensus about this.


This is just part of a much broader issue with JVM TI that I 
tried to have a discussion started based on Richard Reingruber's 
proposals around Escape Analysis:


http://mail.openjdk.java.net/pipermail/serviceability-dev/2019-September/029285.html 



Unfortunately that discussion did not get much traction.
Hmm. I have the emails that precede yours above, but not that 
one. Not sure how what happened. Just read through it and it did 
give me one thought.


Consider a model where the program is designed drive behavior of 
the agent, triggering the agent to do certain things by having 
the program do certain things. Normally an agent monitors the 
application, but in this case the application is purposefully 
controlling actions performed by the agent. If code is elided 
from the program, then the agent no longer performs as expected. 
It's a kind of backwards jvmti programming model, and you may ask 
why would anyone do this. I'm not sure if there's a good reason 
for it, but should it be expected to work given how the spec is 
written?


My interpretation is that the current JVM TI PopFrame behavior 
does not break this model.
The spec says: "any changes to the arguments, which occurred in 
the called method, remain;"
As the code was eliminated by the compiler then no changes to this 
argument occurred.
So, the PopFrame behavior follows the spec. So, I think, the 
option #2 is not right. But it depends on our basic philosophy.
If the developer wants to control the agent then the program has 
to be designed to do something meaningful that is not going to be 
optimized out by the JIT compiler.
You misunderstood my point. What I'm saying is that someone might 
do something like assign to a local with the specific intent of 
having that trigger a jmvti event, with the specific intent of 
having the agent perform some expected action as a result. Think of 
it as being a trigger for the agent, not as the agent monitoring 
the app. For example, you could right a program + agent, and 
setting a specific local in the program triggers the agent to turn 
on a light, and setting some other local turns it off. Absurd, but 
possible, and maybe there are less absurd applications.


I think, I understood your point correctly.
Your point is that the code that can be eliminated (e.g. local++) is 
not that meaningless as it seems to be.
My point is that there are still other more reliable ways to trigger 
the agent.
So that relying on something that can be eliminated by JIT compilers 
is not important to support.


You are making the assumption that the agent author understands what
Java code/variables *might* be eliminated by the JIT compiler. I don't
think that's a good assumption. I might have code that does a really
complicated thing in a local variable that is only useful to the
agent itself. The JIT will see that the local variable cannot escape
the function and is not used outside the function (as far as it can
see) so it will elide the local variable and the code that was used
to generated the local result in the variable.

If that local result happens to be some computation that the agent
needed to see to do its next operation...


Thank you for sharing your point.
I'm not insisting on my assumptions here, just not sure this is more 
important than allowing optimizations.

Do you actually think this use case needs to be supported?

In general, to identify our philosophy about interaction between JIT 
compiler

code elimination and JVM TI we need to make some assumptions.


Let's temporarily put JVM TI out of scope.
Are there any assumptions when JIT compilers eliminate some code?
Is it based on some vision what code is observable?
If it was decided some code can be eliminated then is it JVM TI only 
that breaks such assumptions about observability?


If so, then such optimizations can be disabled at some level.
Then we end up debugging/profiling/monitoring, and finally, observing 
a slightly different application.

Are we Okay with this? Do we need any compromises here?
Maybe we need more flags to control the JIT compiler behavior.

Dan is restating the point I was making, but I also agree that unless 
someone can show us a useful application of that kind of use of jvmti 
events, I don't think we need to support it. We do need to clarify it in 
the spec however.


Chris

Thanks,
Serguei



Dan




Thanks,
Serguei


Chris





We have 3 options:

Option #1:
   The JIT optimization to delete a 

Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

2019-12-06 Thread Chris Plummer

On 12/6/19 3:26 PM, serguei.spit...@oracle.com wrote:

On 12/6/19 13:52, Chris Plummer wrote:

On 12/6/19 1:18 PM, serguei.spit...@oracle.com wrote:

On 12/6/19 11:07, Chris Plummer wrote:

On 12/5/19 6:45 PM, David Holmes wrote:

Hi Serguei,

On 6/12/2019 11:31 am, serguei.spit...@oracle.com wrote:

Hi Chris and Alex,

(I've also included Dan, David and Dean to the mailing list)

We have to reach a consensus about this.


This is just part of a much broader issue with JVM TI that I tried 
to have a discussion started based on Richard Reingruber's 
proposals around Escape Analysis:


http://mail.openjdk.java.net/pipermail/serviceability-dev/2019-September/029285.html 



Unfortunately that discussion did not get much traction.
Hmm. I have the emails that precede yours above, but not that one. 
Not sure how what happened. Just read through it and it did give me 
one thought.


Consider a model where the program is designed drive behavior of 
the agent, triggering the agent to do certain things by having the 
program do certain things. Normally an agent monitors the 
application, but in this case the application is purposefully 
controlling actions performed by the agent. If code is elided from 
the program, then the agent no longer performs as expected. It's a 
kind of backwards jvmti programming model, and you may ask why 
would anyone do this. I'm not sure if there's a good reason for it, 
but should it be expected to work given how the spec is written?


My interpretation is that the current JVM TI PopFrame behavior does 
not break this model.
The spec says: "any changes to the arguments, which occurred in the 
called method, remain;"
As the code was eliminated by the compiler then no changes to this 
argument occurred.
So, the PopFrame behavior follows the spec. So, I think, the option 
#2 is not right. But it depends on our basic philosophy.
If the developer wants to control the agent then the program has to 
be designed to do something meaningful that is not going to be 
optimized out by the JIT compiler.
You misunderstood my point. What I'm saying is that someone might do 
something like assign to a local with the specific intent of having 
that trigger a jmvti event, with the specific intent of having the 
agent perform some expected action as a result. Think of it as being 
a trigger for the agent, not as the agent monitoring the app. For 
example, you could right a program + agent, and setting a specific 
local in the program triggers the agent to turn on a light, and 
setting some other local turns it off. Absurd, but possible, and 
maybe there are less absurd applications.


I think, I understood your point correctly.
Your point is that the code that can be eliminated (e.g. local++) is 
not that meaningless as it seems to be.
My point is that there are still other more reliable ways to trigger 
the agent.
So that relying on something that can be eliminated by JIT compilers 
is not important to support.


Yes, I wasn't trying to imply that it is important. However, the spec 
should be clear about it.


Chris

Thanks,
Serguei


Chris





We have 3 options:

Option #1:
   The JIT optimization to delete a code which "looks useless"
   has to be disabled if can_pop_frame capability is enabled.
   Than this problem becomes a JIT compiler bug.

Option #2:
   Consider to relax the JVMTI PopFrame spec by changing it to 
something like:

   "Note however, that the original argument values are not
    preserved and can be changed by the called method;"
   Than this problem becomes a JVM TI spec bug.

Option #3:
   Consider it is Okay for compiler to eliminate useless code,
   so the argument values can be reinitialized by the PopFrame.
   Than this problem becomes just a test bug.


My preference is option #3.
The point is that if the arguments are not really used in
a method then restoring them to any values is a no-op.
It is really meaningless use case, so why should we care about it.
Is "restoring" the proper term here? I thought they were just left 
on the stack and reused on the subsequent invoke.


Agreed. The term "restoring" is not accurate here.

In fact I figured the reason for the language in the spec in the 
first place is to alleviate JVMTI from having to restore them to 
their original values, which is probably not even possible.


Right.



Thanks for setting that out clearly.

I'd like to agree this is particular case is a test bug. If we 
have a method:


int incr(int val) {
  val++;
  popFrameHere();
  return val;
}

then the change to the argument is necessary and must be 
preserved. In contrast:


void incr(int val) {
  val++;
  popFrameHere();
}

the change to the argument is meaningless and I would hope any 
decent JIT would simply elide it.
So, this goes back to my example above where the program is trying 
to elicit behavior from the agent. It's not meaningless in that 
case, but that doesn't mean I think we need to support it.


Even with this model it is possible and better to 

Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

2019-12-06 Thread serguei.spit...@oracle.com

On 12/6/19 17:24, Daniel D. Daugherty wrote:

On 12/6/19 6:26 PM, serguei.spit...@oracle.com wrote:

On 12/6/19 13:52, Chris Plummer wrote:

On 12/6/19 1:18 PM, serguei.spit...@oracle.com wrote:

On 12/6/19 11:07, Chris Plummer wrote:

On 12/5/19 6:45 PM, David Holmes wrote:

Hi Serguei,

On 6/12/2019 11:31 am, serguei.spit...@oracle.com wrote:

Hi Chris and Alex,

(I've also included Dan, David and Dean to the mailing list)

We have to reach a consensus about this.


This is just part of a much broader issue with JVM TI that I 
tried to have a discussion started based on Richard Reingruber's 
proposals around Escape Analysis:


http://mail.openjdk.java.net/pipermail/serviceability-dev/2019-September/029285.html 



Unfortunately that discussion did not get much traction.
Hmm. I have the emails that precede yours above, but not that one. 
Not sure how what happened. Just read through it and it did give 
me one thought.


Consider a model where the program is designed drive behavior of 
the agent, triggering the agent to do certain things by having the 
program do certain things. Normally an agent monitors the 
application, but in this case the application is purposefully 
controlling actions performed by the agent. If code is elided from 
the program, then the agent no longer performs as expected. It's a 
kind of backwards jvmti programming model, and you may ask why 
would anyone do this. I'm not sure if there's a good reason for 
it, but should it be expected to work given how the spec is written?


My interpretation is that the current JVM TI PopFrame behavior does 
not break this model.
The spec says: "any changes to the arguments, which occurred in the 
called method, remain;"
As the code was eliminated by the compiler then no changes to this 
argument occurred.
So, the PopFrame behavior follows the spec. So, I think, the option 
#2 is not right. But it depends on our basic philosophy.
If the developer wants to control the agent then the program has to 
be designed to do something meaningful that is not going to be 
optimized out by the JIT compiler.
You misunderstood my point. What I'm saying is that someone might do 
something like assign to a local with the specific intent of having 
that trigger a jmvti event, with the specific intent of having the 
agent perform some expected action as a result. Think of it as being 
a trigger for the agent, not as the agent monitoring the app. For 
example, you could right a program + agent, and setting a specific 
local in the program triggers the agent to turn on a light, and 
setting some other local turns it off. Absurd, but possible, and 
maybe there are less absurd applications.


I think, I understood your point correctly.
Your point is that the code that can be eliminated (e.g. local++) is 
not that meaningless as it seems to be.
My point is that there are still other more reliable ways to trigger 
the agent.
So that relying on something that can be eliminated by JIT compilers 
is not important to support.


You are making the assumption that the agent author understands what
Java code/variables *might* be eliminated by the JIT compiler. I don't
think that's a good assumption. I might have code that does a really
complicated thing in a local variable that is only useful to the
agent itself. The JIT will see that the local variable cannot escape
the function and is not used outside the function (as far as it can
see) so it will elide the local variable and the code that was used
to generated the local result in the variable.

If that local result happens to be some computation that the agent
needed to see to do its next operation...


Thank you for sharing your point.
I'm not insisting on my assumptions here, just not sure this is more 
important than allowing optimizations.

Do you actually think this use case needs to be supported?

In general, to identify our philosophy about interaction between JIT 
compiler

code elimination and JVM TI we need to make some assumptions.


Let's temporarily put JVM TI out of scope.
Are there any assumptions when JIT compilers eliminate some code?
Is it based on some vision what code is observable?
If it was decided some code can be eliminated then is it JVM TI only 
that breaks such assumptions about observability?


If so, then such optimizations can be disabled at some level.
Then we end up debugging/profiling/monitoring, and finally, observing a 
slightly different application.

Are we Okay with this? Do we need any compromises here?
Maybe we need more flags to control the JIT compiler behavior.

Thanks,
Serguei



Dan




Thanks,
Serguei


Chris





We have 3 options:

Option #1:
   The JIT optimization to delete a code which "looks useless"
   has to be disabled if can_pop_frame capability is enabled.
   Than this problem becomes a JIT compiler bug.

Option #2:
   Consider to relax the JVMTI PopFrame spec by changing it to 
something like:

   "Note however, that the original argument values are not
    

Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

2019-12-06 Thread Daniel D. Daugherty

On 12/6/19 6:26 PM, serguei.spit...@oracle.com wrote:

On 12/6/19 13:52, Chris Plummer wrote:

On 12/6/19 1:18 PM, serguei.spit...@oracle.com wrote:

On 12/6/19 11:07, Chris Plummer wrote:

On 12/5/19 6:45 PM, David Holmes wrote:

Hi Serguei,

On 6/12/2019 11:31 am, serguei.spit...@oracle.com wrote:

Hi Chris and Alex,

(I've also included Dan, David and Dean to the mailing list)

We have to reach a consensus about this.


This is just part of a much broader issue with JVM TI that I tried 
to have a discussion started based on Richard Reingruber's 
proposals around Escape Analysis:


http://mail.openjdk.java.net/pipermail/serviceability-dev/2019-September/029285.html 



Unfortunately that discussion did not get much traction.
Hmm. I have the emails that precede yours above, but not that one. 
Not sure how what happened. Just read through it and it did give me 
one thought.


Consider a model where the program is designed drive behavior of 
the agent, triggering the agent to do certain things by having the 
program do certain things. Normally an agent monitors the 
application, but in this case the application is purposefully 
controlling actions performed by the agent. If code is elided from 
the program, then the agent no longer performs as expected. It's a 
kind of backwards jvmti programming model, and you may ask why 
would anyone do this. I'm not sure if there's a good reason for it, 
but should it be expected to work given how the spec is written?


My interpretation is that the current JVM TI PopFrame behavior does 
not break this model.
The spec says: "any changes to the arguments, which occurred in the 
called method, remain;"
As the code was eliminated by the compiler then no changes to this 
argument occurred.
So, the PopFrame behavior follows the spec. So, I think, the option 
#2 is not right. But it depends on our basic philosophy.
If the developer wants to control the agent then the program has to 
be designed to do something meaningful that is not going to be 
optimized out by the JIT compiler.
You misunderstood my point. What I'm saying is that someone might do 
something like assign to a local with the specific intent of having 
that trigger a jmvti event, with the specific intent of having the 
agent perform some expected action as a result. Think of it as being 
a trigger for the agent, not as the agent monitoring the app. For 
example, you could right a program + agent, and setting a specific 
local in the program triggers the agent to turn on a light, and 
setting some other local turns it off. Absurd, but possible, and 
maybe there are less absurd applications.


I think, I understood your point correctly.
Your point is that the code that can be eliminated (e.g. local++) is 
not that meaningless as it seems to be.
My point is that there are still other more reliable ways to trigger 
the agent.
So that relying on something that can be eliminated by JIT compilers 
is not important to support.


You are making the assumption that the agent author understands what
Java code/variables *might* be eliminated by the JIT compiler. I don't
think that's a good assumption. I might have code that does a really
complicated thing in a local variable that is only useful to the
agent itself. The JIT will see that the local variable cannot escape
the function and is not used outside the function (as far as it can
see) so it will elide the local variable and the code that was used
to generated the local result in the variable.

If that local result happens to be some computation that the agent
needed to see to do its next operation...

Dan




Thanks,
Serguei


Chris





We have 3 options:

Option #1:
   The JIT optimization to delete a code which "looks useless"
   has to be disabled if can_pop_frame capability is enabled.
   Than this problem becomes a JIT compiler bug.

Option #2:
   Consider to relax the JVMTI PopFrame spec by changing it to 
something like:

   "Note however, that the original argument values are not
    preserved and can be changed by the called method;"
   Than this problem becomes a JVM TI spec bug.

Option #3:
   Consider it is Okay for compiler to eliminate useless code,
   so the argument values can be reinitialized by the PopFrame.
   Than this problem becomes just a test bug.


My preference is option #3.
The point is that if the arguments are not really used in
a method then restoring them to any values is a no-op.
It is really meaningless use case, so why should we care about it.
Is "restoring" the proper term here? I thought they were just left 
on the stack and reused on the subsequent invoke.


Agreed. The term "restoring" is not accurate here.

In fact I figured the reason for the language in the spec in the 
first place is to alleviate JVMTI from having to restore them to 
their original values, which is probably not even possible.


Right.



Thanks for setting that out clearly.

I'd like to agree this is particular case is a test bug. If we 
have a method:



Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

2019-12-06 Thread serguei.spit...@oracle.com
The PopFrame together with RedefineClasses is a part of the JVM TI 
HotSwap feature.

The use case is to hot patch the methods.
If after class redefinition there are still some method frames then the 
PopFrame is an option to "refresh" such frames.

I agree, this is unreliable and dangerous.
But the whole class redefinition feature is somewhat dangerous. :)
It is because the responsibility is on the agents.
And there are many ways for the agents to break the methods execution 
semantics with redefinition.


Thanks,
Serguei


On 12/6/19 14:59, Dean Long wrote:
This might be a dumb question, but how is PopFrame used in practice? 
Re-invoking the method, especially with modified argument values seems 
dangerous.


dl




Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

2019-12-06 Thread serguei.spit...@oracle.com

On 12/6/19 13:52, Chris Plummer wrote:

On 12/6/19 1:18 PM, serguei.spit...@oracle.com wrote:

On 12/6/19 11:07, Chris Plummer wrote:

On 12/5/19 6:45 PM, David Holmes wrote:

Hi Serguei,

On 6/12/2019 11:31 am, serguei.spit...@oracle.com wrote:

Hi Chris and Alex,

(I've also included Dan, David and Dean to the mailing list)

We have to reach a consensus about this.


This is just part of a much broader issue with JVM TI that I tried 
to have a discussion started based on Richard Reingruber's 
proposals around Escape Analysis:


http://mail.openjdk.java.net/pipermail/serviceability-dev/2019-September/029285.html 



Unfortunately that discussion did not get much traction.
Hmm. I have the emails that precede yours above, but not that one. 
Not sure how what happened. Just read through it and it did give me 
one thought.


Consider a model where the program is designed drive behavior of the 
agent, triggering the agent to do certain things by having the 
program do certain things. Normally an agent monitors the 
application, but in this case the application is purposefully 
controlling actions performed by the agent. If code is elided from 
the program, then the agent no longer performs as expected. It's a 
kind of backwards jvmti programming model, and you may ask why would 
anyone do this. I'm not sure if there's a good reason for it, but 
should it be expected to work given how the spec is written?


My interpretation is that the current JVM TI PopFrame behavior does 
not break this model.
The spec says: "any changes to the arguments, which occurred in the 
called method, remain;"
As the code was eliminated by the compiler then no changes to this 
argument occurred.
So, the PopFrame behavior follows the spec. So, I think, the option 
#2 is not right. But it depends on our basic philosophy.
If the developer wants to control the agent then the program has to 
be designed to do something meaningful that is not going to be 
optimized out by the JIT compiler.
You misunderstood my point. What I'm saying is that someone might do 
something like assign to a local with the specific intent of having 
that trigger a jmvti event, with the specific intent of having the 
agent perform some expected action as a result. Think of it as being a 
trigger for the agent, not as the agent monitoring the app. For 
example, you could right a program + agent, and setting a specific 
local in the program triggers the agent to turn on a light, and 
setting some other local turns it off. Absurd, but possible, and maybe 
there are less absurd applications.


I think, I understood your point correctly.
Your point is that the code that can be eliminated (e.g. local++) is not 
that meaningless as it seems to be.
My point is that there are still other more reliable ways to trigger the 
agent.
So that relying on something that can be eliminated by JIT compilers is 
not important to support.


Thanks,
Serguei


Chris





We have 3 options:

Option #1:
   The JIT optimization to delete a code which "looks useless"
   has to be disabled if can_pop_frame capability is enabled.
   Than this problem becomes a JIT compiler bug.

Option #2:
   Consider to relax the JVMTI PopFrame spec by changing it to 
something like:

   "Note however, that the original argument values are not
    preserved and can be changed by the called method;"
   Than this problem becomes a JVM TI spec bug.

Option #3:
   Consider it is Okay for compiler to eliminate useless code,
   so the argument values can be reinitialized by the PopFrame.
   Than this problem becomes just a test bug.


My preference is option #3.
The point is that if the arguments are not really used in
a method then restoring them to any values is a no-op.
It is really meaningless use case, so why should we care about it.
Is "restoring" the proper term here? I thought they were just left 
on the stack and reused on the subsequent invoke.


Agreed. The term "restoring" is not accurate here.

In fact I figured the reason for the language in the spec in the 
first place is to alleviate JVMTI from having to restore them to 
their original values, which is probably not even possible.


Right.



Thanks for setting that out clearly.

I'd like to agree this is particular case is a test bug. If we have 
a method:


int incr(int val) {
  val++;
  popFrameHere();
  return val;
}

then the change to the argument is necessary and must be preserved. 
In contrast:


void incr(int val) {
  val++;
  popFrameHere();
}

the change to the argument is meaningless and I would hope any 
decent JIT would simply elide it.
So, this goes back to my example above where the program is trying 
to elicit behavior from the agent. It's not meaningless in that 
case, but that doesn't mean I think we need to support it.


Even with this model it is possible and better to do something 
meaningful to control the agent.

This model is very rare use case.
It is hard to justify a need to support it. :)



But we must have a consistent 

Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

2019-12-06 Thread Dean Long
This might be a dumb question, but how is PopFrame used in practice?  
Re-invoking the method, especially with modified argument values seems 
dangerous.


dl


Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

2019-12-06 Thread Chris Plummer

On 12/6/19 1:18 PM, serguei.spit...@oracle.com wrote:

On 12/6/19 11:07, Chris Plummer wrote:

On 12/5/19 6:45 PM, David Holmes wrote:

Hi Serguei,

On 6/12/2019 11:31 am, serguei.spit...@oracle.com wrote:

Hi Chris and Alex,

(I've also included Dan, David and Dean to the mailing list)

We have to reach a consensus about this.


This is just part of a much broader issue with JVM TI that I tried 
to have a discussion started based on Richard Reingruber's proposals 
around Escape Analysis:


http://mail.openjdk.java.net/pipermail/serviceability-dev/2019-September/029285.html 



Unfortunately that discussion did not get much traction.
Hmm. I have the emails that precede yours above, but not that one. 
Not sure how what happened. Just read through it and it did give me 
one thought.


Consider a model where the program is designed drive behavior of the 
agent, triggering the agent to do certain things by having the 
program do certain things. Normally an agent monitors the 
application, but in this case the application is purposefully 
controlling actions performed by the agent. If code is elided from 
the program, then the agent no longer performs as expected. It's a 
kind of backwards jvmti programming model, and you may ask why would 
anyone do this. I'm not sure if there's a good reason for it, but 
should it be expected to work given how the spec is written?


My interpretation is that the current JVM TI PopFrame behavior does 
not break this model.
The spec says: "any changes to the arguments, which occurred in the 
called method, remain;"
As the code was eliminated by the compiler then no changes to this 
argument occurred.
So, the PopFrame behavior follows the spec. So, I think, the option #2 
is not right. But it depends on our basic philosophy.
If the developer wants to control the agent then the program has to be 
designed to do something meaningful that is not going to be optimized 
out by the JIT compiler.
You misunderstood my point. What I'm saying is that someone might do 
something like assign to a local with the specific intent of having that 
trigger a jmvti event, with the specific intent of having the agent 
perform some expected action as a result. Think of it as being a trigger 
for the agent, not as the agent monitoring the app. For example, you 
could right a program + agent, and setting a specific local in the 
program triggers the agent to turn on a light, and setting some other 
local turns it off. Absurd, but possible, and maybe there are less 
absurd applications.


Chris





We have 3 options:

Option #1:
   The JIT optimization to delete a code which "looks useless"
   has to be disabled if can_pop_frame capability is enabled.
   Than this problem becomes a JIT compiler bug.

Option #2:
   Consider to relax the JVMTI PopFrame spec by changing it to 
something like:

   "Note however, that the original argument values are not
    preserved and can be changed by the called method;"
   Than this problem becomes a JVM TI spec bug.

Option #3:
   Consider it is Okay for compiler to eliminate useless code,
   so the argument values can be reinitialized by the PopFrame.
   Than this problem becomes just a test bug.


My preference is option #3.
The point is that if the arguments are not really used in
a method then restoring them to any values is a no-op.
It is really meaningless use case, so why should we care about it.
Is "restoring" the proper term here? I thought they were just left on 
the stack and reused on the subsequent invoke.


Agreed. The term "restoring" is not accurate here.

In fact I figured the reason for the language in the spec in the 
first place is to alleviate JVMTI from having to restore them to 
their original values, which is probably not even possible.


Right.



Thanks for setting that out clearly.

I'd like to agree this is particular case is a test bug. If we have 
a method:


int incr(int val) {
  val++;
  popFrameHere();
  return val;
}

then the change to the argument is necessary and must be preserved. 
In contrast:


void incr(int val) {
  val++;
  popFrameHere();
}

the change to the argument is meaningless and I would hope any 
decent JIT would simply elide it.
So, this goes back to my example above where the program is trying to 
elicit behavior from the agent. It's not meaningless in that case, 
but that doesn't mean I think we need to support it.


Even with this model it is possible and better to do something 
meaningful to control the agent.

This model is very rare use case.
It is hard to justify a need to support it. :)



But we must have a consistent approach to such things. What would 
happen if a breakpoint were to be placed on the instruction that 
uselessly modified the argument - would we still see the 
modification or would it be elided?
Breakpoints force interpreted mode for the method, although I suppose 
that's a hotspot implementation detail and not something a VM would 
be required to do. A VM that allows breakpoints in 

Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

2019-12-06 Thread serguei.spit...@oracle.com

On 12/6/19 11:07, Chris Plummer wrote:

On 12/5/19 6:45 PM, David Holmes wrote:

Hi Serguei,

On 6/12/2019 11:31 am, serguei.spit...@oracle.com wrote:

Hi Chris and Alex,

(I've also included Dan, David and Dean to the mailing list)

We have to reach a consensus about this.


This is just part of a much broader issue with JVM TI that I tried to 
have a discussion started based on Richard Reingruber's proposals 
around Escape Analysis:


http://mail.openjdk.java.net/pipermail/serviceability-dev/2019-September/029285.html 



Unfortunately that discussion did not get much traction.
Hmm. I have the emails that precede yours above, but not that one. Not 
sure how what happened. Just read through it and it did give me one 
thought.


Consider a model where the program is designed drive behavior of the 
agent, triggering the agent to do certain things by having the program 
do certain things. Normally an agent monitors the application, but in 
this case the application is purposefully controlling actions 
performed by the agent. If code is elided from the program, then the 
agent no longer performs as expected. It's a kind of backwards jvmti 
programming model, and you may ask why would anyone do this. I'm not 
sure if there's a good reason for it, but should it be expected to 
work given how the spec is written?


My interpretation is that the current JVM TI PopFrame behavior does not 
break this model.
The spec says: "any changes to the arguments, which occurred in the 
called method, remain;"
As the code was eliminated by the compiler then no changes to this 
argument occurred.
So, the PopFrame behavior follows the spec. So, I think, the option #2 
is not right. But it depends on our basic philosophy.
If the developer wants to control the agent then the program has to be 
designed to do something meaningful that is not going to be optimized 
out by the JIT compiler.





We have 3 options:

Option #1:
   The JIT optimization to delete a code which "looks useless"
   has to be disabled if can_pop_frame capability is enabled.
   Than this problem becomes a JIT compiler bug.

Option #2:
   Consider to relax the JVMTI PopFrame spec by changing it to 
something like:

   "Note however, that the original argument values are not
    preserved and can be changed by the called method;"
   Than this problem becomes a JVM TI spec bug.

Option #3:
   Consider it is Okay for compiler to eliminate useless code,
   so the argument values can be reinitialized by the PopFrame.
   Than this problem becomes just a test bug.


My preference is option #3.
The point is that if the arguments are not really used in
a method then restoring them to any values is a no-op.
It is really meaningless use case, so why should we care about it.
Is "restoring" the proper term here? I thought they were just left on 
the stack and reused on the subsequent invoke.


Agreed. The term "restoring" is not accurate here.

In fact I figured the reason for the language in the spec in the first 
place is to alleviate JVMTI from having to restore them to their 
original values, which is probably not even possible.


Right.



Thanks for setting that out clearly.

I'd like to agree this is particular case is a test bug. If we have a 
method:


int incr(int val) {
  val++;
  popFrameHere();
  return val;
}

then the change to the argument is necessary and must be preserved. 
In contrast:


void incr(int val) {
  val++;
  popFrameHere();
}

the change to the argument is meaningless and I would hope any decent 
JIT would simply elide it.
So, this goes back to my example above where the program is trying to 
elicit behavior from the agent. It's not meaningless in that case, but 
that doesn't mean I think we need to support it.


Even with this model it is possible and better to do something 
meaningful to control the agent.

This model is very rare use case.
It is hard to justify a need to support it. :)



But we must have a consistent approach to such things. What would 
happen if a breakpoint were to be placed on the instruction that 
uselessly modified the argument - would we still see the modification 
or would it be elided?
Breakpoints force interpreted mode for the method, although I suppose 
that's a hotspot implementation detail and not something a VM would be 
required to do. A VM that allows breakpoints in compiled methods has 
the potential to miss the breakpoint if code is elided.


Also, what if you put a breakpoint in a method, the call to it is 
elided. You would never hit the breakpoint. That could cause some 
serious head scratching for a debugger user if they know the code 
doing the method call is "executed".


If the method is not actually being called then missing breakpoints 
there gives a clue what is going on.
Otherwise, it will cause cause some serious head scratching for a 
debugger user.

In general, my preference would be to debug actual behavior.
It is not good we have no support breakpoints in compiled methods.




And how do 

Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

2019-12-06 Thread Chris Plummer

On 12/5/19 6:45 PM, David Holmes wrote:

Hi Serguei,

On 6/12/2019 11:31 am, serguei.spit...@oracle.com wrote:

Hi Chris and Alex,

(I've also included Dan, David and Dean to the mailing list)

We have to reach a consensus about this.


This is just part of a much broader issue with JVM TI that I tried to 
have a discussion started based on Richard Reingruber's proposals 
around Escape Analysis:


http://mail.openjdk.java.net/pipermail/serviceability-dev/2019-September/029285.html 



Unfortunately that discussion did not get much traction.
Hmm. I have the emails that precede yours above, but not that one. Not 
sure how what happened. Just read through it and it did give me one 
thought. Consider a model where the program is designed drive behavior 
of the agent, triggering the agent to do certain things by having the 
program do certain things. Normally an agent monitors the application, 
but in this case the application is purposefully controlling actions 
performed by the agent. If code is elided from the program, then the 
agent no longer performs as expected. It's a kind of backwards jvmti 
programming model, and you may ask why would anyone do this. I'm not 
sure if there's a good reason for it, but should it be expected to work 
given how the spec is written?



We have 3 options:

Option #1:
   The JIT optimization to delete a code which "looks useless"
   has to be disabled if can_pop_frame capability is enabled.
   Than this problem becomes a JIT compiler bug.

Option #2:
   Consider to relax the JVMTI PopFrame spec by changing it to 
something like:

   "Note however, that the original argument values are not
    preserved and can be changed by the called method;"
   Than this problem becomes a JVM TI spec bug.

Option #3:
   Consider it is Okay for compiler to eliminate useless code,
   so the argument values can be reinitialized by the PopFrame.
   Than this problem becomes just a test bug.


My preference is option #3.
The point is that if the arguments are not really used in
a method then restoring them to any values is a no-op.
It is really meaningless use case, so why should we care about it.
Is "restoring" the proper term here? I thought they were just left on 
the stack and reused on the subsequent invoke. In fact I figured the 
reason for the language in the spec in the first place is to alleviate 
JVMTI from having to restore them to their original values, which is 
probably not even possible.


Thanks for setting that out clearly.

I'd like to agree this is particular case is a test bug. If we have a 
method:


int incr(int val) {
  val++;
  popFrameHere();
  return val;
}

then the change to the argument is necessary and must be preserved. In 
contrast:


void incr(int val) {
  val++;
  popFrameHere();
}

the change to the argument is meaningless and I would hope any decent 
JIT would simply elide it.
So, this goes back to my example above where the program is trying to 
elicit behavior from the agent. It's not meaningless in that case, but 
that doesn't mean I think we need to support it.


But we must have a consistent approach to such things. What would 
happen if a breakpoint were to be placed on the instruction that 
uselessly modified the argument - would we still see the modification 
or would it be elided?
Breakpoints force interpreted mode for the method, although I suppose 
that's a hotspot implementation detail and not something a VM would be 
required to do. A VM that allows breakpoints in compiled methods has the 
potential to miss the breakpoint if code is elided.


Also, what if you put a breakpoint in a method, the call to it is 
elided. You would never hit the breakpoint. That could cause some 
serious head scratching for a debugger user if they know the code doing 
the method call is "executed".




And how do C1 and C2 avoid this issue? Do they simply not optimise 
away the useless assignment? Or do they actively disable that 
optimization in this context?


We need, IMO, to establish the basic philosophy of how to manage JVM 
TI / JIT interactions, so we know what things must remain visible and 
which can be optimised away.


That said, changing the test allows us to defer having to reach that 
consensus.
Agreed. I think it's ok to work around the test issue as long as we keep 
this overall issue on the radar. Do we have a bug field for that?


thanks,

Chris


David
-


Thanks,
Serguei


On 11/11/19 3:17 AM, serguei.spit...@oracle.com wrote:

Hi Alex,

The fix itself looks Okay.
Minor: replace in the comment: "compiler don't drop" => "compiler 
doesn't drop".


However, we still have to reach a consensus on how we treat this 
issue (as Chris already commented).


Thanks,
Serguei


On 11/8/19 15:22, Alex Menkov wrote:

Hi all,

Please review the fix for
https://bugs.openjdk.java.net/browse/JDK-8215196
webrev:
http://cr.openjdk.java.net/~amenkov/jdk14/popframe_args/webrev/

Currently PopFrame is disabled with JVMCI by [1], so for testing I 
reverted [1] 

Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

2019-12-05 Thread serguei . spitsyn

Hi David,

Thank you for writing this down.
Totally agree with you here.


On 12/5/19 6:45 PM, David Holmes wrote:

Hi Serguei,

On 6/12/2019 11:31 am, serguei.spit...@oracle.com wrote:

Hi Chris and Alex,

(I've also included Dan, David and Dean to the mailing list)

We have to reach a consensus about this.


This is just part of a much broader issue with JVM TI that I tried to 
have a discussion started based on Richard Reingruber's proposals 
around Escape Analysis:


http://mail.openjdk.java.net/pipermail/serviceability-dev/2019-September/029285.html 



Unfortunately that discussion did not get much traction.


I've mentioned the general discussion you started about JIT compiler
optimizations in one of my previous replies to this review threads.
Sorry, I was busy with other things and was not able to participate in 
it properly.

But I'm looking forward to continue this when there is a chance.


We have 3 options:

Option #1:
   The JIT optimization to delete a code which "looks useless"
   has to be disabled if can_pop_frame capability is enabled.
   Than this problem becomes a JIT compiler bug.

Option #2:
   Consider to relax the JVMTI PopFrame spec by changing it to 
something like:

   "Note however, that the original argument values are not
    preserved and can be changed by the called method;"
   Than this problem becomes a JVM TI spec bug.

Option #3:
   Consider it is Okay for compiler to eliminate useless code,
   so the argument values can be reinitialized by the PopFrame.
   Than this problem becomes just a test bug.


My preference is option #3.
The point is that if the arguments are not really used in
a method then restoring them to any values is a no-op.
It is really meaningless use case, so why should we care about it.


Thanks for setting that out clearly.

I'd like to agree this is particular case is a test bug. If we have a 
method:


int incr(int val) {
  val++;
  popFrameHere();
  return val;
}

then the change to the argument is necessary and must be preserved. In 
contrast:


void incr(int val) {
  val++;
  popFrameHere();
}

the change to the argument is meaningless and I would hope any decent 
JIT would simply elide it.


But we must have a consistent approach to such things. What would 
happen if a breakpoint were to be placed on the instruction that 
uselessly modified the argument - would we still see the modification 
or would it be elided?


And how do C1 and C2 avoid this issue? Do they simply not optimise 
away the useless assignment? Or do they actively disable that 
optimization in this context?


We need, IMO, to establish the basic philosophy of how to manage JVM 
TI / JIT interactions, so we know what things must remain visible and 
which can be optimised away.


It is painful that we have not established it yet.



That said, changing the test allows us to defer having to reach that 
consensus.


Right.

Thanks,
Serguei


David
-


Thanks,
Serguei


On 11/11/19 3:17 AM, serguei.spit...@oracle.com wrote:

Hi Alex,

The fix itself looks Okay.
Minor: replace in the comment: "compiler don't drop" => "compiler 
doesn't drop".


However, we still have to reach a consensus on how we treat this 
issue (as Chris already commented).


Thanks,
Serguei


On 11/8/19 15:22, Alex Menkov wrote:

Hi all,

Please review the fix for
https://bugs.openjdk.java.net/browse/JDK-8215196
webrev:
http://cr.openjdk.java.net/~amenkov/jdk14/popframe_args/webrev/

Currently PopFrame is disabled with JVMCI by [1], so for testing I 
reverted [1] changes.


[1] https://bugs.openjdk.java.net/browse/JDK-8218025

--alex








Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

2019-12-05 Thread David Holmes

Hi Serguei,

On 6/12/2019 11:31 am, serguei.spit...@oracle.com wrote:

Hi Chris and Alex,

(I've also included Dan, David and Dean to the mailing list)

We have to reach a consensus about this.


This is just part of a much broader issue with JVM TI that I tried to 
have a discussion started based on Richard Reingruber's proposals around 
Escape Analysis:


http://mail.openjdk.java.net/pipermail/serviceability-dev/2019-September/029285.html

Unfortunately that discussion did not get much traction.


We have 3 options:

Option #1:
   The JIT optimization to delete a code which "looks useless"
   has to be disabled if can_pop_frame capability is enabled.
   Than this problem becomes a JIT compiler bug.

Option #2:
   Consider to relax the JVMTI PopFrame spec by changing it to something 
like:

   "Note however, that the original argument values are not
    preserved and can be changed by the called method;"
   Than this problem becomes a JVM TI spec bug.

Option #3:
   Consider it is Okay for compiler to eliminate useless code,
   so the argument values can be reinitialized by the PopFrame.
   Than this problem becomes just a test bug.


My preference is option #3.
The point is that if the arguments are not really used in
a method then restoring them to any values is a no-op.
It is really meaningless use case, so why should we care about it.


Thanks for setting that out clearly.

I'd like to agree this is particular case is a test bug. If we have a 
method:


int incr(int val) {
  val++;
  popFrameHere();
  return val;
}

then the change to the argument is necessary and must be preserved. In 
contrast:


void incr(int val) {
  val++;
  popFrameHere();
}

the change to the argument is meaningless and I would hope any decent 
JIT would simply elide it.


But we must have a consistent approach to such things. What would happen 
if a breakpoint were to be placed on the instruction that uselessly 
modified the argument - would we still see the modification or would it 
be elided?


And how do C1 and C2 avoid this issue? Do they simply not optimise away 
the useless assignment? Or do they actively disable that optimization in 
this context?


We need, IMO, to establish the basic philosophy of how to manage JVM TI 
/ JIT interactions, so we know what things must remain visible and which 
can be optimised away.


That said, changing the test allows us to defer having to reach that 
consensus.


David
-


Thanks,
Serguei


On 11/11/19 3:17 AM, serguei.spit...@oracle.com wrote:

Hi Alex,

The fix itself looks Okay.
Minor: replace in the comment: "compiler don't drop" => "compiler 
doesn't drop".


However, we still have to reach a consensus on how we treat this issue 
(as Chris already commented).


Thanks,
Serguei


On 11/8/19 15:22, Alex Menkov wrote:

Hi all,

Please review the fix for
https://bugs.openjdk.java.net/browse/JDK-8215196
webrev:
http://cr.openjdk.java.net/~amenkov/jdk14/popframe_args/webrev/

Currently PopFrame is disabled with JVMCI by [1], so for testing I 
reverted [1] changes.


[1] https://bugs.openjdk.java.net/browse/JDK-8218025

--alex






Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

2019-12-05 Thread serguei . spitsyn

Hi Chris and Alex,

(I've also included Dan, David and Dean to the mailing list)

We have to reach a consensus about this.

We have 3 options:

Option #1:
  The JIT optimization to delete a code which "looks useless"
  has to be disabled if can_pop_frame capability is enabled.
  Than this problem becomes a JIT compiler bug.

Option #2:
  Consider to relax the JVMTI PopFrame spec by changing it to something 
like:

  "Note however, that the original argument values are not
   preserved and can be changed by the called method;"
  Than this problem becomes a JVM TI spec bug.

Option #3:
  Consider it is Okay for compiler to eliminate useless code,
  so the argument values can be reinitialized by the PopFrame.
  Than this problem becomes just a test bug.


My preference is option #3.
The point is that if the arguments are not really used in
a method then restoring them to any values is a no-op.
It is really meaningless use case, so why should we care about it.

Thanks,
Serguei


On 11/11/19 3:17 AM, serguei.spit...@oracle.com wrote:

Hi Alex,

The fix itself looks Okay.
Minor: replace in the comment: "compiler don't drop" => "compiler 
doesn't drop".


However, we still have to reach a consensus on how we treat this issue 
(as Chris already commented).


Thanks,
Serguei


On 11/8/19 15:22, Alex Menkov wrote:

Hi all,

Please review the fix for
https://bugs.openjdk.java.net/browse/JDK-8215196
webrev:
http://cr.openjdk.java.net/~amenkov/jdk14/popframe_args/webrev/

Currently PopFrame is disabled with JVMCI by [1], so for testing I 
reverted [1] changes.


[1] https://bugs.openjdk.java.net/browse/JDK-8218025

--alex






Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

2019-11-11 Thread serguei.spit...@oracle.com

  
  
Hi Alex,
  
  The fix itself looks Okay.
  Minor: replace in the comment: "compiler don't
drop" => "compiler doesn't drop".

However, we still have to reach a consensus on how we treat this
issue (as Chris already commented).

Thanks,
Serguei

  
  On 11/8/19 15:22, Alex Menkov wrote:

Hi all,
  
  
  Please review the fix for
  
  https://bugs.openjdk.java.net/browse/JDK-8215196
  
  webrev:
  
  http://cr.openjdk.java.net/~amenkov/jdk14/popframe_args/webrev/
  
  
  Currently PopFrame is disabled with JVMCI by [1], so for testing I
  reverted [1] changes.
  
  
  [1] https://bugs.openjdk.java.net/browse/JDK-8218025
  
  
  --alex
  


  



Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

2019-11-11 Thread serguei.spit...@oracle.com

On 11/11/19 03:06, serguei.spit...@oracle.com wrote:

Hi Chris,


On 11/8/19 16:55, Chris Plummer wrote:

Hi Alex,

Comments below:

On 11/8/19 4:39 PM, Alex Menkov wrote:



On 11/08/2019 15:22, Alex Menkov wrote:

Hi all,

Please review the fix for
https://bugs.openjdk.java.net/browse/JDK-8215196
webrev:
http://cr.openjdk.java.net/~amenkov/jdk14/popframe_args/webrev/
I don't really see a resolution in the JDK-8215196 comments as to 
what is actually broken. Are we sure we want to fix this in the test, 
and not require different behavior by the compiler (and also clarify 
the spec)?


This is one more case to the topic "Should optimizations be observable 
for JVMTI agents?"

which was recently discussed on the serviceability-dev mailing list.
The question is about the following statement of the JVMTI PopFrame spec:
 "Note however, that any changes to the arguments, which occurred in 
the called method, remain;
  when execution continues, the first instruction to execute will be 
the invoke."


One point is we could consider this statement as a possible side 
affect which has to be preserved by the JIT compiler.
So, the optimization to delete such a code which "looks useless" has 
to be disabled.


Another point is that it is hard to understand why such a side effect 
can be really useful.
Maybe it was specified like this just because it does not make sense 
to preserve original argument values at the call side.
We can consider to relax the JVMTI PopFrame spec by changing it to 
something like:
 "Note however, that the original argument values are not preserved 
and can be changed by the called method;"


Forgot to list one more option which is:
  Consider it is Okay for compiler to eliminate useless code,
  so the argument values can be reinitialized by the PopFrame.
  Than this problem becomes just a test bug.


Thanks,
Serguei



Let's wait for other opinions.

Thanks,
Serguei



Currently PopFrame is disabled with JVMCI by [1], so for testing I 
reverted [1] changes.


Just to be clear - I temporary reverted [1] for test runs.

The description for JDK-8218025 says that the intention is to only 
disable these capabilities for JDK12. Is there a CR to re-enabled them?


thanks,

Chris

--alex



[1] https://bugs.openjdk.java.net/browse/JDK-8218025

--alex








Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

2019-11-11 Thread serguei.spit...@oracle.com

Hi Chris,


On 11/8/19 16:55, Chris Plummer wrote:

Hi Alex,

Comments below:

On 11/8/19 4:39 PM, Alex Menkov wrote:



On 11/08/2019 15:22, Alex Menkov wrote:

Hi all,

Please review the fix for
https://bugs.openjdk.java.net/browse/JDK-8215196
webrev:
http://cr.openjdk.java.net/~amenkov/jdk14/popframe_args/webrev/
I don't really see a resolution in the JDK-8215196 comments as to what 
is actually broken. Are we sure we want to fix this in the test, and 
not require different behavior by the compiler (and also clarify the 
spec)?


This is one more case to the topic "Should optimizations be observable 
for JVMTI agents?"

which was recently discussed on the serviceability-dev mailing list.
The question is about the following statement of the JVMTI PopFrame spec:
 "Note however, that any changes to the arguments, which occurred in 
the called method, remain;
  when execution continues, the first instruction to execute will be 
the invoke."


One point is we could consider this statement as a possible side affect 
which has to be preserved by the JIT compiler.
So, the optimization to delete such a code which "looks useless" has to 
be disabled.


Another point is that it is hard to understand why such a side effect 
can be really useful.
Maybe it was specified like this just because it does not make sense to 
preserve original argument values at the call side.
We can consider to relax the JVMTI PopFrame spec by changing it to 
something like:
 "Note however, that the original argument values are not preserved and 
can be changed by the called method;"


Let's wait for other opinions.

Thanks,
Serguei



Currently PopFrame is disabled with JVMCI by [1], so for testing I 
reverted [1] changes.


Just to be clear - I temporary reverted [1] for test runs.

The description for JDK-8218025 says that the intention is to only 
disable these capabilities for JDK12. Is there a CR to re-enabled them?


thanks,

Chris

--alex



[1] https://bugs.openjdk.java.net/browse/JDK-8218025

--alex






Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

2019-11-08 Thread Alex Menkov




On 11/08/2019 16:55, Chris Plummer wrote:

Hi Alex,

Comments below:

On 11/8/19 4:39 PM, Alex Menkov wrote:



On 11/08/2019 15:22, Alex Menkov wrote:

Hi all,

Please review the fix for
https://bugs.openjdk.java.net/browse/JDK-8215196
webrev:
http://cr.openjdk.java.net/~amenkov/jdk14/popframe_args/webrev/
I don't really see a resolution in the JDK-8215196 comments as to what 
is actually broken. Are we sure we want to fix this in the test, and not 
require different behavior by the compiler (and also clarify the spec)?


In the test activeMeth method changes its arguments values and then 
don't use them later.
I think dropping useless code is good compiler optimization and I'd 
prefer to not restrict to do the optimization.




Currently PopFrame is disabled with JVMCI by [1], so for testing I 
reverted [1] changes.


Just to be clear - I temporary reverted [1] for test runs.

The description for JDK-8218025 says that the intention is to only 
disable these capabilities for JDK12. Is there a CR to re-enabled them?


https://bugs.openjdk.java.net/browse/JDK-8218885
Unfortunately the problem why the capabilities were disabled are still 
unresolved and looks like won't be resolved in 14, so for now it's 
targeted to tbd.


--alex



thanks,

Chris

--alex



[1] https://bugs.openjdk.java.net/browse/JDK-8218025

--alex




Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

2019-11-08 Thread Chris Plummer

Hi Alex,

Comments below:

On 11/8/19 4:39 PM, Alex Menkov wrote:



On 11/08/2019 15:22, Alex Menkov wrote:

Hi all,

Please review the fix for
https://bugs.openjdk.java.net/browse/JDK-8215196
webrev:
http://cr.openjdk.java.net/~amenkov/jdk14/popframe_args/webrev/
I don't really see a resolution in the JDK-8215196 comments as to what 
is actually broken. Are we sure we want to fix this in the test, and not 
require different behavior by the compiler (and also clarify the spec)?


Currently PopFrame is disabled with JVMCI by [1], so for testing I 
reverted [1] changes.


Just to be clear - I temporary reverted [1] for test runs.

The description for JDK-8218025 says that the intention is to only 
disable these capabilities for JDK12. Is there a CR to re-enabled them?


thanks,

Chris

--alex



[1] https://bugs.openjdk.java.net/browse/JDK-8218025

--alex




Re: RFR: JDK-8215196: [Graal] vmTestbase/nsk/jvmti/PopFrame/popframe003/TestDescription.java fails with "changes for the arguments of the popped frame's method, did not remain current argument values"

2019-11-08 Thread Alex Menkov




On 11/08/2019 15:22, Alex Menkov wrote:

Hi all,

Please review the fix for
https://bugs.openjdk.java.net/browse/JDK-8215196
webrev:
http://cr.openjdk.java.net/~amenkov/jdk14/popframe_args/webrev/

Currently PopFrame is disabled with JVMCI by [1], so for testing I 
reverted [1] changes.


Just to be clear - I temporary reverted [1] for test runs.

--alex



[1] https://bugs.openjdk.java.net/browse/JDK-8218025

--alex