More on: Small static method marked not entrant, inlining reversed?

[Tom Rodriguez]

On Sep 22, 2010, at 2:30 AM, Rémi Forax wrote:

> Le 21/09/2010 21:48, Tom Rodriguez a écrit :
>> Thanks for the test case.  The reason it works well for C1 is that we have an explicit check for single implementor interfaces and insert an extra checkcast to confirm that the type really is the type we expect.  The extra check is needed since interface types aren't actually checked by the verifier.  In your example the extra check isn't actually needed since all array stores are actually type checked so any type loaded from the array must actually implement the interface.  C2 generally relies on type profiling to get cases like this but since type profiling is tracking the type of the receiver instead of the actual method being invoked it doesn't handle this case that well.  It's not hard to do something better for this case.  I filed 6986483 for this.
>> 
>> tom
>>   
> 
> Thanks Tom,
> I know since a long time that the verifier doesn't check interface but
> I have discovered recently the runtime implication.

Yes it's a bummer.  We've discussed improving C2s type system to handle interface types better since in some cases interfaces type can be trusted.  Adding some static verification of interfaces would also allow us to trust them more which could help.

> 
> I wonder if the fix can improve performance of benchmarks that use java.util collection,
> because this API uses a similar class hierarchy scheme.

There are normally more than 1 or 2 implementors of the collection interfaces so static information isn't as useful.  Much of the time the existing type profiling works great because we're only seeing a single type but of course there are cases where it falls down.  Analyzing the hierarchy of the types that show up in the type profile might allow us to do a slightly better job but I suspect we'd have to modify our profile collection to record more types or to record methods instead of classes if we want to handle more complex call sites.

tom

> 
> Rémi
> 
>> On Sep 18, 2010, at 11:25 AM, Rémi Forax wrote:
>> 
>>   
>>> I take a little time to create a simple test case to reproduce a bug
>>> found by Charles Nutter with c2.
>>> see http://groups.google.com/group/jvm-languages/browse_thread/thread/6c9e05ecd28fdcd4#
>>> 
>>> Here is the test case,
>>> There is 3 classes A, B, C that inherit from AbstractFoo that implements Foo.
>>> The method test do a virtual call to check() and because
>>> check() is implemented in AbstractFoo we expect that this call should be
>>> de-virtualized then inlined.
>>> 
>>> c2 fails, foo.check() is compiled as a virtual call :(
>>> With c1, there is no problem, CHA works correctly.
>>> 
>>> Rémi
>>> 
>>> ------------------------------------------------------------------------------------------------
>>> 
>>> public class InlineTest {
>>>  interface Foo {
>>>    public boolean check(int generation);
>>>  }
>>> 
>>>  static class AbstractFoo implements Foo {
>>>    private final int value;
>>> 
>>>    protected AbstractFoo(int value) {
>>>      this.value = value;
>>>    }
>>> 
>>>    public boolean check(int generation) {
>>>      return this.getClass().hashCode() - value == generation;
>>>    }
>>>  }
>>> 
>>>  static class A extends AbstractFoo {
>>>    public A(int value) {
>>>      super(value);
>>>    }
>>>  }
>>>  static class B extends AbstractFoo {
>>>    public B(int value) {
>>>      super(value);
>>>    }
>>>  }
>>>  static class C extends AbstractFoo {
>>>    public C(int value) {
>>>      super(value);
>>>    }
>>>  }
>>> 
>>>  private static final int CONST = A.class.hashCode();
>>> 
>>>  private static int count;
>>> 
>>>  private static void test(Foo foo) {
>>>    if (foo.check(0)) {
>>>        count += 2;
>>>        //System.out.println("foo");
>>>    } else {
>>>        count += 1;
>>>        //System.out.println("bar");
>>>    }
>>>  }
>>> 
>>>  public static void main(String[] args) {
>>>    Foo[] array = new Foo[100000];
>>>    int threshold = 20000;
>>>    for(int i=0; i<threshold; i++) {
>>>      array[i] = new A(CONST);
>>>    }
>>> 
>>>    for(int i=threshold; i<array.length; i++) {
>>>      array[i] = (i%2 == 0)? new B(0): new C(CONST);
>>>    }
>>> 
>>>    for(int i=0; i<array.length; i++) {
>>>      test(array[i]);
>>>    }
>>> 
>>>    System.out.println(count);
>>>  }
>>> }
>>>     
>>   
>