Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently
On 20 Dec 2013, at 04:33, Mandy Chung mandy.ch...@oracle.com wrote:
Hi Srikalyan,
Maybe you can get add an uncaught handler to see if you can get
any information.
+1. With this, at least the next time we see this failure we should have a
better idea where the OOM is coming from.
-Chris.
I ran it for 1000 times but not able to duplicate
the failure. Did you run it with jtreg (I didn't)?
Below is the patch to install a thread's uncaught handler that
you can take and try.
diff --git a/test/java/lang/ref/OOMEInReferenceHandler.java
b/test/java/lang/ref/OOMEInReferenceHand
ler.java
--- a/test/java/lang/ref/OOMEInReferenceHandler.java
+++ b/test/java/lang/ref/OOMEInReferenceHandler.java
@@ -51,6 +51,14 @@
return first;
}
+ static class UEH implements Thread.UncaughtExceptionHandler {
+ public void uncaughtException(Thread t, Throwable e) {
+ System.err.println(ERROR: + t.getName() + exception +
+ e.getMessage());
+ e.printStackTrace();
+ }
+ }
+
public static void main(String[] args) throws Exception {
// preinitialize the InterruptedException class so that the
reference handler
// does not die due to OOME when loading the class if it is the
first use
@@ -77,6 +85,8 @@
throw new IllegalStateException(Couldn't find Reference Handler
thread.);
}
+ referenceHandlerThread.setUncaughtExceptionHandler(new UEH());
+
ReferenceQueueObject refQueue = new ReferenceQueue();
Object referent = new Object();
WeakReferenceObject weakRef = new WeakReference(referent,
refQueue);
On 12/19/2013 6:57 PM, srikalyan chandrashekar wrote:
Hi David Thanks for your comments, the unguarded part(clean and enqueue) in
the Reference Handler thread does not seem to create any new objects, so it
is the application(the test in this case) which is adding objects to heap
and causing the Reference Handler to die with OOME. I am still unsure about
the side effects of the code change and agree with your thoughts(on memory
exhaustion test's reliability).
PS: hotspot dev alias removed from CC.
--
Thanks
kalyan
On 12/19/13 5:08 PM, David Holmes wrote:
Hi Kalyan,
This is not a hotspot issue so I'm moving this to core-libs, please drop
hotspot from any replies.
On 20/12/2013 6:26 AM, srikalyan wrote:
Hi all, I have been working on the bug JDK-8022321
https://bugs.openjdk.java.net/browse/JDK-8022321 , this is a sporadic
failure and the webrev is available here
http://cr.openjdk.java.net/~srikchan/Regression/JDK-8022321_OOMEInReferenceHandler-webrev/
I'm really not sure what to make of this. We have a test that triggers an
out-of-memory condition but the OOME can actually turn up in the
ReferenceHandler thread causing it to terminate and the test to fail. We
previously accounted for the non-obvious occurrences of OOME due to the
Object.wait and the possible need to load the InterruptedException class -
but still the OOME can appear where we don't want it. So finally you have
just placed the whole for(;;) loop in a try/catch(OOME) that ignores the
OOME. I'm certain that makes the test happy, but I'm not sure it is really
what we want for the ReferenceHandler thread. If the OOME occurs while
cleaning, or enqueuing then we will fail to clean and/or enqueue but there
would be no indication that has occurred and I think that is a bigger
problem than this test failing.
There may be no way to make this test 100% reliable. In fact I'd suggest
that no memory exhaustion test can be 100% reliable.
David
*
**Root Cause:Still not known*
2 places where there is a possibility for OOME
1) Cleaner.clean()
2) ReferenceQueue.enqueue()
1) The cleanup code in turn has 2 places where there is potential for
throwing OOME,
a) thunk Thread which is run from clean() method. This Runnable is
passed to Cleaner and appears in the following classes
java/nio/DirectByteBuffer.java
sun/misc/Perf.java
sun/nio/fs/NativeBuffer.java
sun/nio/ch/IOVecWrapper.java
sun/misc/Cleaner/ExitOnThrow.java
However none of the above overridden implementations ever create an
object in the clean() code.
b) new PrivilegedAction created in try catch Exception block of
clean() method but for this object to be created and to be held
responsible for OOME an Exception(other than OOME) has to be thrown.
2) No new heap objects are created in the enqueue method nor anywhere in
the deep call stack (VM.addFinalRefCount() etc) so this cannot be a
potential cause.
*Experimental change to java.lang.Reference.java* :
- Put one more guard (try catch with OOME block) in the Reference
Handler Thread which may give the Reference Handler a chance to cleanup.
This is fixing the test failure (several 1000 runs with 0 failures)
- Without the above change
Re: RFR of lang level code migration patches
On Dec 19, 2013, at 5:44 PM, Brian Goetz brian.go...@oracle.com wrote:
+1 on all changes, except perhaps for this one in Collections.copy:
ListIterator? super T di=dest.listIterator();
ListIterator? extends T si=src.listIterator();
for (int i=0; isrcSize; i++) {
di.next();
di.set(si.next());
}
The code is bizarre enough (calling next based on a range loop rather than
hasNext()) to leave alone.
Well spotted. I guess the reason is performance, since the size constraints are
checked before looping there is no need to call hasNext (probably for the
second time since next will probably call it :-) ).
On Dec 19, 2013, at 8:22 PM, Mike Duigou mike.dui...@oracle.com wrote:
Compress catches
http://cr.openjdk.java.net/~psandoz/tl/j.u.catch/webrev/
The comment in JarVerifier // e.g. sun.security.pkcs.ParsingException is
now probably too broad to be useful. Maybe just remove it.
Done.
On Dec 19, 2013, at 7:33 PM, Alan Bateman alan.bate...@oracle.com wrote:
I skimmed over the changes too and they look okay. I'm sometimes wary of IDEs
re-arranging things but there's nothing here.
Yes, it is important to verify what the IDE is doing rather, more so for
non-diamond-like transformations, as it sometimes gets it wrong and may monkey
around with formatting [*].
It would be good to get a few other areas done too while things are quiet.
Yes. Both NetBeans and IntelliJ have the appropriate refactoring capabilities.
I can take on java.lang.
Paul.
[*] are we ever likely to agree on a single, tool-compatible, code format in
the JDK before the heat death of the universe?
Re: i18n dev RFR: 8025051: Update resource files for TimeZone display names
Masayoshi, Thank you for the detailed review and your comments. I tried to address all of them. The responses are below. The new webrev can be found here: http://cr.openjdk.java.net/~aefimov/8025051/8/webrev.01/ http://cr.openjdk.java.net/%7Eaefimov/8025051/8/webrev.01/ Michael, As Masayoshi mentioned, we have a list of inconsistent translations in translation files. I suggest two approaches to resolve it: 1. Log different bug with all problems in translation files. 2. Wait for the next resource file translation update to address these problems. -Aleksej On 12/19/2013 12:04 PM, Masayoshi Okutsu wrote: On 12/18/2013 6:43 PM, Aleksej Efimov wrote: Hi, Please help to review a fix [1] for 8025051 bug [2]. The following fix includes: Common to all modified files: - All year ranges in the copyright header should be modified accordingly. Fixed. - The translation of time zone generic names were added to all locales. I haven't fully reviewed translations, especially all \u strings. But I noticed the following. Common to all TimeZoneNames_*.java: - The same generic abbreviation is used for the long name in MET. I thought this was fixed in the root properties... No, the MET is used in root properties both for generic long and short names. I will update these names when new translation update will be available. - Some generic names don't match the style of their standard and/or daylight time names. The same as previous. This is a first large update for generic names and latest translations. All inconsistency in translations will be fixed during next translation update. src/share/classes/sun/util/resources/de/TimeZoneNames_de.java: - Some generic names use Normalzeit. Is that OK? It's not good. But the resolution is same as previous two. src/share/classes/sun/util/resources/ja/TimeZoneNames_ja.java: - Chuuk Time isn't translated. I think it will be translated in next translations update. - Time Zone names were updated according to the latest translations. - Added tz names regression test (test/sun/util/resources/TimeZone/TimeZoneNames) for a timezone names across all locales. This test compares short/long standard/daylight/generic names with translations stored in .properties files. test/sun/util/resources/TimeZone/TimeZoneNames/TimeZoneNamesTest.java: - lines 33, 34: unused imports? Removed - line 75: Should it be detected as an error? Agree, now the test will fail if there is some incorrect entries in the test .properties files. - line 108: IOException should be used as a cause. (OK to assume not found?) The IOException cause is added. Currently, the test will stop with RuntimeException, because the test file is not found. - lines 118 -121: Locale.getDefault() has to be replaced with Locale.ROOT. Regression tests are run in different locales. Thank you for catching this one. Fixed. - Tests updates to address current changes ( GenericTimeZoneNamesTest.sh and Bug6317929.java). The TODO comment in GenericTimeZoneNamesTest.sh should fully been removed. Removed The following fix was tested with JPRT build and the 'jdk_util' test set succeeded (new test is included in this test set). Have you executed all date-time related regression tests (including java.time and java.text)? Yes, I have executed the following set of tests via JTREG: test/sun/util/calendar test/java/util/Calendar test/sun/util/resources/TimeZone test/sun/util/calendar test/closed/java/util/TimeZone test/java/util/TimeZone test/java/time test/java/util/Formatter test/closed/java/util/Calendar All tests gave PASS results. Please, let me know if you think that some other tests should be executed. Thanks, Masayoshi [1] http://cr.openjdk.java.net/~aefimov/8025051/8/webrev.00/ [2] https://bugs.openjdk.java.net/browse/JDK-8025051
Re: RFR: JDK-8028712 : Tidy warnings cleanup for java.sql package
Hi all. Please review a second fix http://cr.openjdk.java.net/~yan/8028712/webrev.03/ for https://bugs.openjdk.java.net/browse/JDK-8028712 I deleted part of java/sql/package.html, and replaced br/ to br for compliance with html 3.2 On 12/05/2013 10:39 PM, Lance Andersen - Oracle wrote: Hi Serge This looks OK. For --- old/src/share/classes/java/sql/package.html 2013-12-05 15:08:50.587885460 + +++ new/src/share/classes/java/sql/package.html 2013-12-05 15:08:50.435885464 + Please remove the following Package Specification . Specification of the JDBC 4.0 API Related Documentation . Getting Started--overviews of the major interfaces . Chapters on the JDBC API--from the online version of The Java Tutorial Continued . JDBCTMAPI Tutorial and Reference, Third Edition-- a complete reference and tutorial for the JDBC 3.0 API The above links keep breaking now that we are off of java.sun.com And I agree with roger, please use br Alan, yes I can look to clean up some of the formatting crud for Java SE 9 once we have access to the workspace Thank you for doing this Serge. Best Lance On Dec 5, 2013, at 10:31 AM, Alan Bateman wrote: On 05/12/2013 17:25, Serge wrote: Hi all, please review the fix http://cr.openjdk.java.net/~yan/8028712/webrev.02/ http://cr.openjdk.java.net/%7Eyan/8028712/webrev.02/ for https://bugs.openjdk.java.net/browse/JDK-8028712 This patch cleanup tidy warnings for generated html documentation, and do not affect the appearance of the documentation. The removal of the p tags seems okay. The only thing that I'm not sure about the addition of br/ to the package docs (is that needed?). Lance - while scanning this patch then it appears that some of the formatting in the javadoc comments is all over the place (java.sql.Connection is one example). It might be good to clean that up once the jdk9 project is open for business. -Alan Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 lance.ander...@oracle.com
Re: RFR for JDK-6963118 Intermittent test failure: test/java/nio/channels/Selector/Wakeup.java fail intermittently (win)
On 20/12/2013 04:18, David Holmes wrote: : Increasing the timeouts seems okay but how confident are you that this is sufficient for a wide range of platforms. In other tests we often see timeouts of a few seconds get extended even further, so three seconds is not so big. Also the yield loops: while (sleeper.entries 5) Thread.yield(); would be better as sleep loops (as used elsewhere) to avoid potential issues with yield being a no-op on some platforms (Yes I see it is already used that way elsewhere in the test but the sleep is better :) ). Kalyan started another thread on this one and in the latest revision then the timeout has been increased to 10 seconds. I agree, it would be better to use a small sleep rather than yield. -Alan.
Re: Bug in Long.parseUnsignedLong
Hi Brian,
It would be nice to avoid the caches, on a hunch i am wondering if the
following will work:
long result = first * radix + second;
if ((first 1) (Long.MAX_VALUE / radix) || // possible overflow of
first * radix
Long.compareUnsigned(result, first) 0) { // overflow of first * radix +
second
It should be possible to write some tests creating strings in the appropriate
base to select the appropriate value for first will kick off an overflow.
Paul.
On Dec 19, 2013, at 9:21 PM, Brian Burkhalter brian.burkhal...@oracle.com
wrote:
Here's a formalization of the suggested fix:
http://cr.openjdk.java.net/~bpb/8030814/webrev/
It works against the test case.
Brian
On Dec 19, 2013, at 11:26 AM, Brian Burkhalter wrote:
Upon inspection only that indeed looks correct.
Thanks …
On Dec 19, 2013, at 10:28 AM, Louis Wasserman wrote:
Here's one approach that works: there is overflow iff
compareUnsigned(first, divideUnsigned(MAX_UNSIGNED, radix)) 0 || (first
== divideUnsigned(MAX_UNSIGNED, radix) second
remainderUnsigned(MAX_UNSIGNED, radix));
Since radix = Character.MAX_RADIX, you can precompute the divides and
remainders in a small table.
Re: Bug in Long.parseUnsignedLong
What is performance of Long.compareUnsigned(x, y) ?
Does HotSpot implement it as a call of Long.compare(x + MIN_VALUE, y +
MIN_VALUE) or as a single machine instruction ?
On Fri, Dec 20, 2013 at 7:53 PM, Paul Sandoz paul.san...@oracle.com wrote:
Hi Brian,
It would be nice to avoid the caches, on a hunch i am wondering if the
following will work:
long result = first * radix + second;
if ((first 1) (Long.MAX_VALUE / radix) || // possible overflow of
first * radix
Long.compareUnsigned(result, first) 0) { // overflow of first *
radix + second
It should be possible to write some tests creating strings in the
appropriate base to select the appropriate value for first will kick off an
overflow.
Paul.
On Dec 19, 2013, at 9:21 PM, Brian Burkhalter brian.burkhal...@oracle.com
wrote:
Here's a formalization of the suggested fix:
http://cr.openjdk.java.net/~bpb/8030814/webrev/
It works against the test case.
Brian
On Dec 19, 2013, at 11:26 AM, Brian Burkhalter wrote:
Upon inspection only that indeed looks correct.
Thanks …
On Dec 19, 2013, at 10:28 AM, Louis Wasserman wrote:
Here's one approach that works: there is overflow iff
compareUnsigned(first, divideUnsigned(MAX_UNSIGNED, radix)) 0 ||
(first == divideUnsigned(MAX_UNSIGNED, radix) second
remainderUnsigned(MAX_UNSIGNED, radix));
Since radix = Character.MAX_RADIX, you can precompute the divides and
remainders in a small table.
Re: Demo for Parallel Core Collection API
Hi Tristan,
Thanks, I need to look at this in more detail, but here are some quick comments.
- recommend you try and avoid using limit with parallel ops, for example the Pi
example cam be reformulated as:
long M = LongStream.range(0, N).parallel().filter(sr - {
double x = ThreadLocalRandom.current().nextDouble(-1, 1);
double y = ThreadLocalRandom.current().nextDouble(-1, 1);
return x * x + y * y R * R; // Don't use need to use sqrt
}).count();
double pi = (M / N) * 4.0;
the Primes example could be reformulated as:
LongStream.range(0, limit).parallel().map(/odd
values/).filter(RandomPrimeNumber::isPrime).findAny();
you don't need to declare unordered() since findAny implicitly makes the stream
unordered by definition.
The key message here is range has better decomposition characteristics than
generate or iterate.
More later, probably after the break,
Paul.
On Dec 19, 2013, at 1:16 PM, Tristan Yan tristan@oracle.com wrote:
Hi Paul And Everyone
Sorry for getting back late.
I took Paul's suggestion and have written other two demos which presents
usage of parallel computation. One is using Monte-Carlo to calculate value of
PI. Other is find a big prime by given length. Please review it.
http://cr.openjdk.java.net/~tyan/sample/webrev.00/
There is another demo which present mandelbrot set was designed Alexander
Kouznetsov has been already in reviewing. It's not my code review request.
Thank you very much
Tristan
hg: jdk8/tl/jdk: 8029955: AIOB in XMLEntityScanner.scanLiteral upon parsing literals with 100 LF chars
Changeset: 73473e9dfc46 Author:joehw Date: 2013-12-20 09:56 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/73473e9dfc46 8029955: AIOB in XMLEntityScanner.scanLiteral upon parsing literals with 100 LF chars Reviewed-by: dfuchs, lancea, ulfzibis + test/javax/xml/jaxp/parsers/8029955/EntityScannerTest.java
Re: RFR for JDK-6772009 Intermittent test failure: java/util/concurrent/locks/ReentrantLock/CancelledLockLoops.java test failed with 'Completed != 2'
Hi David, i retained only the changes to ITERS, ProbleMList.txt and upstream changes by Doug Lea(as pointed by Martin), could you please review the new change available here http://cr.openjdk.java.net/~srikchan/Regression/6772009-CancelledLockLoop-webrev/ . -- Thanks kalyan Ph: (408)-585-8040 On 12/19/13, 8:10 PM, David Holmes wrote: Sorry Kalyan but I don't see the need for all the incidental changes if the primary change is to just increase the iterations. I also don't see why you need to do anything for BrokenBarrierException as it is not expected to happen and the test should just fail if it does. David On 10/12/2013 6:15 AM, srikalyan wrote: Hi David/Martin a gentle reminder for review. -- Thanks kalyan Ph: (408)-585-8040 On 12/2/13, 11:21 AM, srikalyan wrote: Hi David, Thanks for the review, the new webrev is hosted at http://cr.openjdk.java.net/~cl/host_for_kal/6772009-CancelledLockLoop/ . Please see inline text. On 11/20/13, 6:35 PM, David Holmes wrote: On 21/11/2013 10:28 AM, Martin Buchholz wrote: I again tried and failed to reproduce a failure. Even if I go whole hog and multiply TIMEOUT by 100 and divide ITERS by 100, the test continues to PASS. Is it just me?! I think you are going the wrong way Martin - you want the timeout to be smaller than the time it takes to execute ITERS. I don't think there's any reason to make result long. It's not even used except to inhibit hotspot optimizations. +private volatile long result = 17;//Can get int overflow,so using long Further the subsequent use of += is incorrect as it is not an atomic operation. Even if we don't care about the value, it looks bad. Made the necessary changes for atomic update. I'm not sure result must be updated if we get a BrokenBarrierException either. Probably harmless, but necessary? I retained it in the fix for completeness in updating the numbers, please let me know if you still think otherwise. need to fix spelling and spacing below. +barrier.await();//If a BrokeBarrierException happens here(due to There are a number of style issues with spacing around comments. Fixed the spelling error and styling issues. And I don't think this change is sufficient to claim co-author status with Doug either ;-) Removed the claim :) The additional tracing may be useful but seems stylistically different from the rest of the code. Retained the tracking to understand if it is again the timing issue which is the cause in an event of a failure, however i can remove it if you think it is not necessary (OR) include an alternate solution as you may want to suggest. Overall I'm suspicious that the changed timeout actually fixes anything - normally we need to add longer timeouts not shorter ones. Does this fail on a range of machines or only specific ones? Have we verified that the clocks/timers are behaving properly on those systems? Here the time out is not about waiting for threads to complete something but to be interrupted before being considered done, so we decreased the timeout. However we now chose to increase the number of iterations to 500 from 100(thanks to tristan for the suggestion) instead of decreasing the timeout as done earlier because the increasing iterations ensures the threads are busy for long time curtailing the need to touch the timeout. Thanks, David -- Thanks kalyan Ph: (408)-585-8040 On Wed, Nov 20, 2013 at 11:52 AM, srikalyan srikalyan.chandrashe...@oracle.com wrote: Hi Martin , apologies for the delay , was trying to get help for hosting my webrev. . Please see inline text. On 11/19/13, 10:35 PM, Martin Buchholz wrote: Hi Kalyan, None of us can review your changes yet because you haven't given us a URL of your webrev. It is located here http://cr.openjdk.java.net/~cl/host_for_srikalyan_6772009_CancelledLockLoops/ I've tried to make the jsr166 copy of CancelledLockLoops fail by adjusting ITERS and TIMEOUT radically up and down, but the test just keeps on passing for me. Hints appreciated. Bump up the timeout to 500ms and you will see a failure (i can see it consistently on my machine Linux 64bit,8GBRAM,dual cores, with JDK 1.8 latest any promoted build). On Tue, Nov 19, 2013 at 6:39 PM, srikalyan chandrashekar srikalyan.chandrashe...@oracle.com wrote: Suggested Fix: a) Decrease the timeout from 100 to 50ms which will ensure that the test will pass even on faster machines This doesn't look like a permanent fix - it just makes the failing case rarer. Thats true , the other way is to make the main thread wait on TIMEOUT after firing the interrupts instead of other way round, but that would be over-optimization which probably is not desirable as well. The 50 ms was arrived at empirically after running several 100 times on multiple configurations and did not cause failures. -- Thanks kalyan Ph: (408)-585-8040
hg: jdk8/tl/jdk: 2 new changesets
Changeset: 7186275e6ef1 Author:rriggs Date: 2013-12-20 13:06 -0500 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/7186275e6ef1 8030002: Enhance deserialization using readObject Reviewed-by: sherman, chegar, scolebourne ! src/share/classes/java/time/Duration.java ! src/share/classes/java/time/Instant.java ! src/share/classes/java/time/LocalDate.java ! src/share/classes/java/time/LocalDateTime.java ! src/share/classes/java/time/LocalTime.java ! src/share/classes/java/time/MonthDay.java ! src/share/classes/java/time/OffsetDateTime.java ! src/share/classes/java/time/OffsetTime.java ! src/share/classes/java/time/Period.java ! src/share/classes/java/time/Year.java ! src/share/classes/java/time/YearMonth.java ! src/share/classes/java/time/ZoneId.java ! src/share/classes/java/time/ZoneOffset.java ! src/share/classes/java/time/ZoneRegion.java ! src/share/classes/java/time/ZonedDateTime.java ! src/share/classes/java/time/chrono/AbstractChronology.java ! src/share/classes/java/time/chrono/ChronoLocalDateTimeImpl.java ! src/share/classes/java/time/chrono/ChronoPeriodImpl.java ! src/share/classes/java/time/chrono/ChronoZonedDateTimeImpl.java ! src/share/classes/java/time/chrono/HijrahChronology.java ! src/share/classes/java/time/chrono/HijrahDate.java ! src/share/classes/java/time/chrono/IsoChronology.java ! src/share/classes/java/time/chrono/JapaneseChronology.java ! src/share/classes/java/time/chrono/JapaneseDate.java ! src/share/classes/java/time/chrono/JapaneseEra.java ! src/share/classes/java/time/chrono/MinguoChronology.java ! src/share/classes/java/time/chrono/MinguoDate.java ! src/share/classes/java/time/chrono/ThaiBuddhistChronology.java ! src/share/classes/java/time/chrono/ThaiBuddhistDate.java ! src/share/classes/java/time/temporal/ValueRange.java ! src/share/classes/java/time/temporal/WeekFields.java ! src/share/classes/java/time/zone/ZoneOffsetTransition.java ! src/share/classes/java/time/zone/ZoneOffsetTransitionRule.java ! src/share/classes/java/time/zone/ZoneRules.java ! test/java/time/tck/java/time/AbstractTCKTest.java ! test/java/time/tck/java/time/chrono/serial/TCKChronoLocalDateSerialization.java ! test/java/time/tck/java/time/chrono/serial/TCKChronologySerialization.java ! test/java/time/tck/java/time/serial/TCKDurationSerialization.java ! test/java/time/tck/java/time/serial/TCKInstantSerialization.java ! test/java/time/tck/java/time/serial/TCKLocalDateSerialization.java ! test/java/time/tck/java/time/serial/TCKLocalDateTimeSerialization.java ! test/java/time/tck/java/time/serial/TCKLocalTimeSerialization.java ! test/java/time/tck/java/time/serial/TCKMonthDaySerialization.java ! test/java/time/tck/java/time/serial/TCKOffsetDateTimeSerialization.java ! test/java/time/tck/java/time/serial/TCKOffsetTimeSerialization.java ! test/java/time/tck/java/time/serial/TCKPeriodSerialization.java ! test/java/time/tck/java/time/serial/TCKYearMonthSerialization.java ! test/java/time/tck/java/time/serial/TCKYearSerialization.java ! test/java/time/tck/java/time/serial/TCKZoneOffsetSerialization.java ! test/java/time/tck/java/time/serial/TCKZonedDateTimeSerialization.java ! test/java/time/tck/java/time/temporal/serial/TCKValueRangeSerialization.java ! test/java/time/tck/java/time/temporal/serial/TCKWeekFieldsSerialization.java ! test/java/time/tck/java/time/zone/serial/TCKZoneOffsetTransitionRuleSerialization.java ! test/java/time/tck/java/time/zone/serial/TCKZoneOffsetTransitionSerialization.java ! test/java/time/tck/java/time/zone/serial/TCKZoneRulesSerialization.java Changeset: 39a02b18b386 Author:rriggs Date: 2013-12-20 13:06 -0500 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/39a02b18b386 8029909: Clarify equals/hashcode behavior for java.time types Summary: Document the behavior of equals and hashcode in java.time.chrono date types Reviewed-by: sherman, scolebourne ! src/share/classes/java/time/chrono/HijrahDate.java ! src/share/classes/java/time/chrono/JapaneseDate.java ! src/share/classes/java/time/chrono/MinguoDate.java ! src/share/classes/java/time/chrono/ThaiBuddhistDate.java
JDK 9 RFR of JDK-8030785: Missing since 1.8 javadoc for java.lang.reflect.Method:getParameterCount
Hello,
Please review the patch below to fix
JDK-8030785: Missing since 1.8 javadoc for
java.lang.reflect.Method:getParameterCount
The Executable type was added in 8 so none of the method in Executable
should have an @since 1.8 tag.
In the Constructor and Executable subtypes, @since 1.8 is added to the
getParameterCount method. I didn't seen any other similar situations
that needed to be corrected.
Thanks,
-Joe
diff -r 33c3c4c0ebcf src/share/classes/java/lang/reflect/Constructor.java
--- a/src/share/classes/java/lang/reflect/Constructor.javaFri Dec 20
08:59:52 2013 -0800
+++ b/src/share/classes/java/lang/reflect/Constructor.javaFri Dec 20
10:26:15 2013 -0800
@@ -204,6 +204,7 @@
/**
* {@inheritDoc}
+ * @since 1.8
*/
public int getParameterCount() { return parameterTypes.length; }
diff -r 33c3c4c0ebcf src/share/classes/java/lang/reflect/Executable.java
--- a/src/share/classes/java/lang/reflect/Executable.javaFri Dec 20
08:59:52 2013 -0800
+++ b/src/share/classes/java/lang/reflect/Executable.javaFri Dec 20
10:26:15 2013 -0800
@@ -240,7 +240,6 @@
* declared or implicitly declared or neither) for the executable
* represented by this object.
*
- * @since 1.8
* @return The number of formal parameters for the executable this
* object represents
darcy@ubuntu:~/Sun/OpenJDK/9/dev/jdk$ hg diff | more
diff -r 33c3c4c0ebcf src/share/classes/java/lang/reflect/Constructor.java
--- a/src/share/classes/java/lang/reflect/Constructor.javaFri Dec 20
08:59:52 2013 -0800
+++ b/src/share/classes/java/lang/reflect/Constructor.javaFri Dec 20
10:26:17 2013 -0800
@@ -204,6 +204,7 @@
/**
* {@inheritDoc}
+ * @since 1.8
*/
public int getParameterCount() { return parameterTypes.length; }
diff -r 33c3c4c0ebcf src/share/classes/java/lang/reflect/Executable.java
--- a/src/share/classes/java/lang/reflect/Executable.javaFri Dec 20
08:59:52 2013 -0800
+++ b/src/share/classes/java/lang/reflect/Executable.javaFri Dec 20
10:26:17 2013 -0800
@@ -240,7 +240,6 @@
* declared or implicitly declared or neither) for the executable
* represented by this object.
*
- * @since 1.8
* @return The number of formal parameters for the executable this
* object represents
*/
@@ -291,7 +290,6 @@
* have unique names, or names that are legal identifiers in the
* Java programming language (JLS 3.8).
*
- * @since 1.8
* @throws MalformedParametersException if the class file contains
* a MethodParameters attribute that is improperly formatted.
* @return an array of {@code Parameter} objects representing all
@@ -523,7 +521,6 @@
/**
* {@inheritDoc}
* @throws NullPointerException {@inheritDoc}
- * @since 1.8
*/
@Override
public T extends Annotation T[] getAnnotationsByType(ClassT
annotationClass) {
@@ -566,8 +563,6 @@
*
* @return an object representing the return type of the method
* or constructor represented by this {@code Executable}
- *
- * @since 1.8
*/
public abstract AnnotatedType getAnnotatedReturnType();
@@ -576,8 +571,6 @@
* Returns an AnnotatedType object that represents the use of a
type to
* specify the return type of the method/constructor represented
by this
* Executable.
- *
- * @since 1.8
*/
AnnotatedType getAnnotatedReturnType0(Type returnType) {
return
TypeAnnotationParser.buildAnnotatedType(getTypeAnnotationBytes0(),
@@ -607,8 +600,6 @@
*
* @return an object representing the receiver type of the method or
* constructor represented by this {@code Executable}
- *
- * @since 1.8
*/
public AnnotatedType getAnnotatedReceiverType() {
if (Modifier.isStatic(this.getModifiers()))
@@ -635,8 +626,6 @@
* @return an array of objects representing the types of the
* formal parameters of the method or constructor represented by this
* {@code Executable}
- *
- * @since 1.8
*/
public AnnotatedType[] getAnnotatedParameterTypes() {
return
TypeAnnotationParser.buildAnnotatedTypes(getTypeAnnotationBytes0(),
@@ -661,8 +650,6 @@
* @return an array of objects representing the declared
* exceptions of the method or constructor represented by this {@code
* Executable}
- *
- * @since 1.8
*/
public AnnotatedType[] getAnnotatedExceptionTypes() {
return
TypeAnnotationParser.buildAnnotatedTypes(getTypeAnnotationBytes0(),
diff -r 33c3c4c0ebcf src/share/classes/java/lang/reflect/Method.java
--- a/src/share/classes/java/lang/reflect/Method.javaFri Dec 20
08:59:52 2013 -0800
+++ b/src/share/classes/java/lang/reflect/Method.javaFri Dec 20
10:26:17 2013 -0800
@@ -251,7 +251,8 @@
}
/**
- * {@inheritDoc}
+ * {@inheritDoc}1
+ * @since 1.8
*/
public int
RFR: 8030850: Setting .level=FINEST for the root logger in logging configuration file doesn't work
Hi, Please find below a fix for 8030850: Setting .level=FINEST for the root logger in logging configuration file doesn't work https://bugs.openjdk.java.net/browse/JDK-8030850 http://cr.openjdk.java.net/~dfuchs/webrev_8030850/webrev.00/ This is a regression I introduced with my fix for JDK-8026499. The root logger sets its own level to INFO in its constructor. Then later when the logger is added, it believes that its level has already been initialized/set by the user and so skips its initialization from the config file. Setting the level of the root logger to INFO in the constructor is too early. The fix is very limited - instead of initializing the root level to 'INFO' in the constructor, we can do it just after having added it to the LogManager. At this time the configuration is already read (happened a few lines above), and thus we can set the level to its default value after checking whether it's already been initialized. There should be no synchronization issue since it all happens in the same synchronization lock as before the fix - and the logic ensures that the LogManager will not be published to other threads until the initialization is done. best regards, -- daniel
Re: RFR: 8030850: Setting .level=FINEST for the root logger in logging configuration file doesn't work
On 12/20/2013 10:45 AM, Daniel Fuchs wrote: Hi, Please find below a fix for 8030850: Setting .level=FINEST for the root logger in logging configuration file doesn't work https://bugs.openjdk.java.net/browse/JDK-8030850 http://cr.openjdk.java.net/~dfuchs/webrev_8030850/webrev.00/ Looks good. It's unfortunate that there is no test catching this basic functionality (such kind of tests should exist when j.u.logging was added). You have added many tests to improve the coverage for j.u.logging. Do you think there is a SQE logging test catching this? We will find out. Mandy
Re: JDK 9 RFR of JDK-8030785: Missing since 1.8 javadoc for java.lang.reflect.Method:getParameterCount
+1
Mike
On Dec 20 2013, at 10:29 , Joe Darcy joe.da...@oracle.com wrote:
Hello,
Please review the patch below to fix
JDK-8030785: Missing since 1.8 javadoc for
java.lang.reflect.Method:getParameterCount
The Executable type was added in 8 so none of the method in Executable should
have an @since 1.8 tag.
In the Constructor and Executable subtypes, @since 1.8 is added to the
getParameterCount method. I didn't seen any other similar situations that
needed to be corrected.
Thanks,
-Joe
diff -r 33c3c4c0ebcf src/share/classes/java/lang/reflect/Constructor.java
--- a/src/share/classes/java/lang/reflect/Constructor.javaFri Dec 20
08:59:52 2013 -0800
+++ b/src/share/classes/java/lang/reflect/Constructor.javaFri Dec 20
10:26:15 2013 -0800
@@ -204,6 +204,7 @@
/**
* {@inheritDoc}
+ * @since 1.8
*/
public int getParameterCount() { return parameterTypes.length; }
diff -r 33c3c4c0ebcf src/share/classes/java/lang/reflect/Executable.java
--- a/src/share/classes/java/lang/reflect/Executable.javaFri Dec 20
08:59:52 2013 -0800
+++ b/src/share/classes/java/lang/reflect/Executable.javaFri Dec 20
10:26:15 2013 -0800
@@ -240,7 +240,6 @@
* declared or implicitly declared or neither) for the executable
* represented by this object.
*
- * @since 1.8
* @return The number of formal parameters for the executable this
* object represents
darcy@ubuntu:~/Sun/OpenJDK/9/dev/jdk$ hg diff | more
diff -r 33c3c4c0ebcf src/share/classes/java/lang/reflect/Constructor.java
--- a/src/share/classes/java/lang/reflect/Constructor.javaFri Dec 20
08:59:52 2013 -0800
+++ b/src/share/classes/java/lang/reflect/Constructor.javaFri Dec 20
10:26:17 2013 -0800
@@ -204,6 +204,7 @@
/**
* {@inheritDoc}
+ * @since 1.8
*/
public int getParameterCount() { return parameterTypes.length; }
diff -r 33c3c4c0ebcf src/share/classes/java/lang/reflect/Executable.java
--- a/src/share/classes/java/lang/reflect/Executable.javaFri Dec 20
08:59:52 2013 -0800
+++ b/src/share/classes/java/lang/reflect/Executable.javaFri Dec 20
10:26:17 2013 -0800
@@ -240,7 +240,6 @@
* declared or implicitly declared or neither) for the executable
* represented by this object.
*
- * @since 1.8
* @return The number of formal parameters for the executable this
* object represents
*/
@@ -291,7 +290,6 @@
* have unique names, or names that are legal identifiers in the
* Java programming language (JLS 3.8).
*
- * @since 1.8
* @throws MalformedParametersException if the class file contains
* a MethodParameters attribute that is improperly formatted.
* @return an array of {@code Parameter} objects representing all
@@ -523,7 +521,6 @@
/**
* {@inheritDoc}
* @throws NullPointerException {@inheritDoc}
- * @since 1.8
*/
@Override
public T extends Annotation T[] getAnnotationsByType(ClassT
annotationClass) {
@@ -566,8 +563,6 @@
*
* @return an object representing the return type of the method
* or constructor represented by this {@code Executable}
- *
- * @since 1.8
*/
public abstract AnnotatedType getAnnotatedReturnType();
@@ -576,8 +571,6 @@
* Returns an AnnotatedType object that represents the use of a type to
* specify the return type of the method/constructor represented by this
* Executable.
- *
- * @since 1.8
*/
AnnotatedType getAnnotatedReturnType0(Type returnType) {
return
TypeAnnotationParser.buildAnnotatedType(getTypeAnnotationBytes0(),
@@ -607,8 +600,6 @@
*
* @return an object representing the receiver type of the method or
* constructor represented by this {@code Executable}
- *
- * @since 1.8
*/
public AnnotatedType getAnnotatedReceiverType() {
if (Modifier.isStatic(this.getModifiers()))
@@ -635,8 +626,6 @@
* @return an array of objects representing the types of the
* formal parameters of the method or constructor represented by this
* {@code Executable}
- *
- * @since 1.8
*/
public AnnotatedType[] getAnnotatedParameterTypes() {
return
TypeAnnotationParser.buildAnnotatedTypes(getTypeAnnotationBytes0(),
@@ -661,8 +650,6 @@
* @return an array of objects representing the declared
* exceptions of the method or constructor represented by this {@code
* Executable}
- *
- * @since 1.8
*/
public AnnotatedType[] getAnnotatedExceptionTypes() {
return
TypeAnnotationParser.buildAnnotatedTypes(getTypeAnnotationBytes0(),
diff -r 33c3c4c0ebcf src/share/classes/java/lang/reflect/Method.java
--- a/src/share/classes/java/lang/reflect/Method.javaFri Dec 20 08:59:52
2013 -0800
+++ b/src/share/classes/java/lang/reflect/Method.javaFri Dec 20 10:26:17
Re: Analysis on JDK-8022321 java/lang/ref/OOMEInReferenceHandler.java fails intermittently
Hi Mandy, yes I ran with JTreg to simulate the failure, i will try the
UEH patch to see if it sheds some light and get back to you. Thanks for
the direction :)
--
Thanks
kalyan
Ph: (408)-585-8040
On 12/19/13, 8:33 PM, Mandy Chung wrote:
Hi Srikalyan,
Maybe you can get add an uncaught handler to see if you can get
any information. I ran it for 1000 times but not able to duplicate
the failure. Did you run it with jtreg (I didn't)?
Below is the patch to install a thread's uncaught handler that
you can take and try.
diff --git a/test/java/lang/ref/OOMEInReferenceHandler.java
b/test/java/lang/ref/OOMEInReferenceHand
ler.java
--- a/test/java/lang/ref/OOMEInReferenceHandler.java
+++ b/test/java/lang/ref/OOMEInReferenceHandler.java
@@ -51,6 +51,14 @@
return first;
}
+ static class UEH implements Thread.UncaughtExceptionHandler {
+ public void uncaughtException(Thread t, Throwable e) {
+ System.err.println(ERROR: + t.getName() + exception
+
+ e.getMessage());
+ e.printStackTrace();
+ }
+ }
+
public static void main(String[] args) throws Exception {
// preinitialize the InterruptedException class so that the
reference handler
// does not die due to OOME when loading the class if it is
the first use
@@ -77,6 +85,8 @@
throw new IllegalStateException(Couldn't find Reference
Handler thread.);
}
+ referenceHandlerThread.setUncaughtExceptionHandler(new UEH());
+
ReferenceQueueObject refQueue = new ReferenceQueue();
Object referent = new Object();
WeakReferenceObject weakRef = new
WeakReference(referent, refQueue);
On 12/19/2013 6:57 PM, srikalyan chandrashekar wrote:
Hi David Thanks for your comments, the unguarded part(clean and
enqueue) in the Reference Handler thread does not seem to create any
new objects, so it is the application(the test in this case) which is
adding objects to heap and causing the Reference Handler to die with
OOME. I am still unsure about the side effects of the code change and
agree with your thoughts(on memory exhaustion test's reliability).
PS: hotspot dev alias removed from CC.
--
Thanks
kalyan
On 12/19/13 5:08 PM, David Holmes wrote:
Hi Kalyan,
This is not a hotspot issue so I'm moving this to core-libs, please
drop hotspot from any replies.
On 20/12/2013 6:26 AM, srikalyan wrote:
Hi all, I have been working on the bug JDK-8022321
https://bugs.openjdk.java.net/browse/JDK-8022321 , this is a
sporadic
failure and the webrev is available here
http://cr.openjdk.java.net/~srikchan/Regression/JDK-8022321_OOMEInReferenceHandler-webrev/
I'm really not sure what to make of this. We have a test that
triggers an out-of-memory condition but the OOME can actually turn
up in the ReferenceHandler thread causing it to terminate and the
test to fail. We previously accounted for the non-obvious
occurrences of OOME due to the Object.wait and the possible need to
load the InterruptedException class - but still the OOME can appear
where we don't want it. So finally you have just placed the whole
for(;;) loop in a try/catch(OOME) that ignores the OOME. I'm certain
that makes the test happy, but I'm not sure it is really what we
want for the ReferenceHandler thread. If the OOME occurs while
cleaning, or enqueuing then we will fail to clean and/or enqueue but
there would be no indication that has occurred and I think that is a
bigger problem than this test failing.
There may be no way to make this test 100% reliable. In fact I'd
suggest that no memory exhaustion test can be 100% reliable.
David
*
**Root Cause:Still not known*
2 places where there is a possibility for OOME
1) Cleaner.clean()
2) ReferenceQueue.enqueue()
1) The cleanup code in turn has 2 places where there is potential for
throwing OOME,
a) thunk Thread which is run from clean() method. This
Runnable is
passed to Cleaner and appears in the following classes
java/nio/DirectByteBuffer.java
sun/misc/Perf.java
sun/nio/fs/NativeBuffer.java
sun/nio/ch/IOVecWrapper.java
sun/misc/Cleaner/ExitOnThrow.java
However none of the above overridden implementations ever create an
object in the clean() code.
b) new PrivilegedAction created in try catch Exception block of
clean() method but for this object to be created and to be held
responsible for OOME an Exception(other than OOME) has to be thrown.
2) No new heap objects are created in the enqueue method nor
anywhere in
the deep call stack (VM.addFinalRefCount() etc) so this cannot be a
potential cause.
*Experimental change to java.lang.Reference.java* :
- Put one more guard (try catch with OOME block) in the Reference
Handler Thread which may give the Reference Handler a chance to
cleanup.
This is fixing the test failure (several 1000 runs with 0 failures)
- Without the above change the test fails atleast 3-5 times
Re: JDK 9 RFR of JDK-8030785: Missing since 1.8 javadoc for java.lang.reflect.Method:getParameterCount
--- a/src/share/classes/java/lang/reflect/Method.javaFri Dec 20 08:59:52
2013 -0800
+++ b/src/share/classes/java/lang/reflect/Method.javaFri Dec 20 10:26:17
2013 -0800
@@ -251,7 +251,8 @@
}
/**
- * {@inheritDoc}
+ * {@inheritDoc}1
Is this accident? Other than this, look fine.
Mandy
On 12/20/2013 10:29 AM, Joe Darcy wrote:
Hello,
Please review the patch below to fix
JDK-8030785: Missing since 1.8 javadoc for
java.lang.reflect.Method:getParameterCount
The Executable type was added in 8 so none of the method in Executable
should have an @since 1.8 tag.
In the Constructor and Executable subtypes, @since 1.8 is added to the
getParameterCount method. I didn't seen any other similar situations
that needed to be corrected.
Thanks,
-Joe
diff -r 33c3c4c0ebcf src/share/classes/java/lang/reflect/Constructor.java
--- a/src/share/classes/java/lang/reflect/Constructor.javaFri Dec
20 08:59:52 2013 -0800
+++ b/src/share/classes/java/lang/reflect/Constructor.javaFri Dec
20 10:26:15 2013 -0800
@@ -204,6 +204,7 @@
/**
* {@inheritDoc}
+ * @since 1.8
*/
public int getParameterCount() { return parameterTypes.length; }
diff -r 33c3c4c0ebcf src/share/classes/java/lang/reflect/Executable.java
--- a/src/share/classes/java/lang/reflect/Executable.javaFri Dec
20 08:59:52 2013 -0800
+++ b/src/share/classes/java/lang/reflect/Executable.javaFri Dec
20 10:26:15 2013 -0800
@@ -240,7 +240,6 @@
* declared or implicitly declared or neither) for the executable
* represented by this object.
*
- * @since 1.8
* @return The number of formal parameters for the executable this
* object represents
darcy@ubuntu:~/Sun/OpenJDK/9/dev/jdk$ hg diff | more
diff -r 33c3c4c0ebcf src/share/classes/java/lang/reflect/Constructor.java
--- a/src/share/classes/java/lang/reflect/Constructor.javaFri Dec
20 08:59:52 2013 -0800
+++ b/src/share/classes/java/lang/reflect/Constructor.javaFri Dec
20 10:26:17 2013 -0800
@@ -204,6 +204,7 @@
/**
* {@inheritDoc}
+ * @since 1.8
*/
public int getParameterCount() { return parameterTypes.length; }
diff -r 33c3c4c0ebcf src/share/classes/java/lang/reflect/Executable.java
--- a/src/share/classes/java/lang/reflect/Executable.javaFri Dec
20 08:59:52 2013 -0800
+++ b/src/share/classes/java/lang/reflect/Executable.javaFri Dec
20 10:26:17 2013 -0800
@@ -240,7 +240,6 @@
* declared or implicitly declared or neither) for the executable
* represented by this object.
*
- * @since 1.8
* @return The number of formal parameters for the executable this
* object represents
*/
@@ -291,7 +290,6 @@
* have unique names, or names that are legal identifiers in the
* Java programming language (JLS 3.8).
*
- * @since 1.8
* @throws MalformedParametersException if the class file contains
* a MethodParameters attribute that is improperly formatted.
* @return an array of {@code Parameter} objects representing all
@@ -523,7 +521,6 @@
/**
* {@inheritDoc}
* @throws NullPointerException {@inheritDoc}
- * @since 1.8
*/
@Override
public T extends Annotation T[] getAnnotationsByType(ClassT
annotationClass) {
@@ -566,8 +563,6 @@
*
* @return an object representing the return type of the method
* or constructor represented by this {@code Executable}
- *
- * @since 1.8
*/
public abstract AnnotatedType getAnnotatedReturnType();
@@ -576,8 +571,6 @@
* Returns an AnnotatedType object that represents the use of a
type to
* specify the return type of the method/constructor represented
by this
* Executable.
- *
- * @since 1.8
*/
AnnotatedType getAnnotatedReturnType0(Type returnType) {
return
TypeAnnotationParser.buildAnnotatedType(getTypeAnnotationBytes0(),
@@ -607,8 +600,6 @@
*
* @return an object representing the receiver type of the method or
* constructor represented by this {@code Executable}
- *
- * @since 1.8
*/
public AnnotatedType getAnnotatedReceiverType() {
if (Modifier.isStatic(this.getModifiers()))
@@ -635,8 +626,6 @@
* @return an array of objects representing the types of the
* formal parameters of the method or constructor represented by
this
* {@code Executable}
- *
- * @since 1.8
*/
public AnnotatedType[] getAnnotatedParameterTypes() {
return
TypeAnnotationParser.buildAnnotatedTypes(getTypeAnnotationBytes0(),
@@ -661,8 +650,6 @@
* @return an array of objects representing the declared
* exceptions of the method or constructor represented by this
{@code
* Executable}
- *
- * @since 1.8
*/
public AnnotatedType[] getAnnotatedExceptionTypes() {
return
TypeAnnotationParser.buildAnnotatedTypes(getTypeAnnotationBytes0(),
diff -r
Re: JDK 9 RFR of JDK-8030785: Missing since 1.8 javadoc for java.lang.reflect.Method:getParameterCount
On 12/20/2013 01:21 PM, Mandy Chung wrote:
--- a/src/share/classes/java/lang/reflect/Method.javaFri Dec 20
08:59:52 2013 -0800
+++ b/src/share/classes/java/lang/reflect/Method.javaFri Dec 20
10:26:17 2013 -0800
@@ -251,7 +251,8 @@
}
/**
- * {@inheritDoc}
+ * {@inheritDoc}1
Is this accident? Other than this, look fine.
Oops; yes, that is a typo. I'll correct it before I push.
Thanks for the close reading Mandy,
-Joe
RFR: jdk9: 8029997: [infra] remove Solaris ISA directories and the links
Hello, Please review the removal of ISA (Instruction Specific Architecture) directories namely sparcv9, amd64 and the symlinks in these directories, this was provided to aid transition to jdk8, where solaris 32-bit was removed, and the 32-bit binaries were replaced with 64-bit versions. http://cr.openjdk.java.net/~ksrini/8029997/webrev.0/ Thanks Kumar
Re: RFR: jdk9: 8029997: [infra] remove Solaris ISA directories and the links
Hi Kumar: Please review the removal of ISA (Instruction Specific Architecture) directories namely sparcv9, amd64 and the symlinks in these directories, this was provided to aid transition to jdk8, where solaris 32-bit was removed, and the 32-bit binaries were replaced with 64-bit versions. http://cr.openjdk.java.net/~ksrini/8029997/webrev.0/ Three cheers for code deletion! Looks good to me. Tim
Re: JDK 9 RFR of JDK-8030785: Missing since 1.8 javadoc for java.lang.reflect.Method:getParameterCount
On 12/20/13 2:03 PM, Joe Darcy wrote:
On 12/20/2013 01:21 PM, Mandy Chung wrote:
--- a/src/share/classes/java/lang/reflect/Method.javaFri Dec 20 08:59:52
2013 -0800
+++ b/src/share/classes/java/lang/reflect/Method.javaFri Dec 20 10:26:17
2013 -0800
@@ -251,7 +251,8 @@
}
/**
- * {@inheritDoc}
+ * {@inheritDoc}1
Is this accident? Other than this, look fine.
Oops; yes, that is a typo. I'll correct it before I push.
Thanks for the close reading Mandy,
Although Mike had said +1 I think he really meant -1.
Re: RFR for JDK-7168267: TEST_BUG: Cleanup of rmi regression tests (activation and others)
Hi Tristan, Thanks for cleaning this up. I've gone ahead and pushed the revised changeset. http://hg.openjdk.java.net/jdk9/dev/jdk/rev/eaa533e9778a I did take the liberty of making a couple changes... first, I had to fix some whitespace issues (trailing whitespace on a line) that were caught by jcheck. Second, I changed this line from ReadTimeoutTest.java: throw new Error(Unexpected error happen in reader: + ie); to be this instead: throw new Error(Unexpected interrupt, ie); I had missed this in my earlier review. The point here is to use the exception chaining mechanism, so that when the stack trace for the Error is printed, it includes a caused by segment with the stack trace from the original exception that had been caught. This provides better diagnostic information. The general rule is that any catch-and-rethrow should chain up the exception cause this way. The only exception (heh) to this rule is if including the cause would leak sensitive information to the caller. This situation basically never occurs in test code, though. s'marks On 12/19/13 8:08 PM, Tristan Yan wrote: Thanks Stuart I changed ReadTimeoutTest.java only apply CountdownLatch part. Please review. http://cr.openjdk.java.net/~tyan/JDK-7168267/webrev.02/ Thank you Tristan On 12/20/2013 10:47 AM, Stuart Marks wrote: Hi Tristan, Changes mostly look good. There is an ignored InterruptedException in both versions of UseCustomSocketFactory.java, but that was there already; it's not clear what should be done in this case anyway. This is something to keep in mind for a future cleanup. Hm, some duplicate code here as well, again something to think about for the future. There is a serious problem with the change to ReadTimeoutTest.java, however. The change removes the (old) line 72, which is TestIface stub = impl.export(); probably because there was an IDE warning that the variable stub is unused. This much is true, but it's essential for impl.export() to be called, because that exports the object, which creates a socket using the socket factory, which eventually results in the fac.whichPort() call below returning the port that was open. In the absence of the export() call, whichPort() returns zero which causes the test to abort immediately. In addition, the refactoring to use try-with-resources changes the order of execution of certain code, and it changes the scope of things handled by the finally-block. One purpose of the finally-block is to unexport the remote object so it makes sense to begin the try-block immediately following the export. The original code did this (well, only after a benign local variable declaration). The change moves the try-block few lines down, which means there is a larger window of time within which the finally-block won't be executed. This isn't obviously a problem, but it's a change nonetheless. Also, the change alters the order of opening the client socket and the connecting to listening port message, so the message comes after the port is opened, instead of before. Again, an apparently small change, but if there's an exception opening the port, the port number being opened won't be printed out. The main point of the changes to this file, however, is good, which is to replace the unsafe use of multi-thread access to a boolean array and polling of that value, with a CountDownLatch. So that part of the change should go in. The problem is the apparently innocuous code cleanups (use of try-with-resources, removal of apparently unused local variable) which cause the test to break or otherwise change its behavior. I could go ahead and push this changeset, omitting the changes to ReadTimeoutTest.java. Or, you could update the changeset to revert all of the changes to ReadTimeoutTest.java except for those necessary to implement the use of CountDownLatch. Either way is fine with me. Which would you prefer? s'marks On 12/18/13 6:51 AM, Tristan Yan wrote: Hi Everyone Please review the code fix for bug JDK-7168267 http://cr.openjdk.java.net/~tyan/JDK-7168267/webrev.01/ http://cr.openjdk.java.net/%7Etyan/JDK-7168267/webrev.01/ This is a cleanup for RMI tests. trying to use real timeout to replace a fixed number of loop. Thank you Tristan On 12/12/2013 05:33 AM, Stuart Marks wrote: On 12/10/13 6:10 PM, Tristan Yan wrote: /Hi everyone I am working on bug JDK-7168267 Correct link is https://bugs.openjdk.java.net/browse/JDK-7168267 Root Cause: - Per Stuart's comment, this is a clean up bug. Suggested Fix: - Will use timeout to replace loop. We should probably look at specific cases for this. There are places where the test is waiting for some external service to become ready (e.g., rmiregistry). There's no notification for things like this so wait-with-timeout cannot be used. Pretty much the only thing that can be done is to poll reasonably often until the service is ready, or until the timeout is exceeded. - Also
Re: RFR for JDK-8030284 TEST_BUG: intermittent StackOverflow in RMI bench/serial test
On 12/19/13 8:29 PM, David Holmes wrote: If you were always one frame from the end then it is not so surprising that a simple change pushes you past the limit :) Try running the shell test with additional recursive loads and see when it fails. David doesn't seem surprised, but I guess I still am. :-) Tristan, do you think you could do some investigation here, regarding the shell script based test's stack consumption? Run the shell-based test with some different values for -Xss and see how low you have to set it before it generates a stack overflow. It's also kind of strange that in the two stack traces I've seen (I think I managed to capture only one in the bug report though) the StackOverflowError occurs on loading exactly the 50th class. Since we're observing intermittent behavior (happens sometimes but not others) the stack size is apparently variable. Since it's variable I'd expect to see it failing at different times, possibly the 49th or 48th recursive classload, not just the 50th. And in such circumstances, do we know what the default stack size is? Classloading consumes a reasonable chunk of stack so if the variance elsewhere is quite small it is not that surprising that the test always fails on the 50th class. I would not expect run-to-run stack usage variance to be high unless there is some random component to the test. Hm. There should be no variance in stack usage coming from the test itself. I believe the test does the same thing every time. The thing I'm concerned about is whether the Java-based test is doing something different from the shell-based test, because of the execution environment (jtreg or other). We may end up simply raising the stack limit anyway, but I still find it hard to believe that the shell-based test was consistently just a few frames shy of a stack overflow. The failure is intermittent; we've seen it twice in JPRT (our internal buildtest system). Possible sources of the intermittency are from the different machines on which the test executes. So environmental factors could be at play. How does the JVM determine the default stack size? Could different test runs on different machines be running with different stack sizes? Another source of variance is the JIT. I believe JIT-compiled code consumes stack differently from interpreted code. At least, I've seen differences in stack usage between -Xint and -Xcomp runs, and in the absence of these options (which means -Xmixed, I guess) the results sometimes vary unpredictably. I guess this might have to do with when the JIT compiler decides to kick in. This test does perform a bunch of iterations, so JIT compilation could be a factor. I don't know if you were able to reproduce this issue. If you were, it would be good to understand in more detail exactly what's going on. FWIW there was a recent change in 7u to bump up the number of stack shadow pages in hotspot as suddenly StackOverflow tests were crashing instead of triggering StackOverflowError. So something started using more stack in a way the caused there to not be enough space to process a stackoverflow properly. Finding the exact cause can be somewhat tedious. This seems like a different problem. We're seeing actual StackOverflowErrors, not crashes. Good to look out for this, though. s'marks Cheers, David s'marks
Re: RFR for JDK-8030057: speed up forceLogSnapshot and checkAnnotations
Hi Tristan, First of all, these are two completely separate changes. They're sort-of related in that they involve replacing sleep() loops and polling with different constructs, but other than that, they're completely independent. As you'll see from my review comments, there are different issues going on with each as well. It might be worthwhile to consider splitting these into separate subtasks, one for each of these changes, so that if one of the changes converges more quickly (seems like) it can go in independently of the others. CheckAnnotations This is a pretty complicated test to begin with, and the changes make it even more complicated. It adds two threads, a single Lock, two conditions, and stream subclasses that notify these conditions. It's not clear to me why the low-level constructs from j.u.concurrent.locks are necessary. At first glance the idea of having a notifying specialization of a ByteArrayOutputStream is reasonable. I'd expect the lock and condition to be inside the stream object, though, not external to it. In addition, the condition being notified is simply something has been written. We don't actually know that all the output from the subprocess has been read. The old version waited around for a while and assumed that it had collected it all. This of course is racy; but having the notifying stream is still racy, if it happens that the output is written in multiple chunks. Note that if we want to get subprocess output as it arrives, there are two threads hidden inside of RMI's TestLibrary.JavaVM class that collect stdout and stderr from the subprocess. By default they simply write to whatever stream they are given. An alternative approach would be to plumb an interface to these threads and have the output sent directly to the test as it comes in. (I think one of the other RMI tests does this, though it does it in a particularly clumsy way that I wouldn't recommend emulating.) But stepping back a bit, the original test is over-complicated to begin with, and I'm reluctant to add all these additional mechanisms in this proposed change, because they add more complexity, and also because I can't quite convince myself that they're correct or that they even solve the problem that exists in the original test. The original test probably needs to be reconceived entirely. The idea is to activate several objects in several different activation groups (which are child JVMs of the RMID process), and to make sure that the output from each is annotated with a prefix that indicates which activation group it came from. It seems to me that a much simpler way to approach the problem is to activate several objects and have them each emit a unique bit of output. Then, shut everything down, collect ALL the output, and check to make sure that each object's unique output is on a line that was annotated properly. This completely avoids threads, locks, conditions, timing loops, and race conditions. If you think this alternative approach would be an effective way to proceed, I'd be happy to help you out with it. ForceLogSnapshot This is another fairly complicated test that is partially helped by the addition of CountDownLatches. Basically, it registers 50 activatable+restartable objects and 50 activatable (but not restartable) objects with the activation system. Then it restarts the activation system. The test wants to ensure that the restartable objects are all restarted, but that *none* of the non-restartable objects are restarted. In the first case, having a CountDownLatch that starts at 50 and awaits for them all to be restarted makes perfect sense. The await() call has a timeout to make sure we don't wait excessively long for all 50 to be restarted, but the CountDownLatch allows the test to continue immediately as soon as all 50 have been restarted. For this case, it's also not necessary to have a retry loop in waitAllStarted(). We just wait once until the latch is decremented to zero, or until we time out. It might make sense to raise the timeout here, since there's a lot of activity; 30 or even 60 seconds might be reasonable. In the normal case the restarts will all occur quickly, so we'll only wait the full timeout if there's a failure. In the second case, we expect zero objects to be restarted, so having a CountDownLatch starting at 50 doesn't make sense. There is no notion of having the test proceed immediately as soon as all of some events have happened. One choice here is to wait around for a while and count how many of the non-restartable objects actually get restarted. An AtomicInteger would be sufficient for that. An alternative might be to have a CountDownLatch initialized to 1. If any one of the non-restartable objects is restarted, we know immediately that the test has failed and we can report that. We might not care if additional non-restartable objects are restarted after we've observed the first one having been
Re: RFR for JDK-6772009 Intermittent test failure: java/util/concurrent/locks/ReentrantLock/CancelledLockLoops.java test failed with 'Completed != 2'
On 21/12/2013 4:19 AM, srikalyan wrote: Hi David, i retained only the changes to ITERS, ProbleMList.txt and upstream changes by Doug Lea(as pointed by Martin), could you please review the new change available here http://cr.openjdk.java.net/~srikchan/Regression/6772009-CancelledLockLoop-webrev/ . Ok. Thanks, David -- Thanks kalyan Ph: (408)-585-8040 On 12/19/13, 8:10 PM, David Holmes wrote: Sorry Kalyan but I don't see the need for all the incidental changes if the primary change is to just increase the iterations. I also don't see why you need to do anything for BrokenBarrierException as it is not expected to happen and the test should just fail if it does. David On 10/12/2013 6:15 AM, srikalyan wrote: Hi David/Martin a gentle reminder for review. -- Thanks kalyan Ph: (408)-585-8040 On 12/2/13, 11:21 AM, srikalyan wrote: Hi David, Thanks for the review, the new webrev is hosted at http://cr.openjdk.java.net/~cl/host_for_kal/6772009-CancelledLockLoop/ . Please see inline text. On 11/20/13, 6:35 PM, David Holmes wrote: On 21/11/2013 10:28 AM, Martin Buchholz wrote: I again tried and failed to reproduce a failure. Even if I go whole hog and multiply TIMEOUT by 100 and divide ITERS by 100, the test continues to PASS. Is it just me?! I think you are going the wrong way Martin - you want the timeout to be smaller than the time it takes to execute ITERS. I don't think there's any reason to make result long. It's not even used except to inhibit hotspot optimizations. +private volatile long result = 17;//Can get int overflow,so using long Further the subsequent use of += is incorrect as it is not an atomic operation. Even if we don't care about the value, it looks bad. Made the necessary changes for atomic update. I'm not sure result must be updated if we get a BrokenBarrierException either. Probably harmless, but necessary? I retained it in the fix for completeness in updating the numbers, please let me know if you still think otherwise. need to fix spelling and spacing below. +barrier.await();//If a BrokeBarrierException happens here(due to There are a number of style issues with spacing around comments. Fixed the spelling error and styling issues. And I don't think this change is sufficient to claim co-author status with Doug either ;-) Removed the claim :) The additional tracing may be useful but seems stylistically different from the rest of the code. Retained the tracking to understand if it is again the timing issue which is the cause in an event of a failure, however i can remove it if you think it is not necessary (OR) include an alternate solution as you may want to suggest. Overall I'm suspicious that the changed timeout actually fixes anything - normally we need to add longer timeouts not shorter ones. Does this fail on a range of machines or only specific ones? Have we verified that the clocks/timers are behaving properly on those systems? Here the time out is not about waiting for threads to complete something but to be interrupted before being considered done, so we decreased the timeout. However we now chose to increase the number of iterations to 500 from 100(thanks to tristan for the suggestion) instead of decreasing the timeout as done earlier because the increasing iterations ensures the threads are busy for long time curtailing the need to touch the timeout. Thanks, David -- Thanks kalyan Ph: (408)-585-8040 On Wed, Nov 20, 2013 at 11:52 AM, srikalyan srikalyan.chandrashe...@oracle.com wrote: Hi Martin , apologies for the delay , was trying to get help for hosting my webrev. . Please see inline text. On 11/19/13, 10:35 PM, Martin Buchholz wrote: Hi Kalyan, None of us can review your changes yet because you haven't given us a URL of your webrev. It is located here http://cr.openjdk.java.net/~cl/host_for_srikalyan_6772009_CancelledLockLoops/ I've tried to make the jsr166 copy of CancelledLockLoops fail by adjusting ITERS and TIMEOUT radically up and down, but the test just keeps on passing for me. Hints appreciated. Bump up the timeout to 500ms and you will see a failure (i can see it consistently on my machine Linux 64bit,8GBRAM,dual cores, with JDK 1.8 latest any promoted build). On Tue, Nov 19, 2013 at 6:39 PM, srikalyan chandrashekar srikalyan.chandrashe...@oracle.com wrote: Suggested Fix: a) Decrease the timeout from 100 to 50ms which will ensure that the test will pass even on faster machines This doesn't look like a permanent fix - it just makes the failing case rarer. Thats true , the other way is to make the main thread wait on TIMEOUT after firing the interrupts instead of other way round, but that would be over-optimization which probably is not desirable as well. The 50 ms was arrived at empirically after running several 100 times on multiple configurations and did not cause failures. -- Thanks kalyan Ph: (408)-585-8040
