Tobi,

Thank you for the thoughtful proposal. 
Couple of questions/comments on the proposal

> On Jul 6, 2018, at 11:38 AM, Tobi Ajila <tobi_aj...@ca.ibm.com> wrote:
> 
> I would like to propose an alternative strategy, one that would effectively 
> provide the same consistency checks outlined in 
> http://cr.openjdk.java.net/~acorn/value-types-consistency-checking-details.pdf
>  
> <http://cr.openjdk.java.net/~acorn/value-types-consistency-checking-details.pdf>
>  without requiring eager loading of value types (VT) in the return/args of 
> methods during the preparation phase. This is born out of our experience with 
> PackedObjects which took a very similar approach to the ValueTypes attribute. 
>   
> 
> This proposal makes use of the existing ValueTypes attribute. In addition, it 
> requires that each classloader keeps a registry of ‘expected’ VTs.  
> 
> Proposal
> --------------------------
> The following steps occur in the loading phase:
> 
> 1. Prior to loading the current class each type in the ValueTypes attribute 
> is checked see if it is loaded. If it is, the type must be a VT or ICCE is 
> thrown. Otherwise, the type is registered with the initiating classloaders 
> expected VT registry (EVTR).
> 
I have not walked through the implications of using an initiating loader to 
record EVTR. What confuses me here is that when defining a class, we look at 
the defining loader, so the class could successfully be defined, but there 
would be an ICCE when returning the type to the initiating loader so the 
loading/resolution would fail?
> 
> 2. Pre-loading of ACC_FLATTENABLE instance fields follow the same rules as 
> the ones specified in Karen's proposal.
> 
> 3. Prior to loading the current class, if the current class is a VT it must 
> be in the classloader's EVTR or ICCE is thrown. If the current class is not a 
> VT and does appear in the EVTR, ICCE is thrown.
> 
What happens if there is a call to define a class which is a VT but it has not 
yet been referenced by another class and is therefore not in any class loader’s 
EVTR? (we get a defineClass call with a byte[]). It won’t occur in an EVTR but 
we don’t want an ICCE at this point, right?

Concern: Who pays the cost of a mistake?
If we check during preparation of a local method, then the ICCE is thrown to 
the class declaring the method with the missing VT attribute.
If we check lazily, then the ICCE is given to the class that we expected to be 
a VT or not which doesn’t match some class’ expectations.
I grant that if the expected class has already been loaded, the ICCE will go to 
the referring class.
Is it the case then, that by adding the wrong class e.g. Foo to my ValueTypes 
attribute, or by accidentally leaving out Point from my ValueTypes attribute - 
then neither Foo nor Point will be able to load without getting an ICCE, and no 
one else can use them?


> 
> In link phase prior to preparation:
> - Same as Karen's static field rules, "static fields declared with the 
> ACC_FLATTENABLE flag set have their type pre-loaded..."
> 
> In preparation phase:
> - Same as Karen's method overriding rules, "any method overriding needs to 
> perform value type consistency checking between declarer of the overridden 
> method ..."
> 
> At resolution time:
> - Same as Karen’s CONSTANT_Class, field and method resolution rules
> ---------------------------
> 
> The benefit of this approach is that as soon as anyone believes a class is a 
> VT, it must be loaded as a VT or it fails. As a result, there is no 
> inconsistency of loaded VTs. This proposal doesn't guard against cases where 
> a class was not expecting a method return/arg type to be a VT but then later 
> on turned out to be a VT when it was resolved. However, I think don’t think 
> Karen’s proposal offered these guarantees either. 
> 
// Not sure if my assumptions work - my proposal is that for the case you 
describe: a class was not expecting a method return/arg to be a VT but then 
later on turned out to be a VT when it was resolved:
*5 in my proposal
    - method invocation can work with NULL
    - but the method invocation can not work with an actual value type - that 
gets caught when the method tries to get its hands on the actual value type
    - I walked through my logic - that doesn’t count as a guarantee - love your 
double-checking
> 
> > Some aspects of the implementation complexity that we have identified so 
> > far:
> > a) At method resolution time, if there is a mismatch in value type 
> > consistency 
> > between the declaring class and the actual type in the signature, then 
> > there 
> > is also a mismatch in all classes that have overridden the current method. 
> > This is particularly painful with default methods in interfaces.
> 
> With this proposal the only possible inconsistency here is: 
> Method has a return/arg type that is believed to not be a VT but turns out to 
> be a VT. In this case any compiled code is doing pass by reference calling 
> convention which works for both VT and non-VT types. 
> 
// And the generated method does not know it is dealing with a VT, so no other 
VT optimizations either.
> 
> > b) We need to ensure that we catch all method resolution, through all of 
> > the 
> > alternate accessor paths, including MethodHandles, VarHandles, Reflection, 
> > JNI,
> > so that we catch both the specification and implementation changes.
> 
> All these cases are covered with the class loading consistency checks (EVTR).
> 
Still sanity checking that at the implementation level - independent of which 
of the proposal variations
we use.
> 
> > c) Our favorite, invokespecial ACC_SUPER, needs special handling, since it 
> > performs selection which is not based on overriding, but based on virtually 
> > re-resolving.
> 
> same as above
> 
> > e) If we modify the specification to allow eager loading, and save errors 
> > to throw at method resolution, we need to work through the JVMS question 
> > of which errors would be saved (e.g. OOME, SOE might be thrown as part of 
> > the implementation vs. saving LinkageError), as well as designing a new 
> > implementation mechanism to repeat exceptions relative to signatures used 
> > for method resolution.
> 
> This wouldn’t be required in this proposal
> 
Thanks!
> 
> > d) Pass by value calling convention optimizations depend on loading the 
> > actual type. Loading of the parameter types on first method resolution 
> > implies that if the caller is compiled, the caller method requires 
> > deoptimization/recompilation to pass arguments by value for future calls, 
> > which is a performance cost.
> 
> Typically, a method is run a few times before it is compiled (perhaps I’m 
> making implementation assumptions?). At this stage, the return/arg types are 
> either loaded or they are always null. This seems to suggest that 
> deoptimization/recompilation scenario would not be a common occurrence. 
> 
I believe that is an implementation assumption, which I initially shared.
At least with -Xcomp, this is not the case.

thanks,
Karen
> 
> 
> --Tobi
> 
> > From: Karen Kinnear <karen.kinn...@oracle.com>
> > To: valhalla-spec-experts <valhalla-spec-experts@openjdk.java.net>
> > Date: 2018/06/26 10:32 AM
> > Subject: EG help please with getting to LW1 re: Value Types 
> > Consistency Checking
> > Sent by: "valhalla-spec-experts" <valhalla-spec-experts-
> > boun...@openjdk.java.net>
> > 
> > Summary: Could we please allow eager loading for value types in the 
> > locally declared method signatures
> > prior to preparation  for LW1? 
> > 
> > Without that we seriously risk being able to offer an LW1 early 
> > access binary for the JVMLS.
> > We believe it is more important to get this into people’s hands for 
> > experimentation and feedback than
> > to delay the eager loading at this time.
> > 
> > Details:
> > At our EG discussion on June 20, 2018, we discussed the proposal for
> > Value Types Consistency checking
> > at http://cr.openjdk.java.net/~acorn/value-types-consistency- 
> > <http://cr.openjdk.java.net/~acorn/value-types-consistency->
> > checking-details.pdf
> > 
> > Part of the proposal for checking value type consistency relative to
> > the actual type
> > was for locally declared methods. The proposal was to check the 
> > value types in arguments/return type
> > before preparation of the declaring class.
> > 
> > During the meeting, there was a request to explore whether we could either:
> > 1)  delay checking the value type consistency until an attempt to 
> > resolve the method for invocation, or 
> > 2) write the JVMS is such as way as to allow eager loading, but only
> > throw an error related to the eager loading at method resolution.
> > 
> > My understanding is that the goals of this are two-fold:
> > 1) if the method is never called, the rest of the code will continue to run
> > 2) reduce eager loading
> > 
> > We have started this investigation, and there is non-trivial 
> > complexity in implementing either of these approaches,
> > and we would like more time to explore how to make this possible, 
> > after making LW1 available.
> > 
> > Some aspects of the implementation complexity that we have identified so 
> > far:
> > a) At method resolution time, if there is a mismatch in value type 
> > consistency between the declaring class and the actual
> > type in the signature, then there is also a mismatch in all classes 
> > that have overridden the current method. This is particularly
> > painful with default methods in interfaces.
> > b) We need to ensure that we catch all method resolution, through 
> > all of the alternate accessor paths, including MethodHandles, VarHandles,
> > Reflection, JNI, so that we catch both the specification and 
> > implementation changes.
> > c) Our favorite, invokespecial ACC_SUPER, needs special handling, 
> > since it performs selection which is not based on overriding, but 
> > based on virtually re-resolving.
> > d) Pass by value calling convention optimizations depend on loading 
> > the actual type. Loading of the parameter types on first method 
> > resolution implies that if the caller is compiled, the caller method
> > requires deoptimization/recompilation to pass arguments by value for
> > future calls, which is a performance cost.
> > e) If we modify the specification to allow eager loading, and save 
> > errors to throw at method resolution, we need to work through the 
> > JVMS question of which errors would be saved (e.g. OOME, SOE might 
> > be thrown as part of the implementation vs. saving LinkageError), as
> > well as designing a new implementation mechanism to repeat 
> > exceptions relative to signatures used for method resolution.
> > 
> > thanks,
> > Karen

Reply via email to