Looks good. On Oct 2, 2013, at 12:33 PM, John Rose <john.r.r...@oracle.com> wrote:
> Push-button webrev generator to the rescue: > http://cr.openjdk.java.net/~jrose/8024599/webrev.01 > > — John > > On Oct 2, 2013, at 11:23 AM, Christian Thalinger > <christian.thalin...@oracle.com> wrote: > >> Since there is no new webrev I assume you incorporated all the stuff below. >> If that's the case then it looks good. >> >> On Sep 20, 2013, at 6:18 PM, John Rose <john.r.r...@oracle.com> wrote: >> >>> On Sep 20, 2013, at 8:29 AM, Vladimir Ivanov <vladimir.x.iva...@oracle.com> >>> wrote: >>> >>>> John, >>>> >>>> I don't see much value in documenting buggy behavior of early JDK7 in JDK8 >>>> code. So, I would remove it. >>> >>> OK. I think I had it in mainly to make sure the unit tests did something >>> interesting. >> >>> >>>> Regarding the test: >>>> 31 * @run main/othervm/timeout=3600 >>>> - why do you have timeout set to 1h? >>> >>> Copy-and-paste from some other test. Removed. >>> >>>> I like the idea how you count events. >>>> >>>> As a suggestion for enhancement - maybe it's more reliable to check the >>>> "type" of event as well? To ensure that particular class was initialized. >>> >>> Good idea. But since each unique init event is stored in a separate >>> variable, it's easy to check this without explicit event types. I did the >>> following, for each of the six test cases: >>> >>> @@ -150,9 +150,11 @@ >>> } >>> >>> private static int runFoo() throws Throwable { >>> + assertEquals(Init1Tick, 0); // Init1 not initialized yet >>> int t1 = tick("runFoo"); >>> int t2 = (int) INDY_foo().invokeExact(); >>> int t3 = tick("runFoo done"); >>> + assertEquals(Init1Tick, t2); // when Init1 was initialized >>> assertEquals(t1+2, t3); // exactly two ticks in between >>> assertEquals(t1+1, t2); // init happened inside >>> return t2; >> >>> >>> >>> — John >>> >>>> Best regards, >>>> Vladimir Ivanov >>>> >>>> On 9/20/13 1:38 AM, John Rose wrote: >>>>> On Sep 12, 2013, at 7:24 PM, John Rose <john.r.r...@oracle.com >>>>> <mailto:john.r.r...@oracle.com>> wrote: >>>>> >>>>>> Please review this change for a change to the JSR 292 implementation: >>>>>> >>>>>> http://cr.openjdk.java.net/~jrose/8024599/webrev.00/ >>>>>> >>>>>> Summary: Align MH semantic with bytecode behavior of constructor and >>>>>> static member accesses, regarding <clinit> invocation. >>>>>> >>>>>> The change is to javadoc and unit tests, documenting and testing some >>>>>> corner cases of JSR 292 APIs. >>>>> >>>>> I have a reviewer (Alex Buckley) for the documentation changes, but I >>>>> would also like a quick code review for the unit test. >>>>> >>>>> Also, there is a code insertion (predicated on a "false" symbolic >>>>> constant) which serves to document the buggy JDK 7 behavior. I'm not >>>>> particularly attached to it, so I'm open to either a yea or nay on >>>>> keeping it. Leaning nay at the moment. >>>>> >>>>> — John >>>>> >>>>> >>>>> _______________________________________________ >>>>> mlvm-dev mailing list >>>>> mlvm-...@openjdk.java.net >>>>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev >>>>> >>> >>> _______________________________________________ >>> mlvm-dev mailing list >>> mlvm-...@openjdk.java.net >>> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev >> >> _______________________________________________ >> mlvm-dev mailing list >> mlvm-...@openjdk.java.net >> http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev > > _______________________________________________ > mlvm-dev mailing list > mlvm-...@openjdk.java.net > http://mail.openjdk.java.net/mailman/listinfo/mlvm-dev
