Re: Making java.util.Iterator.remove() for the iterators for EnumSet more resilient

Hi Neil; I think your message presents the perfect "Speak now or forever hold your peace" opportunity. Barring any new substantive objections (I think you have addressed the previous objections more than adequately) I believe we should commit your patch to EnumSet and I will do so at the end of

Review Request -- 5045147 : When TreeMap is empty explicitly check for null keys in put()

This is a review request for an issue which was previously committed in 2006 but was quickly withdrawn because it was believed to cause a regression in other software. That removal was mistaken and this fix appears to be bona-fide beneficial. http://cr.openjdk.java.net/~mduigou/5045147/0/webrev

Review request : 4884238 : Constants for standard charsets

http://cr.openjdk.java.net/~mduigou/4884238/0/webrev/ This webrev proposes a class java.nio.charset.Charsets to contain definitions for the (currently) six standard character sets and a seventh definition for the platform default character set. There are also minor amendments to the existing ja

Re: Request for review: 6312706: Map entrySet iterators should return different entries on each call to next()

the impact of this change. If you have any metrics that you can contribute which show this change to have negligible impact upon performance across common benchmarks it would definitely help to allay any fears. Thanks, Mike On Mar 25 2011, at 11:37 , Mike Duigou wrote: > I've

Re: Request for review: 6312706: Map entrySet iterators should return different entries on each call to next()

I think your suspicion is correct that the anonymous inner Object is used rather than new Object so that it can be distinguished more easily in heap dumps or via other tools. I would be fine with using Integer(0) as the sentinel value. I've always used new String("something useful") rather than

hg: jdk7/tl/jdk: 6565585: Remove critical section in Method.invoke, Constructor.newInstance, Field.getFieldAccessor improving performance

Changeset: 9f08a221e5f2 Author:mduigou Date: 2011-04-04 11:55 -0700 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/9f08a221e5f2 6565585: Remove critical section in Method.invoke, Constructor.newInstance, Field.getFieldAccessor improving performance Reviewed-by: alanb, dholmes, bri

Re: Review (Updated) : 4884238 : Constants for Standard Charsets

Oops, missed that one in my search-and-replace. I've updated the online webrev. Thanks! Mike On Apr 18 2011, at 16:13 , David Schlosnagle wrote: > On Mon, Apr 18, 2011 at 6:24 PM, Mike Duigou wrote: >> The latest webrev : http://cr.openjdk.java.net/~mduigou/4884238/2/webr

hg: jdk7/tl/jdk: 2 new changesets

Changeset: 5b05f8d1c0e5 Author:mduigou Date: 2011-04-26 14:25 -0700 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/5b05f8d1c0e5 4884238: Adds java.nio.charset.StandardCharset to provide static final constants for the standard charsets. Reviewed-by: alanb, sherman, darcy ! src/sha

hg: jdk7/tl/jdk: 2 new changesets

Changeset: 37722a0a1c65 Author:mduigou Date: 2011-04-28 10:12 -0700 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/37722a0a1c65 7040381: Add StandardCharset.java to FILES_java.gmk Reviewed-by: alanb ! make/java/nio/FILES_java.gmk Changeset: 7b7c1ffd0752 Author:mduigou Date:

Re: hg: jdk7/tl/jdk: 2 new changesets

Essentially we ran out of time for this task and were confident enough in out decision about the design and choice of names to go ahead. More discussion didn't seem to be leading anywhere but inaction and there are other issues (always) that need attention. Mike On Apr 28 2011, at 10:15 , Ulf

hg: jdk7/tl/jdk: 7040572: Fix broken java/nio/charset/StandardCharset/Standard.java and add more tests.

Changeset: 36dd30b5f85d Author:mduigou Date: 2011-04-29 14:09 -0700 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/36dd30b5f85d 7040572: Fix broken java/nio/charset/StandardCharset/Standard.java and add more tests. Reviewed-by: alanb ! test/java/nio/charset/StandardCharset/Standa

Review Request : 7041612 : Rename StandardCharset to StandardCharsets

Hello All; Per the most recent message from Mark Reinhold (http://mail.openjdk.java.net/pipermail/core-libs-dev/2011-May/006827.html) the recently added StandardCharset class will be renamed to StandardCharsets to better reflect that it is not an enum. We would like to get this change in ASAP.

hg: jdk7/tl/jdk: 2 new changesets

Changeset: 7c9780ea0c5a Author:mduigou Date: 2011-05-03 16:32 -0700 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/7c9780ea0c5a 7041612: Rename StandardCharset to StandardCharsets Reviewed-by: alanb, mr, darcy ! make/java/nio/FILES_java.gmk ! src/share/classes/java/nio/charset/Cha

hg: jdk7/tl/jdk: 7043104: disable test java/lang/invoke/InvokeDynamicPrintArgs.java

Changeset: bd8c10d1db87 Author:mduigou Date: 2011-05-09 09:13 -0700 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/bd8c10d1db87 7043104: disable test java/lang/invoke/InvokeDynamicPrintArgs.java Reviewed-by: alanb ! test/ProblemList.txt

hg: jdk7/tl/jdk: 2 new changesets

Changeset: e941ff30d005 Author:mduigou Date: 2011-05-10 10:16 -0700 URL: http://hg.openjdk.java.net/jdk7/tl/jdk/rev/e941ff30d005 7043513: Update test for StandardCharsets Reviewed-by: alanb - test/java/nio/charset/StandardCharset/Standard.java + test/java/nio/charset/StandardCharse

Re: Code review request for 7049774

This change looks good to me! Mike On Jun 9 2011, at 08:26 , Seán Coffey wrote: > 7049774 : UID construction appears to hang if time changed backwards > > I'm looking for code review of above reported issue. If system clock goes > backwards on rmi server with active clients and 64k UID boundary

Re: JDK 8 code review request for 7055295: (reflect) add conventional constructor to GenericSignatureFormatError

Perhaps the chained exception constructor would also be useful. I can imagine that whatever was parsing the signature might throw an exception with it's parse state that could then be captured (without needing to use initCause()) in the thrown GenericSignatureFormatError Mike On Jun 15 2011,

Re: Code Review 7051516: ThreadLocalRandom seed is never initialized so all instances generate the same sequence

Hi Chris; The getClass() seems unnecessary. Why not : public Random(long seed) { this.seed = new AtomicLong(); setSeed(seed); } or private final AtomicLong seed = new AtomicLong(); public Random(long seed) { setSeed(seed); } Both of these would seem to

Re: JDK 8 code review request for 7055295: (reflect) add conventional constructor to GenericSignatureFormatError

onstructors. It has usually been an exception class outside the JDK but it seems like a reasonable practice to include all four. Mike On Jun 16 2011, at 09:35 , Joe Darcy wrote: > On 6/16/2011 9:21 AM, Mike Duigou wrote: >> Perhaps the chained exception constructor would also be u

Re: AbstractCollection.removeAll(Collection) and AbstractSet.removeAll(Collection)

It looks to me that AbstractCollection could benefit from a c.isEmpty() check but that's the only optimization I currently see. It's necessary to iterate the target collection because values can be repeated for some collection types. I agree that AbstractSet.removeAll appears to make a poor judg

Re: AbstractCollection.removeAll(Collection) and AbstractSet.removeAll(Collection)

On Jul 13 2011, at 15:19 , Jason Mehrens wrote: > worst case AbstractCollection.isEmpty could be O(N). > JDKs 5 and 6 shipped with two collections (CHM views) that had O(N) isEmpty > methods. This is very interesting. The common wisdom has been that size() should be avoided because of O(>1)

Re: JDK 8 code review request for 7062430 "Minor iconsistency in ulp descriptions"

Perhaps a linkplain from the method javadoc to an anchor in the Math class javadoc could be added so that an explanation for the added definition is available. Mike On Jul 14 2011, at 18:14 , Joe Darcy wrote: > Hello. > > Please review this small doc change to spell out what "ulp" means in th

Re: Launcher fix: 7067922, please review

Looks good. There's no explicit handling for the trimmed string being empty but it doesn't appear that anything bad happens. The ClassNotFoundException resulting from an empty string might be confusing. Mike On Jul 18 2011, at 14:48 , Kumar Srinivasan wrote: > Hi, > > Please review > htt

Re: JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

This looks good to me but I agree with David that it's probably important to look at the generated javadoc and specdiff output before finalizing. Mike On Jul 18 2011, at 00:51 , Joe Darcy wrote: > Peter and David. > > Thanks for the careful review; the @throws information still needs its own

Re: JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

19 2011, at 12:49 , Joe Darcy wrote: > Agreed; I've posted a BlenderRev corresponding to the current patch at: > > http://cr.openjdk.java.net/~darcy/7007535.4/BR-7007535.html > > Thanks, > > -Joe > > Mike Duigou wrote: >> This looks good to me but I agree

Re: Contribution to Bug 100120

One other valuable bit of valuable info missing from the current bug writeup is an explanation of why the RFE (request for enhancement) matters. Where is the problem encountered, how widespread is the problem, how much of a bottleneck does it represent, etc.? Any context you can provide can only

Re: ArrayList.subList

I had a similar issue with Set recently. The problem is that structural changes in sub-sub-lists aren't correctly reflected in the sub-list parents if the sub-sub-lists are disconnected from the parent. The chain back to to the parent through all intermediate sub-lists is used to update list bou

7073296: Review : Bug in Executable.equalParamTypes() results in incorrect comparison for differing number of params

Hello All; A fairly simple bug to review which snuck through testing. http://cr.openjdk.java.net/~mduigou/7073296/0/webrev/ The changes to Class are incidental but so trivial that opted to include them. I can remove if anyone feels strongly (or even weakly) that they should be excluded. Mike

Re: 7073296: Review : Bug in Executable.equalParamTypes() results in incorrect comparison for differing number of params

an.getBoolean(). > Given the default semantics of jtreg, you could omit the @compile and @run > lines in the test. Will do. > Cheers, > > -Joe > > On 8/2/2011 12:44 PM, Mike Duigou wrote: >> Hello All; >> >> A fairly simple bug to review which snuck through tes

hg: jdk8/tl/jdk: 7073296: Executable.equalParamTypes() incorrectly returns true when the number of params differs.

Changeset: 56e89034 Author:mduigou Date: 2011-08-04 08:53 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/56e89034 7073296: Executable.equalParamTypes() incorrectly returns true when the number of params differs. Reviewed-by: alanb, darcy ! src/share/classes/java/lan

Re: JDK 8 code review request for 7075098: Remove unused fdlibm files

Looks good to me. I did wonder if we have a policy for tracking fdlibm updates. The fdlibm people don't seem to ascribe dates to their releases or think to include version numbers in their headers. Are we using 5.3, the apparent latest version? Their release page mentions an outstanding incorr

Re: 7015589: (spec) BufferedWriter.close leaves stream open if close of underlying Writer fails

This change looks good. I don't believe a compatibility switch for the FilterOutputStream is warranted. On Aug 9 2011, at 04:43 , Alan Bateman wrote: > > A few months back there was a bug report [1] pointing out that it's possible > to continue writing to a BufferedWriter after invoking its cl

Re: Any chance to see EnumSet implement SortedSet in JDK8?

There's an existing RFE for this request: CR#6278287 http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6278287 Wherein Kevin Bourrillion provides some excellent analysis. The RFE hasn't been targeted to Java 8 and I'm unaware of any plan to escalate the priority. It would be a very good issue

Re: Request for review for 7066490: @since 1.7 tag is missing for java.util.regex.Matcher.group(java.lang.String)

Looks good. This should be backported to 7 updates. Mike On Aug 17 2011, at 14:37 , Xueming Shen wrote: > The @since 1.7 tag was missed when regex groupname suport was added in JDK7. > Here is the webrev for adding it back in. > > http://cr.openjdk.java.net/~sherman/7066490/webrev >

Re: Request for Review: Chain more Exceptions (RuntimeException)

I will echo Joe's comments about putting the "|" at the end of the line rather than at the beginning of the next line. Not a comment on your patch but some of the use of raw RuntimeException is rather weak. I saw a few cases where IllegalStateException or UnsupportedOperationException would be

Re: JDK 8 code review request for 7082231 "Put a @since 1.7 on System.lineSeparator"

looks good. On Aug 23 2011, at 15:43 , Joe Darcy wrote: > Hello. > > Please review this simple patch to fix 7082231 "Put a @since 1.7 on > System.lineSeparator", which will correct an @since tag omitted by the JDK 7 > fix for 6900043 "Add method to return line.separator property." > > diff --

Re: Enhance toString() to return structured info, for certificate and probably more

On Aug 25 2011, at 14:01 , Dr Andrew John Hughes wrote: > On 10:41 Thu 25 Aug , Weijun Wang wrote: >> Hi All >> >> I was talking with Xuelei on how to better display certificate info. >> There are 3 cases we can currently think of: >> >> 1. debug output >> 2. keytool/jarsigner output >> 3.

Re: JDK 8 code review request for 6838776 Defer initialization of static fields in java.math.BigInteger

Looks good. Nice little performance tweak. You may want to use ExceptionInInitializerError rather than plain Error. Mike On Sep 2 2011, at 17:44 , joe.da...@oracle.com wrote: > Hello. > > Please review this change to only instantiated an Unsafe object in BigInteger > and BigDecimal if seriali

Re: JDK 8 code review request for 7088500 "there is no @since tag on SafeVarargs"

Looks good! On Sep 14 2011, at 12:38 , Joe Darcy wrote: > Hello. > > Please review the simple patch below to add an omitted @since tag to the > SafeVargs annotation type, which was added to the platform as part of Project > Coin in JDK 7. > > Thanks, > > -Joe > > --- a/src/share/classes/jav

Re: JDK 8 code review request for 7091682 "Move sun.misc.FpUtils code into java.lang.Math"

I am curious why some of the tests still refer to FpUtils. For example: Log10Tests.java: 156 up = Math.nextUp(1.0); 157 down = FpUtils.nextDown(1.0); Would it be possible to further reduce or eliminate the references to FpUtils from the tests? Mike O

hg: jdk8/tl/jdk: 7074264: Switches to packages tree view and adds unit tests to sources

Changeset: c77b41652266 Author:mduigou Date: 2011-09-20 12:27 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c77b41652266 7074264: Switches to packages tree view and adds unit tests to sources Reviewed-by: igor ! make/netbeans/README ! make/netbeans/common/closed-share-view.

Review Request: 7088913/7088952 : Additions to primitive wrappers

Hello all; Here's a webrev for two small additions to the primitive wrapper types (Boolean, Byte, Character, Double, Float, Integer, Long, Short). http://cr.openjdk.java.net/~mduigou/7088913/0/webrev/ The webrev combines two issues: 7088913: Add compatible static hashCode(primitive) to pri

Re: JDK 8 code review request for 6268216 "Boolean.getBoolean() throws SecurityException"

Looks good. Do you think it's worth eliminating the private static method toBoolean(String) in favour of the public static parseBoolean(String) method? Mike On Sep 20 2011, at 16:46 , Joe Darcy wrote: > Hello. > > Please review this simple fix to add some informative text detailing when an

Re: JDK 8 code review request for 6268216 "Boolean.getBoolean() throws SecurityException"

Changes look good. On Sep 20 2011, at 17:54 , Joe Darcy wrote: > Mike Duigou wrote: >> Looks good. >> >> Do you think it's worth eliminating the private static method >> toBoolean(String) in favour of the public static parseBoolean(String) method? >>

Re: Review Request: 7088913/7088952 : Additions to primitive wrappers

, Ulf Zibis wrote: > Hi, > > why don't we have Boolean.SIZE and Boolean.BYTES ? > > -Ulf > > > Am 20.09.2011 22:11, schrieb Mike Duigou: >> Hello all; >> >> Here's a webrev for two small additions to the primitive wrapper types >>

Re: Review Request: 7088913/7088952 : Additions to primitive wrappers

On Sep 22 2011, at 14:34 , David Holmes wrote: > On 22/09/2011 11:23 PM, Ulf Zibis wrote: >> Am 22.09.2011 12:04, schrieb David Holmes: >>> On 22/09/2011 7:40 PM, Ulf Zibis wrote: Yep, not easy to make a right decision here. One if the questions might be, if SIZE/BYTES are meant a

hg: jdk8/tl/jdk: 5029031: Add Collections.checkedQueue()

Changeset: c5c91589b126 Author:mduigou Date: 2011-10-19 14:17 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/c5c91589b126 5029031: Add Collections.checkedQueue() Reviewed-by: mduigou Contributed-by: darryl.mo...@oracle.com ! src/share/classes/java/util/Collections.java + tes

Re: Code Review 7104209: Cleanup and remove librmi (native library)

Looks good. Nice catch. Mike On Oct 24 2011, at 07:20 , Chris Hegarty wrote: > It appears that the only function of librmi is to export the VM function > JVM_LatestUserDefinedLoader. It seems really silly to have a whole native > library to perform such a trivial operation. JVM_LatestUserDefin

Re: Code Review 7104209: Cleanup and remove librmi (native library)

Looks good from me as well. Seems like a strange location to have the load library! Mike On Oct 25 2011, at 01:08 , Alan Bateman wrote: > > It turns out there's a problem with these changes as it didn't remove the > code to load the rmi library (we missed it in the code review too). The > re

hg: jdk8/tl/jdk: 4533691: Add Collections.emptySortedSet()

Changeset: 2f2f56ac8b82 Author:mduigou Date: 2011-11-03 13:26 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2f2f56ac8b82 4533691: Add Collections.emptySortedSet() Reviewed-by: mduigou, alanb, dholmes Contributed-by: darryl.mo...@oracle.com ! src/share/classes/java/util/Coll

Re: Code Review Request for 4533691 (add Collections.EMPTY_SORTED_SET)

On Nov 3 2011, at 17:40 , David Holmes wrote: > Mike, > > I see that you have pushed a version of this change and that I was listed as > a reviewer. However I never saw an updated webrev and there was no response > to my query below. In fact I never saw any response to any of the reviewers >

Re: Code Review 7107516: LinkedBlockingQueue/Deque.drainTo(Collection, int) returns 'maxElements' if its value is negative

The change looks good. The creation of node instances could use diamond. ie. Node node = new Node(e); could be : Node node = new Node<>(e); Mike On Nov 9 2011, at 06:55 , Chris Hegarty wrote: > > According to the specification for BlockingQueue.drainTo(Collection c, int > maxElements), thi

Re: Code Review Request for Bug #5035850

On Nov 20 2011, at 11:39 , Rémi Forax wrote: > On 11/20/2011 08:14 PM, Alan Bateman wrote: >> On 18/11/2011 18:27, Darryl Mocek wrote: >>> Hello. Please review this patch to fix a serialization issue with String's >>> CASE_INSENSITIVE_ORDER. If you serialize, then deserialize the class, the >

Re: Request for Review: 7116914 (Miscellaneous warnings (sun.text))

Looks good. In NormalizerImpl.java: - The parens are probably not needed around 'length=(srcIndex-prevSrc);' UnicodeSet.java: - I confirmed, just to be sure, that the added 'break;' statements have no effect. On Dec 1 2011, at 23:51 , Yuka Kamiya wrote: > Hello, > > Could someone please revi

Re: Collections.enumeration() WAS Warning Cleanup Day

a.util.PropertyPermission >>> because java.util.Collections.enumeration is wrongly typed. >>> >> Rémi - you might want to sync up with Mike Duigou as I believe he has been >> working on the java.util area including collections. Also best to move any >> discussions on collections/util area

Re: java.util patch for Warning Cleanup Day

l >>> and I'm not able to cleanly retrofit java.util.PropertyPermission >>> because java.util.Collections.enumeration is wrongly typed. >>> >> Rémi - you might want to sync up with Mike Duigou as I believe he has been >> working on the java.util area inc

Re: RFR 7118066: Warnings in java.util.concurrent package

Was the SnychronousQueue test recently added or did something else change to cause the failure? Mike On Dec 8 2011, at 03:33 , Chris Hegarty wrote: > On 08/12/2011 05:13, David Holmes wrote: >> >> >> I also think the SynchronousQueue has no business being used in this >> test. Though I co

Re: A behavior mismatch in AbstractCollection.toArray(T[] )

I agree that most people probably won't check whether the array returned is the array they provided. It is possible that they incorrectly assume it is. ie. Collection collection; ... Integer[] foo = new Integer[collection.size()]; // of sufficient size for all elements // at this point collectio

Re: Code Review Request for Bug #4802647

This looks complete to me. Mike On Dec 19 2011, at 10:53 , Brandon Passanisi wrote: > Hello core-libs. I was wondering if somebody could please review the > following updated webrev for this bug. There was an e-mail thread regarding > the first webrev [1] that prompted the changes in this we

Re: Request for review: 7123229: (coll) EnumMap.containsValue(null) returns true

This looks correct. I appreciate the toString() method with a unique result. Mike On Jan 9 2012, at 17:05 , Neil Richards wrote: > Hi all, > When proposing the change for 6312706 [1], I erroneously managed to > convince myself (and others!) that it would be safe to use 'new > Integer(0)' for ja

Re: Code review request for javadoc problem in java.lang.invoke.MethodHandle

+1 On Jan 9 2012, at 18:17 , Joe Darcy wrote: > Hello, > > Please review the simple patch below to fix a javadoc problem in > java.lang.invoke.MethodHandle which stems from the arguments to the {@link} > javadoc tag being reversed. > > Once reviewed, I'll file a bug for this and push the fix.

Re: Code review request for trivial javadoc issue in Throwable

Looks good. On Jan 10 2012, at 17:20 , Joe Darcy wrote: > Hello, > > Please review this simple fix to the javadoc of java.lang.Throwable; use of > "<" and ">" rather than "<" and ">" currently causes malformed HTML to > be generated. > > --- a/src/share/classes/java/lang/Throwable.javaTue

Re: JDK 8 code review request for initial unsigned integer arithmetic library support

Really cool stuff Joe. One initial note: parseUnsigned*() returns a signed value that may not be able to hold the entire result. I would rather see a larger size be returned to be sure that subsequent operations don't have to be special cased for > MAX_VALUE. At minimum some documentation des

Re: java.io.File field "path" is not declared final

Throwing plain Error could probably be improved to InternalError. (Though it will probably never be thrown). Mike On Feb 17 2012, at 04:00 , Rémi Forax wrote: > On 02/17/2012 12:36 PM, Rémi Forax wrote: >> On 02/16/2012 11:37 AM, Alan Bateman wrote: >>> On 15/02/2012 17:34, Rémi Forax wrote: >>

Re: RFR : 7144488 StackOverflowError occurres on list via Collections.synchronizedList(List)

The revised version looks good to me. It's worth getting all four. On Feb 23 2012, at 06:46 , Seán Coffey wrote: > ok, > > I've updated the changes (and bug synopsis) based on comments. > equals method tweaked for : > > Collections.SynchronizedList > Collections.SynchronizedSet > Collections.Sy

hg: jdk8/tl/jdk: 7143162: Allow disable building of jdk demos and samples

Changeset: 323abe0e8973 Author:mduigou Date: 2012-02-27 18:10 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/323abe0e8973 7143162: Allow disable building of jdk demos and samples Reviewed-by: ohair ! make/Makefile ! make/common/Release.gmk ! make/common/shared/Sanity-Setting

hg: jdk8/tl: 7143162: Allow disable building of jdk demos and samples

Changeset: 28f2fe471725 Author:mduigou Date: 2012-02-27 18:09 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/rev/28f2fe471725 7143162: Allow disable building of jdk demos and samples Reviewed-by: ohair ! make/sanity-rules.gmk

RFR 7149320: Move sun.misc.VM.booted() to end of System.initializeSystemClass()

Hello all; This issue is a patch for review that I have split out of a larger issue I'll be submitting later. WEBREV @ http://cr.openjdk.java.net/~mduigou/7149320/0/webrev/ sun.misc.VM.booted() is currently called before setJavaLangAccess(). For code which uses the JavaLangAccess shared secret

Re: java.util.UUID.fromString performance

The easiest wat to proceed would be for you to submit a webrev patch with both changes along with your microbenchmark test. Once the change is incorporated in the jdk8 mainline the sustaining teams can evaluate it for backporting to jdk 7 and 6. Mike On Feb 28 2012, at 06:26 , Vitaly Davidovic

Re: JDK 8 code review request for 7007535: (reflect) Please generalize Constructor and Method

And then: underlying method -> underlying executable >> >> 241 * parameter types of the underlying method, in declaration order >> 243 * if the generic method signature does not conform to the format >> 247 * types of the underlying method

Re: RFR 7149320: Move sun.misc.VM.booted() to end of System.initializeSystemClass()

lt too risky to consider? Are there validations that could/should be done to reduce or eliminate the risk? > Cheers, > David > - > > > On 28/02/2012 1:57 PM, Mike Duigou wrote: >> Hello all; >> >> This issue is a patch for review that I have split out of a

hg: jdk8/tl/jdk: 7149320: Move sun.misc.VM.booted() to the end of System.initializeSystemClass()

Changeset: 971a86421f51 Author:mduigou Date: 2012-03-01 09:40 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/971a86421f51 7149320: Move sun.misc.VM.booted() to the end of System.initializeSystemClass() Summary: Ensure that sun.misc.VM.booted() is the last action in System.in

RFR 7065380 : Allow Collections.sort to sort Collections.singletonList() result

Hello all; Currently Collections.sort() refuses to sort the lists which result from calling Collections.singletonList(). This makes some sense because the singleton lists are immutable but they are also alway sorted. This patch allows Collections.sort() to be used with empty and singleton lists

RFR 7065380 : Allow Collections.sort to sort Collections.singletonList() result

Hello all; Currently Collections.sort() refuses to sort the lists which result from calling Collections.singletonList(). This makes some sense because the singleton lists are immutable but they are also alway sorted. This patch allows Collections.sort() to be used with empty and singleton lists

Re: RFR 7065380 : Allow Collections.sort to sort Collections.singletonList() result

I thought about that but the usual issue of non-O(1) size() methods lead me to avoid having a size() in addition to the toArray(). Not perfect. :( Mike On Mar 1 2012, at 12:26 , Rémi Forax wrote: > On 03/01/2012 08:50 PM, Mike Duigou wrote: >> Hello all; >> >> Curre

Re: RFR 7065380 : Allow Collections.sort to sort Collections.singletonList() result

; alerted to that error in their assumptions so they can correct it before it > bites them elsewhere. > > -- > Colin > 5 > > On Thu, Mar 1, 2012 at 2:40 PM, Mike Duigou wrote: > Hello all; > > Currently Collections.sort() refuses to sort the lists which result from

Re: Asking about the interesting behaviours of TreeMap.putAll

I'm in agreement with Doug on this. "The effect of this call is equivalent to that of calling put(k, v) on this map once for each mapping from key k to value v in the specified map." is not the same as "The effect of this call is to call put(k, v) on this map once for each mapping from key k to

hg: jdk8/tl/jdk: 7151595: Disable creation of db demos if NO_DEMOS is specified

Changeset: 2900d4ae2d39 Author:mduigou Date: 2012-03-08 13:44 -0800 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2900d4ae2d39 7151595: Disable creation of db demos if NO_DEMOS is specified Reviewed-by: weijun, dholmes ! make/common/Release.gmk

Re: RFR 7065380 : Allow Collections.sort to sort Collections.singletonList() result

On Mar 1 2012, at 13:35 , Jason Mehrens wrote: > What about exception cases where the single element is not comparable? > http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=5045147 > > Consider the following: > > = > Object[] a = new Object[]{new Object()}; > Arrays.sort(a); > List l

Re: Request for review : 7121314 : Behavior mismatch between AbstractCollection.toArray(T[] ) and its spec

This looks good to me. Mike On Mar 12 2012, at 23:58 , Sean Chou wrote: > Hi Ulf and David, > >I modified the patch and added the testcase, it's now : > http://cr.openjdk.java.net/~zhouyx/7121314/webrev.02/. > >Ulf's compact version is used, it looks beautiful; however I replaced >

Re: No replaceLast() method in String

I took a look through the bug database and can't find a reason (if there was one). It's likely not implemented because Matcher doesn't implement replaceLast(). Mike On Mar 20 2012, at 09:28 , Alex Kravets wrote: > Hi, > > I was looking at String API and noticed that there are methods for stri

Re: Request for review: 7156459: Remove unnecessary get() from Currency.getInstance()

Looks fine to me as well. Mike On Mar 26 2012, at 12:01 , Naoto Sato wrote: > Hi, > > Please review the following fix in java.util.Currency. This fix is to remove > an unnecessary get() introduced with the prior fix to 7145454. > > bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=71564

Re: Code Review Request: 7157893: Warnings Cleanup in java.util.*

This thread has gone on long enough I've lost track of the reasoning behind some of the choices. Specifically, I'm wondering why Entry[] is preferred over Entry[] for HashMap and Hashtable. Understandably is needed to the array creation but the technique used by WeakHashMap would seem to be eq

Re: Code Review Request: 7157893: Warnings Cleanup in java.util.*

Looks good for commit. Just a few notes - The inconsistent use of by WeakHashMap and by the other Hash Map classes for their tables is a bit annoying but not a new problem. We should either make all of the maps use (my preference) or all use . There were lots of cases of introduced and tha

Re: RFR: 7160714 - Strange or obsolete @see tags in some exception java.util javadoc

Looks good with the copyright change. Mike On Apr 18 2012, at 12:15 , Jim Gish wrote: > Description: just a trivial change to add in Iterator#next() as a @see > reference > > (Alan and I discussed the possibilities here of completeness vs brevity, and > we opted for brevity. There are actua

Review Request : 7071826 : Avoid benign race condition in initialization of UUID

Hello all; A benign but potentially expensive race condition was discovered in java.util.UUID. The initialization of numberGenerator may cause the creation of a number of SecureRandom instances if multiple threads are attempting to initialize the field at the same time. This can happen because

Re: Review Request : 7071826 : Avoid benign race condition in initialization of UUID

On May 11 2012, at 03:12 , Alan Bateman wrote: > On 11/05/2012 00:11, Mike Duigou wrote: >> >> Hello all; >> >> A benign but potentially expensive race condition was discovered in >> java.util.UUID. The initialization of numberGenerator may cause the creatio

hg: jdk8/tl/jdk: 7071826: Avoid benign race condition in initialization of UUID

Changeset: 944676ef3c58 Author:mduigou Date: 2012-05-11 11:31 -0700 URL: http://hg.openjdk.java.net/jdk8/tl/jdk/rev/944676ef3c58 7071826: Avoid benign race condition in initialization of UUID Summary: Avoids mostly benign but sometimes expensive race condition on initialization of

Review Request CR#7118743 : Alternative Hashing for String with Hash-based Maps

Dear OpenJDK CoreLibs community, A significant enhancement to the Java SE hashing-based Map implementations in planned for inclusion in Java SE 7u6. All of the hashing based Map implementations: HashMap, Hashtable, LinkedHashMap, WeakHashMap and ConcurrentHashMap will now use an enhanced hashin

Re: Review Request CR#7118743 : Alternative Hashing for String with Hash-based Maps

Hi Mike; The problem with using instanceof Hashable32 is that is much slower (often more than 25X) than instanceof String. It's slow enough that we won't reasonably consider using instanceof Hashable32 in JDK 8. We have considered making Object implement Hashable32 and add a virtual extension m

Re: Review Request CR#7118743 : Alternative Hashing for String with Hash-based Maps

he location of the resize check moved slightly. > In WeakHashMap.Entry, why hash is not declared 'final' anymore ? Mis-port from JDK 7 version which needs it to be non-final for rehashing. It can be final in the JDK 8 version. > > cheers, > Rémi > > On 05/23/

Re: Review Request CR#7118743 : Alternative Hashing for String with Hash-based Maps

> (Prominent people have said, it will never make sense to change the String's > hash algorithm.) > See: http://markmail.org/message/ig3nzmfinfuvgbwz > http://markmail.org/message/h3nlhhae5qlmf37a Yes, there other other reasons as well. Mike > > > Am 23.05.2012

Re: Review Request CR#7118743 : Alternative Hashing for String with Hash-based Maps

quot; webrev : http://cr.openjdk.java.net/~mduigou/althashing8/9/webrev/ On May 22 2012, at 22:16 , Mike Duigou wrote: > Dear OpenJDK CoreLibs community, > > A significant enhancement to the Java SE hashing-based Map implementations in > planned for inclusion in Java SE 7

Request for Review : CR#6924259: Remove String.count/String.offset

Hello all; For a long time preparations and planing have been underway to remove the offset and count fields from java.lang.String. These two fields enable multiple String instances to share the same backing character buffer. Shared character buffers were an important optimization for old bench

Re: Review Request CR#7118743 : Alternative Hashing for String with Hash-based Maps

On May 23 2012, at 16:31 , David Holmes wrote: > On 24/05/2012 2:24 AM, Mike Duigou wrote: >> Hi Mike; >> >> The problem with using instanceof Hashable32 is that is much slower (often >> more than 25X) than instanceof String. It's slow enough that we won

Re: Request for Review : CR#6924259: Remove String.count/String.offset

On May 24 2012, at 15:56 , Ulf Zibis wrote: > Am 24.05.2012 22:45, schrieb Mike Duigou: >> A patch of the changes to java.lang.String for JDK 8 are >> at<http://cr.openjdk.java.net/~mduigou/6924259/0/webrev/>. The changes for >> JDK 7 have only superficial diffe

Re: Review Request CR#7118743 : Alternative Hashing for String with Hash-based Maps

planation of which dispatching techniques have good performance and an explanation of which idioms will never be fast I'd be interested to read it. Mike > Thanks > > Sent from my phone > > On May 24, 2012 5:26 PM, "Mike Duigou" wrote: > > On May 23 2012,

Re: Request for Review : CR#6924259: Remove String.count/String.offset

> A non-material comment is that there are a couple of style changes that I > found annoying. Everyone is an expert on such matters, I'm not, but the one > that bugged me a bit was adding the space after the cast, eg: > (toffset > (long) value.length - len) > I found myself needing to re-read it

Re: Request for Review : CR#6924259: Remove String.count/String.offset

t wasn't used. I've reverted to using the 3 param version. > So this doesn't really share code > but just add one level to the depth of the call stack. > > cheers, > Rémi > > On 05/25/2012 02:08 PM, Alan Bateman wrote: >> On 24/05/2012 21:45, Mike Duigou

Re: Review Request CR#7118743 : Alternative Hashing for String with Hash-based Maps

On May 26 2012, at 08:43 , Ulf Zibis wrote: > Am 24.05.2012 21:34, schrieb Mike Duigou: >> Hello All; >> >> I have updated the webrevs for alternative hashing for String with feedback >> from Remi, Doug, Ulf and internal reviewers. >> >> Addition

  1   2   3   4   5   6   7   8   9   10   >