September 2014 - core-libs-dev

Re: [9] Review request : JDK-8058728: TEST_BUG: Make java/lang/invoke/LFCaching/LFGarbageCollectedTest.java skip arrayElementSetter and arrayElementGetter methods
by igor.ignatyev＠oracle.com 19 Sep '14

19 Sep '14

thx. Reviewed. -- II ----- Reply message ----- From: "Konstantin Shefov" <konstantin.shefov(a)oracle.com> To: "Igor Ignatyev" <igor.ignatyev(a)oracle.com> Cc: "VLADIMIR.X.IVANOV" <vladimir.x.ivanov(a)oracle.com>, <core-libs-dev(a)openjdk.java.net>, <mlvm-dev(a)openjdk.java.net> Subject: [9] Review request : JDK-8058728: TEST_BUG: Make java/lang/invoke/LFCaching/LFGarbageCollectedTest.java skip arrayElementSetter and arrayElementGetter methods Date: Fri, Sep 19, 2014 16:04 Yes, the test compiles and runs ok, but fails because of 8057020. If we remove -Djava.lang.invoke.MethodHandle.USE_LF_EDITOR=true, the test passes. -Konstantin On 19.09.2014 16:01, Igor Ignatyev wrote: > Hi, > > looks good to me. > > how have you tested your changes? > > Thanks, > Igor > > On 09/19/2014 01:50 PM, Konstantin Shefov wrote: >> Hello, >> >> Please review the test bug fix >> https://bugs.openjdk.java.net/browse/JDK-8058728 >> Webrev is http://cr.openjdk.java.net/~kshefov/8058728/webrev.00 >> >> Thanks >> >> -Konstantin

1 0

[9] Review request : JDK-8058728: TEST_BUG: Make java/lang/invoke/LFCaching/LFGarbageCollectedTest.java skip arrayElementSetter and arrayElementGetter methods
by Konstantin Shefov 19 Sep '14

19 Sep '14

Hello, Please review the test bug fix https://bugs.openjdk.java.net/browse/JDK-8058728 Webrev is http://cr.openjdk.java.net/~kshefov/8058728/webrev.00 Thanks -Konstantin

4 4

RFR: 8058550: Clarify that TimerTasks are not reusable
by Martin Buchholz 19 Sep '14

19 Sep '14

Hi Chris, I'd like you to do a code review. http://cr.openjdk.java.net/~martin/webrevs/openjdk9/TimerTask-clarification/ https://bugs.openjdk.java.net/browse/JDK-8058550

5 10

RFR: 8055232 (ref) Exceptions while processing Reference pending list
by Peter Levart 19 Sep '14

19 Sep '14

Hi, This story has a long tail. It started with: https://bugs.openjdk.java.net/browse/JDK-7038914 Some stress tests triggered OOME in ReferenceHandler thread which would die. The first attempt at fixing this was the following discussion: https://www.mail-archive.com/core-libs-dev%40openjdk.java.net/msg16250.html Which resulted in patch: http://hg.openjdk.java.net/jdk8/jdk8/jdk/rev/0b8dab7fec54 This assumed that ReferenceHandler thread doing Object.wait() could be interrupted (by stress test - normal application don't do that) and failed to allocate InterruptedException object. A jtreg test was designed which triggered that situation and a fix would catch OOME and ignore it. But the stress tests (the same or some other, I don't know) apparently were not entirely happy with this fix. The following discussion describes this: https://www.mail-archive.com/core-libs-dev%40openjdk.java.net/msg23596.html The other "unprotected" point at which OOME could be thrown and was later confirmed by debugging is (r instanceof Cleaner) test. The assumption was that it could trigger Cleaner class loading at 1st execution which would cause OOME to be thrown. The fix that finally silenced stress tests was the following: http://hg.openjdk.java.net/jdk9/dev/jdk/rev/d04102f69d46 This part of code (the j.l.r.Reference class and its members) has undergone further changes afterwards for fixing another bug: https://bugs.openjdk.java.net/browse/JDK-6857566 With following patch: http://hg.openjdk.java.net/jdk9/jdk9/jdk/rev/9934d34ed3c0 But this did not change the code paths - just factored-out the content of the loop into a separate method that could be used from outside. All well until kim.barrett(a)oracle.com noticed that the 2nd fix introduced a potentially illegal situation. There is a j.l.r.Reference.Lock inner class and a singleton object assigned to static field in j.l.Reference class with the following notice: /* Object used to synchronize with the garbage collector. The collector * must acquire this lock at the beginning of each collection cycle. It is * therefore critical that any code holding this lock complete as quickly * as possible, allocate no new objects, and avoid calling user code. */ static private class Lock { } private static Lock lock = new Lock(); The conflicting part is "allocate no new objects". Catching OOME inside a synchronized block holding this lock implies that new objects could be allocated. I have a feeling that the 2nd fix prevented that by pre-loading the Cleaner class at Reference class initialization time. But because it was hard to reproduce the situation where OOME was thrown from (r instanceof Cleaner) check, we nevertheless handled this hypothetical situation. Perhaps it would be better that we didn't and just see if OOME returned after just adding the pre-loading of Cleaner class... So here we are, at an attempt to clean this up: https://bugs.openjdk.java.net/browse/JDK-8055232 We can move (r instanceof Cleaner) check outside of synchronized block to where it was before the 2nd fix and wait what stress tests will show. Another possibility is to move the instanceof check outside of synchronized block, but handle the hypothetical OOME by re-linking the unlinked reference back into the pending chain: http://cr.openjdk.java.net/~plevart/jdk9-dev/ReferenceHandlerExceptions/web… What would you suggest? Regards, Peter

2 4

RFR: 8058520: jar xf does not work on zip files with leading garbage.
by Martin Buchholz 18 Sep '14

18 Sep '14

Hi Alan, Xueming, I'd like you to do a code review. https://bugs.openjdk.java.net/browse/JDK-8058520 http://cr.openjdk.java.net/~martin/webrevs/openjdk9/jar-leadingGarbage/

2 4

[8u40] RFR: JDK-8058632: Revert JDK-8054984 from 8u40
by Joel Borggrén-Franck 18 Sep '14

18 Sep '14

Hi, Can I get a review for this anti-patch for JDK-8054984 (which is a backport of JDK-8044629 "(reflect) Constructor.getAnnotatedReceiverType() returns wrong value” to 8u). The fix shouldn’t have been back ported, so this change reverts it in 8u only. Webrev: http://cr.openjdk.java.net/~jfranck/8058632/webrev.00/ Side-by-side-diff: http://cr.openjdk.java.net/~jfranck/8058632/side-by-side.diff Original jdk8u webrev for reference: http://cr.openjdk.java.net/~jfranck/8044629/jdk8u/ cheers /Joel

2 2

[9] RFR (XXS): 8058309: Unsafe.defineAnonymousClass deoptimization checks scale devastatingly poorly
by Vladimir Ivanov 18 Sep '14

18 Sep '14

http://cr.openjdk.java.net/~vlivanov/8058309/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8058309 j.l.i.LambdaForm class has ~50-100 dependent nmethods on average. For every compiled LF, Unsafe.defineAnonymousClass() should validate all dependencies on parent class. Since thousands of LFs are compiled, VM wastes considerable amount of time re-validating dependencies. Most of the dependencies on j.l.i.LambdaForm are of type unique_concrete_method and can't be invalidated with the code generated for LFs. The fix is to decouple compiled LambdaForms and j.l.i.LambdaForm class and use j.l.Object as a parent class instead. Since LambdaForm is already used as a host class, all access checks pass. Reviewed-by: vlivanov, jrose, ? Contributed-by: aleksey.shipilev(a)oracle.com Testing: jck (api/java_lang/invoke), jdk/java/lang/invoke, jdk/java/util/streams w/ "-ea -esa" and COMPILE_THRESHOLD={0,30}, Aleksey refers to performance analysis in the bug [1]. Thanks! Best regards, Vladimir Ivanov [1] http://cr.openjdk.java.net/~shade/8058309/usdacdependency.txt

3 8

RFR 8058569: Update java/lang/invoke/lambda tests to eliminate dependency on sun.tools.jar.Main
by Amy Lu 18 Sep '14

18 Sep '14

Some java/lang/invoke/lambda tests have dependency on sun.tools.jar.Main This fix is to remove above internal JDK API dependency from the tests. bug: https://bugs.openjdk.java.net/browse/JDK-8058569 webrev: http://cr.openjdk.java.net/~tyan/amylu/8058569/webrev.01/ Thanks, Amy

4 7

JDK 9 RFR of 8058664: Bad fonts in BigIntegerTest
by Brian Burkhalter 17 Sep '14

17 Sep '14

Hello, Issue: https://bugs.openjdk.java.net/browse/JDK-8058664 Webrev: http://cr.openjdk.java.net/~bpb/8058664/webrev.00/ Somehow some bad fonts got in to the line prefixes of some comments and none of the tools caught it. Thanks, Brian

3 2

RFR 8057793 BigDecimal is no longer effectively immutable
by Robert Gibson 17 Sep '14

17 Sep '14

Hi there, I came across a case in BigDecimal division where the dividend ends up getting mutated, which is rather strange for a seemingly immutable class! (It's a subset of the cases where the Burnikel-Ziegler algorithm is used, I haven't done the analysis to find out under which exact conditions it's triggered.) The attached patch - against the JDK8 version - should fix the problem, at the cost of an extra array copy. Could somebody review and/or comment please? Thanks, Robert --- MutableBigInteger.java 2014-09-04 09:42:23.426815000 +0200 +++ MutableBigInteger.java.patched 2014-09-04 09:46:21.344132000 +0200 @@ -1261,19 +1261,20 @@ int sigma = (int) Math.max(0, n32 - b.bitLength()); // step 3: sigma = max{T | (2^T)*B < beta^n} MutableBigInteger bShifted = new MutableBigInteger(b); bShifted.safeLeftShift(sigma); // step 4a: shift b so its length is a multiple of n - safeLeftShift(sigma); // step 4b: shift this by the same amount + MutableBigInteger aShifted = new MutableBigInteger (this); + aShifted.safeLeftShift(sigma); // step 4b: shift a by the same amount - // step 5: t is the number of blocks needed to accommodate this plus one additional bit - int t = (int) ((bitLength()+n32) / n32); + // step 5: t is the number of blocks needed to accommodate a plus one additional bit + int t = (int) ((aShifted.bitLength()+n32) / n32); if (t < 2) { t = 2; } - // step 6: conceptually split this into blocks a[t-1], ..., a[0] - MutableBigInteger a1 = getBlock(t-1, t, n); // the most significant block of this + // step 6: conceptually split a into blocks a[t-1], ..., a[0] + MutableBigInteger a1 = aShifted.getBlock(t-1, t, n); // the most significant block of a // step 7: z[t-2] = [a[t-1], a[t-2]] - MutableBigInteger z = getBlock(t-2, t, n); // the second to most significant block + MutableBigInteger z = aShifted.getBlock(t-2, t, n); // the second to most significant block z.addDisjoint(a1, n); // z[t-2] // do schoolbook division on blocks, dividing 2-block numbers by 1-block numbers @@ -1284,7 +1285,7 @@ ri = z.divide2n1n(bShifted, qi); // step 8b: z = [ri, a[i-1]] - z = getBlock(i-1, t, n); // a[i-1] + z = aShifted.getBlock(i-1, t, n); // a[i-1] z.addDisjoint(ri, n); quotient.addShifted(qi, i*n); // update q (part of step 9) } @@ -1292,7 +1293,7 @@ ri = z.divide2n1n(bShifted, qi); quotient.add(qi); - ri.rightShift(sigma); // step 9: this and b were shifted, so shift back + ri.rightShift(sigma); // step 9: a and b were shifted, so shift back return ri; } }

5 14