On Nov 25, 2010, at 2:27 AM, Christian Thalinger wrote: > On Nov 25, 2010, at 8:42 AM, John Rose wrote: >> On Nov 22, 2010, at 6:57 PM, John Rose wrote: >> >>> Because of a strong and reasonable request from the IBM JVM team, the JSR >>> 292 class file format for CONSTANT_InvokeDynamic is changing one more time. >>> ... >>> I will be putting out a review request soon for this. >> >> >> Here is a JVM change which moves the variadic parts of >> CONSTANT_InvokeDynamic specifiers out of the constant pool and into a new >> attribute, BootstrapMethods. >> http://cr.openjdk.java.net/~jrose/7001379/webrev.hs.00 > > src/share/vm/classfile/classFileParser.hpp: > > + //@@ static int start_operand_group(GrowableArray<int>* &operands, int > op_count, TRAPS); > + //@@ static void store_operand_array(GrowableArray<int>* operands, > constantPoolHandle cp, TRAPS); > > Do we still need them?
Oops, nope. > src/share/vm/oops/constantPoolOop.hpp: > > - void copy_entry_to(int from_i, constantPoolHandle to_cp, int to_i, TRAPS); > + static void copy_entry_to(constantPoolHandle from_cp, int from_i, > constantPoolHandle to_cp, int to_i, TRAPS); > > Maybe I'm missing the obvious here but why do need to pass a handle in? > There is nothing fancy going on in that method. In principle, the call to 'klass_at' could trigger a GC. We know it won't, because of the tag of the CP entry. But that requires a private, invisible contract with the present implementation of 'klass_at'. It is safer to use the handle. Since I had to introduce a handle in copy_cp_to_impl, it was natural to move it down into the per-entry routine. >> For review, here is the specification change, in the javadoc in the jdk repo: >> http://cr.openjdk.java.net/~jrose/7001379/webrev.jdk.00 > > 301 * Each {...@code CONSTANT_InvokeDynamic} entry refers has a reference to > a > 302 * bootstrap method specifier defined in a separate array. > > I think there are some superfluous words. Thanks. Fixed and clarified: * Each {...@code CONSTANT_InvokeDynamic} entry contains an index which references * a bootstrap method specifier; all such specifiers are contained in a separate array. * This array is defined by a class attribute named {...@code BootstrapMethods}. > 317 * {...@code BootstrapMethods} attribute. In particular, each cosntant > > Typo. Fixed. >> The JDK webrev also includes an adjusted unit test. >> >> The base revision was reviewed by Christian yesterday: >> http://cr.openjdk.java.net/~jrose/7001423/webrev.00 >> >> The JVM code passes the unit test, of course. > > > -- Christian Thanks. Can I cite you as a reviewer on both change sets? -- John _______________________________________________ mlvm-dev mailing list mlvm-dev@openjdk.java.net http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
