RFR: 8224162: assert(profile.count() == 0) failed: sanity in InlineTree::is_not_reached

[Vladimir Ivanov]

Nice catch, Jie!

I'm in favor of fixing the overflow instead and keep counts always 
non-negative. The root cause for negative counts is uint->int conversion 
and values can be normalized to max_int.

Moreover, on 64-bit platforms it's easy to catch (and fix) uint overflow 
since all the info is there - counts already are 64-bit (intptr_t).

Regarding the test:

test/hotspot/jtreg/compiler/profiling/TestProfileCounterOverflow.java:

   28  * @requires (vm.debug == true)

The test runs just fine with product binaries. So, I'd prefer to avoid 
limiting it only to debug.


   51         } else {
   52             // Sleep to wait for the compilation to be finished
   53             Thread.sleep(2000);
   54         }

You can just turn off background compilation instead (-Xbatch).


I was able to significantly speed up the test with the following chnages:

/**
  * @test
  * @run main/othervm -Xbatch -XX:-UseOnStackReplacement 
-XX:MaxTrivialSize=0 compiler.profiling.TestProfileCounterOverflow
  */

package compiler.profiling;

public class TestProfileCounterOverflow {
     public static void test(long iterations) throws Exception {
         for (long j = 0; j < iterations; j++) {
             call();
         }
     }
     public static void call() {}

     public static void main(String[] args) throws Exception {
         // trigger profiling on tier3
         for (int i = 0; i < 500; i++) {
             test(1);
         }

         test(Integer.MAX_VALUE + 10000L); // overflow call counter

         // trigger c2 compilation
         for (int i = 0; i < 10_000; i++) {
             test(1);
         }
         System.out.println("TEST PASSED");
     }
}

Best regards,
Vladimir Ivanov

On 22/05/2019 11:55, Jie Fu wrote:
> On 2019/5/22 上午11:57, Leonid Mesnik wrote:
>> Could you please add reproducer as regression test for fix. Please 
>> verify that test failed without fix and pass with fix.
> 
> Done.
> 
> Updated: http://cr.openjdk.java.net/~jiefu/8224162/webrev.03/
> 
> # Testing
> On an i7-8700 at 3.20GHz machine, the test results:
> -----------------------------------------------
> - fastdebug: make test 
> TEST="test/hotspot/jtreg/compiler/profiling/TestProfileCounterOverflow.java" 
> CONF=fastdebug
>    Before fix: failed, elapsed time: 37s
>    After  fix: pass,   elapsed time: 43s
> - slowdebug: make test 
> TEST="test/hotspot/jtreg/compiler/profiling/TestProfileCounterOverflow.java" 
> CONF=slowdebug
>    Before fix: failed, elapsed time: 38s
>    After  fix: pass,   elapsed time: 43s
> -----------------------------------------------
> 
> Please review it and give me some advice.
> 
> Thanks.
> Best regards,
> Jie
> 
>>
>> Leonid
>>
>>> On May 21, 2019, at 8:54 PM, Jie Fu <fujie at loongson.cn> wrote:
>>>
>>> On 2019/5/21 下午10:30, Vladimir Ivanov wrote:
>>>> It doesn't explain the failure since the assert is hit while parsing 
>>>> invoke* bytecode while type check failures are recorded for 
>>>> checkcast, aastore, and instanceof. Am I missing something important 
>>>> here?
>>>>
>>> Good question. I'm so sorry to make you confused.
>>>
>>> After a long time of digging into the code, I think this failure was 
>>> cause by the overflow of profile.cout.
>>>
>>> A reproducer is constructed here:
>>>   - http://cr.openjdk.java.net/~jiefu/8224162/CounterOverflow.java
>>>
>>> And I've changed the comment for the patch:
>>>   - http://cr.openjdk.java.net/~jiefu/8224162/webrev.02/
>>>
>>> Please review it and give me some advice.
>>>
>>> Thanks.
>>> Best regards,
>>> Jie
>>>
>>>
>