On Tue, 20 Aug 2024 17:28:12 GMT, Dean Long <dl...@openjdk.org> wrote:

>> Leonid Mesnik has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   fixed identation.
>
> src/hotspot/share/runtime/javaCalls.cpp line 403:
> 
>> 401:         } else {
>> 402:           // Since the call stub sets up like the interpreter we call 
>> the from_interpreted_entry
>> 403:           // so we can go compiled via a i2c.
> 
> Do you think removed comment "Otherwise initial entry method will always run 
> interpreted.", or it's understood and redundant?

Yes.  I believe that expression in the 'if'  clearly explain the reason of 
using interpreter. So comment is redundant.

> test/hotspot/jtreg/vmTestbase/nsk/jvmti/SetFieldModificationWatch/setfmodw001/TestDescription.java
>  line 57:
> 
>> 55:  *          /test/lib
>> 56:  * @run main/othervm/native -agentlib:setfmodw001 
>> -XX:TraceJVMTI=ec+,+ioe,+s -Xlog:jvmti=trace:file=vm.%p.log 
>> nsk.jvmti.SetFieldModificationWatch.setfmodw001
>> 57:  */
> 
> What's the reason for removing this test run?

I was not able to reproduce failure locally at the beginning. So I added this 
test run just to get more information about failure from CI runs for initial 
analysis. 
The information from logging clearly pointed that we just never got event while 
set Thread-1 to interprter_only mode correctly. 
However, it is not needed now so I am removing it.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/20587#discussion_r1723701025
PR Review Comment: https://git.openjdk.org/jdk/pull/20587#discussion_r1723704840

Reply via email to