Following test case is failing from JDK7 build 20

<testcase>
import java.util.*;

public class VectorIntegerTest {
    static Random rnd = new Random();
    public static void main(String[] args) throws Exception {
		List<Integer> list1 = new Vector<Integer>();
		AddRandoms(list1, 400);
		List<Integer> list2 = new Vector<Integer>();
		AddRandoms(list2, 400);

		List<Integer> copyofs2 = new Vector<Integer>();
		copyofs2.addAll(list2);

		if (!list2.equals(copyofs2))
			throw new Exception("Exception");

		list1.clear();
		list1.addAll(0,list2);

		if (!(list1.equals(list2) && list2.equals(list1)))
			throw new Exception ("Exception");

		if (!(list1.equals(list2) && list2.equals(list1)))
			throw new Exception("Exception");

		List<Integer> l = new Vector<Integer>();
		AddRandoms(l,400);
		Integer [] ia = l.toArray(new Integer[0]);
		if (!l.equals(Arrays.asList(ia)))
			 throw new Exception("Exception");
    }

    static void AddRandoms(List<Integer> s, int n) throws Exception {
		for (int i=0; i<n; i++) {
			int r = rnd.nextInt() % n;
			Integer e = new Integer(r < 0 ? -r : r);
			s.add(e);
		}
  	}
}
</testcase>

<output>
bash-3.00$ /net/sqindia/export/disk09/jdk/7/b17/binaries/solsparc/bin/java VectorIntegerTest
bash-3.00$ /net/sqindia/export/disk09/jdk/7/b18/binaries/solsparc/bin/java VectorIntegerTest
bash-3.00$ /net/sqindia/export/disk09/jdk/7/b19/binaries/solsparc/bin/java VectorIntegerTest

**** failing from build 20 ******

bash-3.00$ /net/sqindia/export/disk09/jdk/7/b20/binaries/solsparc/bin/java VectorIntegerTest
Exception in thread "main" java.lang.IncompatibleClassChangeError
        at java.util.AbstractList.equals(AbstractList.java:522)
        at java.util.Vector.equals(Vector.java:953)
        at VectorIntegerTest.VectorIntegerTestTest01(VectorIntegerTest.java:63)
        at VectorIntegerTest.main(VectorIntegerTest.java:9)
bash-3.00$ /net/sqindia/export/disk09/jdk/7/b21/binaries/solsparc/bin/java VectorIntegerTest
Exception in thread "main" java.lang.IncompatibleClassChangeError
        at java.util.AbstractList.equals(AbstractList.java:522)
        at java.util.Vector.equals(Vector.java:953)
        at VectorIntegerTest.VectorIntegerTestTest01(VectorIntegerTest.java:63)
        at VectorIntegerTest.main(VectorIntegerTest.java:9)
</output>
Posted Date : 2007-10-04 11:06:40.0

Additional comments and test case from Christian Wimmer (  xxxxx@xxxxx  ) from the C1 collaborative research project (also on the bugs.sun.com public database):

I get a similar bug: the "eclipse" benchmark of the DaCapo benchmark suite fails with the latest JDK 7 builds (starting with build b20 which changed the handling of dependencies). In debug builds, a message is printed that a method should have been marked for deoptimization, but was not marked by the optimized dependency checking code.

I could reduce the problem to the small testcase that is attached. The Interface has two implementations: the classes Impl1 and Impl2. At first, only Impl1 is loaded, so the method of the interface call can be inlined because there is only one implementation. When the second implementation is loaded, the method must be deoptimized.

The second class Impl2 implements the Interface, but the method is already defined in the base class BaseImpl2. The optimized dependency checking code only looks at the methods defined in Impl2, does not see the method, and therefore does not trigger deoptimization. The slow verification code of the debug build detects the inconsistency and prints an error (I would prefer an assertion in such cases).



A possible fix would be to also look at methods defined in superclasses. When the line 753 in the file dependencies.cpp is changed from

  methodOop m = instanceKlass::cast(k)->find_method(_name, _signature);
   
to

  methodOop m = instanceKlass::cast(k)->uncached_lookup_method(_name, _signature);
  
the dependency checking is correct in my example. However, I do not know if this change causes other problems (it could detect too many conflicts) or is too slow.

[Follow-up from Christian: My nightly benchmarks revealed that my possible fix of the bug is too conservative. It literally kills the performance of some benchmarks, e.g. _227_mtrt is more than 50% slower because many accessor methods are no longer inlined. So just forget my suggestion...]

public class DependencyBug {

  public static interface Interface {
    public void method();
  }
  
  public static class Impl1 implements Interface {
    public void method() {
      // Nothing to do here
    }
  }
  
  public static class BaseImpl2 {
    public void method() {
      System.out.print("#");
    }
  }

  public static class Impl2 extends BaseImpl2 implements Interface {
    // Interface method already implemented in base class.
  }

  
 
  public static void callMethod(Interface obj) {
    // method() is inlined as long as only class Impl1() is loaded.
    obj.method();
  }
  
  public static void main(String[] args) throws Exception {
    
    Interface obj = new Impl1();
    for (int i = 0; i < 2000; i++) {
      // Force compilation of the method callMethod()
      callMethod(obj);
    }
    
    System.out.println("**** Initiating class loading of Impl2 ****");
    
    obj = new Impl2();
    callMethod(obj);
  }
}
Posted Date : 2007-11-16 18:52:59.0

JDK: 7, 6u10 b09
testbase: /net/cady/export/dtf/unified/knight-ws/suites/6.0_cady/libs
Failing testcases:
java_util/generics/list/LinkedListDoubleTest
java_util/generics/list/LinkedListIntegerTest
Posted Date : 2007-12-20 21:48:36.0

failing cases: 
java_util/generics/list/VectorIntegerTest
java_util/generics/list/VectorDoubleTest
java_util/generics/list/ArrayListDoubleTest
java_util/generics/list/ArrayListIntegerTest
java_util/generics/list/StackDoubleTest
java_util/generics/list/StackIntegerTest
Posted Date : 2008-02-25 17:59:24.0

N/A

This is deeply mysterious.  On one machine the supplied test works perfectly,
while on another, using the *exact*same* binaries, it fails:


(  xxxxx@xxxxx  ) ~/src/toy/6610906 $ time jver -v /net/sqindia.india/export/disk09/jdk/7/b20/binaries/solsparc jr VectorIntegerTest
Using JDK /net/sqindia.india/export/disk09/jdk/7/b20/binaries/solsparc
==> javac -Xlint:all VectorIntegerTest.java
==> java -esa -ea VectorIntegerTest


(  xxxxx@xxxxx  ) ~/src/toy/6610906 $ jver -v /net/sqindia.india/export/disk09/jdk/7/b20/binaries/solsparc java VectorIntegerTest
Using JDK /net/sqindia.india/export/disk09/jdk/7/b20/binaries/solsparc
Exception in thread "main" java.lang.IncompatibleClassChangeError
        at java.util.AbstractList.equals(AbstractList.java:522)
        at java.util.Vector.equals(Vector.java:953)
        at VectorIntegerTest.VectorIntegerTestTest01(VectorIntegerTest.java:62)
        at VectorIntegerTest.main(VectorIntegerTest.java:8)


Seetharam, could you reduce the test case to the minimum required to reproduce this?
It's starting to look like a hotspot or ClassLoader bug.
Posted Date : 2007-10-03 20:08:50.0

The provided test case fails on solaris-sparc and on windows-x86, 
but only with the client compiler.
This looks very much like a hotspot compiler bug, so I am redispatching
to hotspot/compiler1 (although investigation to find the b20 change
that caused this should continue in any case)
Posted Date : 2007-10-05 05:25:07.0

It appears the changes for 6471009 cause us to miss a dependence.

% /java/re/jdk/1.7.0/promoted/all/b20/binaries/solaris-sparc/fastdebug/bin/java -client -Xbatch VectorIntegerTest                 Should have been marked for deoptimization:
  dependee = java.util.AbstractList$ListItr
  context supers = 2, interfaces = 2
Compiled (c1)  13   nmethod java.util.AbstractList::equals (117 bytes)
 total in heap  [0xfb10a608,0xfb10b28c] = 3204
 relocation     [0xfb10a6bc,0xfb10a798] = 220
 main code      [0xfb10a7a0,0xfb10ace0] = 1344
 stub code      [0xfb10ace0,0xfb10ad98] = 184
 scopes data    [0xfb10ad98,0xfb10af60] = 456
 scopes pcs     [0xfb10af60,0xfb10b218] = 696
 dependencies   [0xfb10b218,0xfb10b220] = 8
 nul chk table  [0xfb10b220,0xfb10b254] = 52
 oops           [0xfb10b254,0xfb10b28c] = 56
Dependencies:
Dependency of type unique_concrete_method
  context = *java.util.ListIterator
  method  = {method} 'hasNext' '()Z' in 'java/util/Vector$Itr'
   [nmethod<=klass]java.util.ListIteratorDependency of type unique_concrete_method
  context = *java.util.ListIterator
  method  = {method} 'next' '()Ljava/lang/Object;' in 'java/util/Vector$Itr'
   [nmethod<=klass]java.util.ListIteratorException in thread "main" java.lang.IncompatibleClassChangeError
        at java.util.AbstractList.equals(AbstractList.java:522)
        at java.util.Vector.equals(Vector.java:953)
        at VectorIntegerTest.main(VectorIntegerTest.java:29)
Posted Date : 2007-10-05 15:05:48.0

The test case is described in the comments in dependencies.cpp, around line 961:
    // Avoid this case: *I.m > { A.m, C }; B.m > C
    // Here, I.m has 2 concrete implementations, but m appears unique
    // as A.m, because the search misses B.m when checking C.
    // The inherited method B.m was getting missed by the walker
    // when interface 'I' was the starting point.
    // %%% Until this is fixed more systematically, bail out.
    // (Old CHA had the same limitation.)
    return context_type;

The question is, if the dependency module (apparently) knows about this case, why isn't it getting asked about it?

I think the code in c1_GraphBuilder.cpp (near 1614) needs an extra dependency to detect when nof_implementors changes.

      ciInstanceKlass* singleton = NULL;
      if (target->holder()->nof_implementors() == 1) {
        singleton = target->holder()->implementor(0);
      }
      if (singleton) {
        cha_monomorphic_target = target->find_monomorphic_target(calling_klass, target->holder(), singleton);
        if (cha_monomorphic_target != NULL) {
          // If CHA is able to bind this invoke then update the class
          // to match that class, otherwise klass will refer to the
          // interface.
          klass = cha_monomorphic_target->holder();
    +     dependency_recorder()->assert_unique_concrete_subtype(actual_recv, singleton);
          actual_recv = target->holder();

Since the C1 code updates actual_recv to a unique implementor, it is C1's responsibility to issue a dependency on that uniqueness.  The recent changes in b20 made the dependency checking less sloppy.  As a side effect, this missing dependency is now exposed.
Posted Date : 2007-11-21 03:36:45.0

My previous evaluation isn't quite right.  The original dependency issued by C1 is correct, and used to work, and was broken by the change that created "spot checks" for dependencies.

The bail-out on nof_impls > 1 in find_witness_anywhere needs to be duplicated in the optimized query find_witness at.  See suggested fix.
Posted Date : 2007-11-27 23:56:22.0

I get a similar bug: the "eclipse" benchmark of the DaCapo benchmark suite fails with the latest JDK 7 builds (starting with build b20 which changed the handling of dependencies). In debug builds, a message is printed that a method should have been marked for deoptimization, but was not marked by the optimized dependency checking code.

I could reduce the problem to the small testcase that is attached. The Interface has two implementations: the classes Impl1 and Impl2. At first, only Impl1 is loaded, so the method of the interface call can be inlined because there is only one implementation. When the second implementation is loaded, the method must be deoptimized.

The second class Impl2 implements the Interface, but the method is already defined in the base class BaseImpl2. The optimized dependency checking code only looks at the methods defined in Impl2, does not see the method, and therefore does not trigger deoptimization. The slow verification code of the debug build detects the inconsistency and prints an error (I would prefer an assertion in such cases).



A possible fix would be to also look at methods defined in superclasses. When the line 753 in the file dependencies.cpp is changed from

  methodOop m = instanceKlass::cast(k)->find_method(_name, _signature);
   
to

  methodOop m = instanceKlass::cast(k)->uncached_lookup_method(_name, _signature);
  
the dependency checking is correct in my example. However, I do not know if this change causes other problems (it could detect too many conflicts) or is too slow.

public class DependencyBug {

  public static interface Interface {
    public void method();
  }
  
  public static class Impl1 implements Interface {
    public void method() {
      // Nothing to do here
    }
  }
  
  public static class BaseImpl2 {
    public void method() {
      System.out.print("#");
    }
  }

  public static class Impl2 extends BaseImpl2 implements Interface {
    // Interface method already implemented in base class.
  }

  
 
  public static void callMethod(Interface obj) {
    // method() is inlined as long as only class Impl1() is loaded.
    obj.method();
  }
  
  public static void main(String[] args) throws Exception {
    
    Interface obj = new Impl1();
    for (int i = 0; i < 2000; i++) {
      // Force compilation of the method callMethod()
      callMethod(obj);
    }
    
    System.out.println("**** Initiating class loading of Impl2 ****");
    
    obj = new Impl2();
    callMethod(obj);
  }
}

6312651

My nightly benchmarks revealed that my possible fix of the bug is too conservative. It literally kills the performance of some benchmarks, e.g. _227_mtrt is more than 50% slower because many accessor methods are no longer inlined. So just forget my suggestion...