From: Erik B. <eri...@gm...> - 2011-12-10 17:03:35
|
Hi, > > I think RVM-137 is worried about the following issue. Suppose I have > > class A { ... } > class B extends A { ... } > class C extends B { ... } > class D extends C { ... } > > Some RegisterOperand t100 has a type of C, but the preciseType bit > isn't set (so it could be a C or a D). The RegisterOperand API > allows one to make the call t100.refine(B). This would result in > worse static type information, potentially degrading downstream > optimizations, because "refining" the type of t100 from C to B > actually loses information. My interpretation is that RVM-137 was > asking for us to add checking to the implementation of the refine > method to make sure that this wouldn't happen. > > Initially when I looked at the code I had convinced myself that the > surrounding logic prevents this from happening (unless classes weren't > loaded yet, which is honestly not an interesting case considering we > mostly care about the adaptive system). I still think this is true > in the case of the type being refined to by the checkcast is a class, > since there is no multiple inheritance in Java. > You may be right about classes being unproblematic. I added a simple assertion to refine() to check if there were problems with incorrect refining and tried executing some benchmarks (DaCapo 2006, Dacapo Bach, scalaBench). These did not reveal any problems except for a special case in DaCapo 2006 lusearch. There is a "refinement" of TypeReference.NULL_TYPE to a class in the method "public final Hits search(Query query)" in "org.apache.lucene.Searcher". The "refinement" seems to happen because of an unnecessary checkcast of null. > However, I do think the current code in checkcast could be improved > when the thing being cast to is an interface. Consider > > interface I {} > > class A { ... } > class B extends A { ... } > class C extends B { ... } > class D extends C, implements I { ... } > > class E implement I > > Again t100 has a non-precise type of C. This time we are casting it > to an I. We can't prove/disprove that cast statically, so we will end > up setting the type of t100 to I (instead of D) downstream of the > checkcast. What we really need to do in this case is set the type to > I intersect {C,D} which yields D. By setting it to I, we admit the > possibility of t100 being an E, which is not true. > that makes sense. Thanks for the explanation. Kind regards, Erik Brangs |