RE: Deadlock between FileHandler and ConsoleHandler when using customized formatter

2013-12-05 Thread Jason Mehrens
Shi Jun Zhang, This problem is like hooking up your sink drain to your sink faucet. Even if it you can get it to work you still would not want to use it. In your code example you could just pre-pend the thread name to the formatter string and return it. However, if you really, really,

RE: Theoretical data race on java.util.logging.Handler.sealed

2013-12-17 Thread Jason Mehrens
Handlers can be subclassed. Is it a security concern when doPrivileged is invoking non-final public/protected methods? For example, @Override public void setOutputStream(OutputStream out) { LogManager.getLogManager().reset(); } @Override public void setLevel(Level l) {

RE: Optimization in collections API

2014-01-07 Thread Jason Mehrens
Robert, If you can create a micro benchmark that fools all of the core-libs-dev or real world benchmark that actually shows performance improvements you might be able to get this patch in to the source code. Previous attempts are covered under

RE: RFR [9] 8011645: CopyOnWriteArrayList.COWSubList.subList does not validate range properly

2014-01-31 Thread Jason Mehrens
Martin, Unifying the List testing code might be kind of tricky with https://bugs.openjdk.java.net/browse/JDK-4506427 as unresolved. http://docs.oracle.com/javase/7/docs/api/java/util/List.html http://docs.oracle.com/javase/7/docs/api/java/util/AbstractList.html The patch looks good though.

RE: Random access in ArrayDeque

2014-02-10 Thread Jason Mehrens
Too bad there is no interface with only get(index), set(index, x), and indexOf(x). Implementing only these would probably satisfy all candidate usages. With default methods can't we modify the RandomAccess marker interface with hostile implementations? Jason

RE: RFR: 8035584: (s) ArrayList(c) should avoid inflation if c is empty

2014-03-12 Thread Jason Mehrens
Mike, In the constructor do you think you should skip the call to isEmpty and just check the length of toArray? Seems like since the implementation of the given collection is unknown it is probably best to perform as little interaction with it as possible. Also it would be possible for

RE: RFR: 8035584: (s) ArrayList(c) should avoid inflation if c is empty

2014-03-13 Thread Jason Mehrens
for toArray() and I have also added this to ArrayList's toArray. http://cr.openjdk.java.net/~mduigou/JDK-8035584/3/webrev/ Mike On Mar 12 2014, at 07:06 , Jason Mehrens jason_mehr...@hotmail.com wrote: Mike, In the constructor do you think you should skip the call to isEmpty and just check the length

RE: RFR: 8035584 : (s) ArrayList(c) should avoid inflation if c is empty

2014-04-25 Thread Jason Mehrens
Looks good to me. Jason From: mike.dui...@oracle.com Subject: RFR: 8035584 : (s) ArrayList(c) should avoid inflation if c is empty Date: Wed, 23 Apr 2014 15:33:48 -0700 To: core-libs-dev@openjdk.java.net Hello all; Revisiting this issue again at long last I have updated the proposed

RE: RFR: 8035584: (s) ArrayList(c) should avoid inflation if c is empty - (toArray spec)

2014-04-25 Thread Jason Mehrens
MIke, The inner T.V. lawyer in me has been trying and find some loophole that will allow returning the same empty array from toArray. The spec states ..no references to it are maintained by this collection. The Saul Goodman loophole is that this collection implies object member

Zombie FileHandler locks can exhaust all available log file locks.

2014-06-20 Thread Jason Mehrens
Daniel, Jim, In JDK8 the FileHandler file locking was changed to use FileChannel.open with CREATE_NEW. If the file exists (locked or not) the FileHandler will rotate due to safety concerns about writing to the same log file. The FileHandler has an upper bound of 100 as the number of file

RE: Zombie FileHandler locks can exhaust all available log file locks.

2014-06-23 Thread Jason Mehrens
, -- daniel On 6/20/14 10:54 PM, Jason Mehrens wrote: Daniel, Jim, In JDK8 the FileHandler file locking was changed to use FileChannel.open with CREATE_NEW. If the file exists (locked or not) the FileHandler will rotate due to safety concerns about writing to the same log file. The FileHandler

RE: Zombie FileHandler locks can exhaust all available log file locks.

2014-06-24 Thread Jason Mehrens
Daniel, With regard to JDK-4420020, in the original webrev Jim was incorrectly using java.io.File.canWrite() but that webrev was replaced by the current version. The NIO.2 code performs the effective access checks correctly. With regard to JDK-6244047 my concern was that checking the

RE: RFR: 8048020 - Regression on java.util.logging.FileHandler

2014-06-25 Thread Jason Mehrens
Daniel, FileChannel.open(WRITE,APPEND) could throw NoSuchFileException during a startup and shutdown race between two VMs. That case needs to either perform a bounded retry or continue and rotate. Jason Date: Tue, 24 Jun 2014 20:34:46 +0200 From:

RE: RFR: 8048020 - Regression on java.util.logging.FileHandler

2014-07-01 Thread Jason Mehrens
on java.util.logging.FileHandler On 6/25/14 2:34 PM, Jason Mehrens wrote: Daniel, FileChannel.open(WRITE,APPEND) could throw NoSuchFileException during a startup and shutdown race between two VMs. That case needs to either perform a bounded retry or continue and rotate. Hi Jason, Alan, Here

RE: RFR [9]: 8050142: Optimize java.util.Formatter

2014-07-14 Thread Jason Mehrens
Claes, Maybe change (or not): -throw new UnknownFormatConversionException(String.valueOf(c)); +throw new UnknownFormatConversionException(String.valueOf(conv)); I haven't examined it too deeply but it seems odd that some of those print methods don't use the given locale when converting

RE: java.util.logging.FileHandler integer overflow prevents file rotation

2014-08-19 Thread Jason Mehrens
Stanimir, Looks like the int overflow on the metered stream is an issue that hasn't been tracked. The other issues have been reported under https://bugs.openjdk.java.net/browse/JDK-6433253 and https://bugs.openjdk.java.net/browse/JDK-8028786 Jason

RE: A List from Iterator

2014-08-28 Thread Jason Mehrens
Pavel, The stated reason is to discourage use of a 'poor man's collection'. See Java Collections API Design FAQ (http://docs.oracle.com/javase/7/docs/technotes/guides/collections/designfaq.html) under the 'Collection Interface' section. Jason

RE: RFR: 8043306 - Provide a replacement for the API that allowed to listen for LogManager configuration changes

2014-09-10 Thread Jason Mehrens
Daniel I think you should be able to remove the 'other instanceof ConfigurationListener' branch in the ConfigurationListener.equals method. Should work the same with or without that branch. Jason Date: Wed, 10 Sep 2014 11:49:51 +0200 From:

RE: RFR: 8043306 - Provide a replacement for the API that allowed to listen for LogManager configuration changes

2014-09-12 Thread Jason Mehrens
Daniel, I suppose that the propagating uncaught exceptions to the caller was the previous behavior of the old property change methods but it seems out of place for the LogManager. The LogManager is a global resource so broken listener code in web app A could suppress notifications in web app

RE: RFR: 8043306 - Provide a replacement for the API that allowed to listen for LogManager configuration changes

2014-09-12 Thread Jason Mehrens
-libs-dev@openjdk.java.net Subject: Re: RFR: 8043306 - Provide a replacement for the API that allowed to listen for LogManager configuration changes On 9/12/14 5:39 PM, Jason Mehrens wrote: Daniel, I suppose that the propagating uncaught exceptions to the caller was the previous behavior

RE: RFR: 8059767: FileHandler should allow 'long' limits and handle overflow of MeteredStream.written.

2014-10-07 Thread Jason Mehrens
Hi Daniel, The only thing I noticed is a missing @since tag on the FileHandler. Jason Date: Tue, 7 Oct 2014 15:13:13 +0200 From: daniel.fu...@oracle.com To: core-libs-dev@openjdk.java.net Subject: RFR: 8059767: FileHandler should allow 'long'

JDK-6774110 lock file is not deleted when child logger is used

2014-10-09 Thread Jason Mehrens
Daniel, The evaluation on this bug is not quite correct. What is going on here is the child logger is garbage collected which makes the FileHandler unreachable from the LogManager$Cleaner which would have closed the attached FileHandler. In the example, there is no hard reference that

RE: JDK-6774110 lock file is not deleted when child logger is used

2014-10-09 Thread Jason Mehrens
), it will be a good incentive to attempt a fix :-) Thanks again, -- daniel On 10/9/14 9:56 PM, Jason Mehrens wrote: Daniel, The evaluation on this bug is not quite correct. What is going on here is the child logger is garbage collected which makes the FileHandler unreachable from the LogManager

RE: JDK-6774110 lock file is not deleted when child logger is used

2014-10-10 Thread Jason Mehrens
that scenario, thanks for pointing it out! If i can write a reproducer (which should not be too difficult), it will be a good incentive to attempt a fix :-) Thanks again, -- daniel On 10/9/14 9:56 PM, Jason Mehrens wrote: Daniel, The evaluation on this bug is not quite correct. What is going

RE: RFR: 8060132: Handlers configured on abstract nodes in logging.properties are not always properly closed

2014-10-10 Thread Jason Mehrens
Daniel, Looks good. As always, thanks for fixing this. Jason Date: Fri, 10 Oct 2014 17:39:55 +0200 From: daniel.fu...@oracle.com To: stani...@riflexo.com; core-libs-dev@openjdk.java.net CC: jason_mehr...@hotmail.com Subject: Re: RFR: 8060132:

RE: RFR: 6244047: impossible to specify directories to logging FileHandler unless they exist

2012-11-09 Thread Jason Mehrens
Jim, You might just want to change the code to create and close a FileOutputStream in a way that doesn't truncate or damage the target file. Or maybe use the NIO file code if that is possible. See BUG ID 4420020. Jason Date: Fri, 9 Nov 2012 16:37:02 -0500 From: jim.g...@oracle.com

RE: RFR: 6244047: impossible to specify directories to logging FileHandler unless they exist

2012-11-13 Thread Jason Mehrens
Jim, Looking at the webrev again I think I tricked myself into thinking that 'parentFile' was a file that could be opened with a stream when it actually is a directory. I think the best fix would be to add a check in the catch block (around line 432) and only continue if the directory of

RE: RFR 2: JDK-8005263: Logging APIs takes SupplierString for message

2012-12-22 Thread Jason Mehrens
The msg argument in most cases is a string literal because it is either a resource bundle key or a MessageFormat literal. The established best practice is to convert on the fly construction of messages to use the MessageFormat syle logging. This current patch is kind of anti-pattern of that

RE: RFR 2: JDK-8005263: Logging APIs takes SupplierString for message

2012-12-27 Thread Jason Mehrens
Henry, Please don't apply this patch. This patch and the suggested workarounds are still an anti-pattern of the logging API. You don't want to encourage this type of on the fly message construction because it can't be localized. Even Netbeans has a code hint to undo this pattern

RE: RFR 2: JDK-8005263: Logging APIs takes SupplierString for message

2012-12-27 Thread Jason Mehrens
Brian, It's on my list too for lambdafying I just disagree with current implementation. I understand and agree that having to create a guard should not be required. It's awful to have to do. The point is that patch is still way too eager because you don't want to evaluate a message

RE: RFR 2: JDK-8005263: Logging APIs takes SupplierString for message

2012-12-28 Thread Jason Mehrens
. Cheers, Henry On Dec 27, 2012, at 4:16 PM, Jason Mehrens jason_mehr...@hotmail.com wrote: Brian, It's on my list too for lambdafying I just disagree with current implementation. I understand and agree that having to create a guard should not be required. It's awful to have to do

RE: RFR 2: JDK-8005263: Logging APIs takes SupplierString for message

2013-01-01 Thread Jason Mehrens
Henry, For point 1 yes. Not just me, even NetBeans considers it an anti-pattern. Supplier return values should be placed in the LogRecord.setParameters method. The 'msg' argument, if present, should always be string. Using the patch I just described, it would allow callers to write the

RE: RFR: 8007806: Need a Throwables performance counter

2013-02-22 Thread Jason Mehrens
From this webrev http://cr.openjdk.java.net/~nloodin/exception-tracing/webrev.01/ you are counting the number of throwables constructed. You might want to change the name to reflect that. I don't think anyone would want to write a spec for how many throwables are thrown given that a

RE: RFR: 8007806: Need a Throwables performance counter

2013-02-26 Thread Jason Mehrens
Just to be clear, what I was trying to say that this review is just to add the counter, and the discussion on how and when to access it is something that I'd much rather have in context of a review of that code... Then the counter name should not bind you to how it is counting. +1 for

RE: Code review request for 8012044: Give more information about self-suppression from Throwable.addSuppressed

2013-04-12 Thread Jason Mehrens
Joe, Should this same logic be applied to the exceptions thrown from initCause? Seems like that would be consistent with this change. Jason Date: Thu, 11 Apr 2013 18:19:30 -0700 From: joe.da...@oracle.com To: core-libs-dev@openjdk.java.net Subject: Code review request for 8012044: Give more

RE: Code review request for 8012044: Give more information about self-suppression from Throwable.addSuppressed

2013-04-12 Thread Jason Mehrens
The landmines are the retrofitted exception classes as shown here https://netbeans.org/bugzilla/show_bug.cgi?id=150969 and https://issues.jboss.org/browse/JBREM-552. Really, if the ISE or IAE is thrown it is going to suppress 'this' and 'cause'. It would be nice to see the given 'cause'

RE: Code review request for 8012044: Give more information about self-suppression from Throwable.addSuppressed

2013-04-12 Thread Jason Mehrens
2013 12:08:07 -0700 From: joe.da...@oracle.com To: jason_mehr...@hotmail.com CC: core-libs-dev@openjdk.java.net Subject: Re: Code review request for 8012044: Give more information about self-suppression from Throwable.addSuppressed On 04/12/2013 11:22 AM, Jason Mehrens wrote: The landmines

RE: Code review request for 8012044: Give more information about self-suppression from Throwable.addSuppressed

2013-04-15 Thread Jason Mehrens
...@oracle.com CC: jason_mehr...@hotmail.com; core-libs-dev@openjdk.java.net Subject: Re: Code review request for 8012044: Give more information about self-suppression from Throwable.addSuppressed On 13/04/2013 5:08 AM, Joe Darcy wrote: On 04/12/2013 11:22 AM, Jason Mehrens wrote: The landmines

RE: Code review request for 8012044: Give more information about self-suppression from Throwable.addSuppressed

2013-04-17 Thread Jason Mehrens
; david.hol...@oracle.com Subject: Re: Code review request for 8012044: Give more information about self-suppression from Throwable.addSuppressed On 04/14/2013 07:36 PM, Joe Darcy wrote: On 04/12/2013 07:29 PM, Jason Mehrens wrote: Joe, You'll have guard ise.addSuppressed against null. Looks

RE: REASSERT Code review request for 8012044: Give more information about self-suppression from Throwable.addSuppressed

2013-04-23 Thread Jason Mehrens
I still find the use of addSuppressed in initCause to be questionable. Given: catch(SomeException s) { sharedException.initCause(s); // oops already has a cause throw sharedException; } then the ISE isn't suppressing 's', but replacing/suppressing sharedException in my view,

RE: REASSERT Code review request for 8012044: Give more information about self-suppression from Throwable.addSuppressed

2013-04-25 Thread Jason Mehrens
Looks good. I still think last sentence of the Throwable.addSuppressed javadocs side steps the counter arguments. Thanks for working on this, Jason Date: Thu, 25 Apr 2013 00:16:05 -0700 From: joe.da...@oracle.com To: david.hol...@oracle.com;

RE: RFR [8005953] Speedup construction of CopyOnWriteArraySet in special cases

2013-04-30 Thread Jason Mehrens
Ivan, The addAllAbsent() function has O(c.length^2) complexity, so construction time quickly grows with the input size. However, if we knew that c is a Set, we could construct the COWAS in linear time. You have to be able to prove that the given Set uses the same equivalence relation as the

RE: RFR : 7129185 : Add Collections.{checked|empty|unmodifiable}Navigable{Map|Set}

2013-05-07 Thread Jason Mehrens
Mike, EmptyNavigableMap and EmptyNavigableSet should contain the static final references themselves and not the Collections class. That way they load on demand and not with the Collections class. Use default visibility inside the inner class so the compiler doesn't generate method to promote

RE: RFR 8009581: Xpathexception does not honor initcause()

2013-05-17 Thread Jason Mehrens
Aleksej, Should readObject call super.initCause instead of this.initCause? Maybe initCause should be only called if scause != null super.getCause() == null. If super.getCause is not null initCause will always fail. Jason Date: Fri, 17 May 2013

RE: RFR 8009581: Xpathexception does not honor initcause()

2013-05-28 Thread Jason Mehrens
Alan, David, thank you for comments - I also agree with all of them. And as a result v3: http://cr.openjdk.java.net/~dmeetry/8009581/webrev.3/ I think this looks better. I assume that since the super.getCause() is null that there is no need to handle IllegalStateException now. You can

RE: RFR 8009581: Xpathexception does not honor initcause()

2013-06-04 Thread Jason Mehrens
Aleksej, Looks good to me. I think Alan wanted you to use the ser files to create a byte array + ByteArrayInputStream in the actual test class instead of including actual serial data files along side the test. Jason The next version of webrev:

RE: RFR 8009736: Comparator API cleanup

2013-06-14 Thread Jason Mehrens
Any chance the static method Comparator.reverseOrder() will be renamed to something like reversedNaturalOrder? Now all of the calls to Comparator.reverseOrder() take on new meaning. Jason Date: Tue, 11 Jun 2013 14:04:38 -0700 From: henry@oracle.com To: lambda-...@openjdk.java.net;

RE: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

2013-08-29 Thread Jason Mehrens
Hi Daniel, Why not take the code the opposite direction and change everything to volatile? The properties of handler are mostly read and rarely written. If isLoggable, getLevel, and getFilter were lock free the LogRecords that are not loggable could barge past the publish method. That way

RE: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

2013-08-29 Thread Jason Mehrens
Yes - this is a possibility I envisaged too. But would that be better? I didn't want to remove any synchronized blocks because I'm really not sure of what consequences it might have. getLevel()/setLevel() are already synchronized on Handler. It is my belief that most

RE: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

2013-08-29 Thread Jason Mehrens
PM, Jason Mehrens wrote: Yes - this is a possibility I envisaged too. But would that be better? I didn't want to remove any synchronized blocks because I'm really not sure of what consequences it might have. getLevel()/setLevel() are already synchronized on Handler

RE: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

2013-08-30 Thread Jason Mehrens
The coarse grain locking in FileHandler, MemoryHandler, SocketHandler, and StreamHandler publish could/should be relaxed so we are not calling isLoggable while holding the lock. Jason Date: Fri, 30 Aug 2013 13:50:56 +0200 From: daniel.fu...@oracle.com To: david.hol...@oracle.com Subject:

RE: RFR: JDK-6823527: java.util.logging.Handler has thread safety issues

2013-08-30 Thread Jason Mehrens
I'm not sure I'd want to attempt that. Modifications in logging code have a tendency to come back and bite you ;-(... I understand. Maybe a future RFE. I'm happy with your changes then. isLoggable() is not synchronized and no longer calls synchronized methods since we're now using

RE: hg: jdk7/tl/jdk: 7008595: Class loader leak caused by keepAliveTimer thread in KeepAliveCache

2011-02-03 Thread Jason Mehrens
I understand why the previous code leaks but, why is a null value safe to use for the CCL? Is it because this code is part of the JDK (not ext classloader) or is it because no more class loading is done on that thread? For 3rd party libs, should the CCL be set to null or set to the

RE: Crash in ntdll.dll due to JdbcOdbcConnection

2011-03-01 Thread Jason Mehrens
Lance is the best person to comment on this but I don't think the JDBC-ODBC bridge has been maintained for a few years and probably isn't up to the latest JDBC version. No problem proposing a patch but I just wonder if it is actually used these days. The only thing I've used the JDBC-ODBC

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

2011-03-15 Thread Jason Mehrens
Hi Steve, I was one of the people that provided feedback on Mike's patch. In my case, it was a mishap of reply to sender vs. reply to all. I don't have the original email but, the result are visible in the test case that Mike wrote. My main concern with the old patch that if you use a raw

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

2011-03-29 Thread Jason Mehrens
Is it necessary for 'NULL' in EnumMap to have hashCode of zero? If so, would using new Integer(0) be better than creating a subclass with regards to footprint and classloading? A similar issue was brought up before: http://mail.openjdk.java.net/pipermail/core-libs-dev/2011-March/006154.html

RE: Code review request for 7021922: java.lang.annoation.IncompleteExceptions throws NPE when type is null

2011-06-15 Thread Jason Mehrens
Typo in the javadocs. is either.. should be if either... Jason Date: Wed, 15 Jun 2011 11:00:27 -0700 From: joe.da...@oracle.com To: core-libs-dev@openjdk.java.net Subject: Code review request for 7021922: java.lang.annoation.IncompleteExceptions throws NPE when type is null Hello.

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

2011-07-13 Thread Jason Mehrens
Mike, The history is in the evaluation of http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6394757 I don't think that even adding a empty check can be considered an optimization when dealing with two abstract things. The iterator creation here is 'good garbage' and worst case

RE: JDK 8 Code Review Request for 5029031 (coll) Collections.checkedQueue(QueueT, Class) is missing

2011-10-03 Thread Jason Mehrens
What's about moving public boolean equals(Object o) {return o == this || c.equals(o);} to CheckedCollection and remove it in CheckedSet (L2394), CheckedList (L2506) and public int hashCode() {return c.hashCode();} to CheckedCollection and remove it in CheckedSet (L2395) That

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

2011-10-29 Thread Jason Mehrens
Darryl, I would get rid of the public static field so this class can be lazy loaded like EmptyIterator and ReverseComparator (in 1.7). What about accepting a comparator as an argument? I bet the bug report is predates 1.6, so maybe you should target NavigableSet instead of sorted set.

Collections.checkedQueue() offer method should not call add.

2011-10-31 Thread Jason Mehrens
Darryl, CheckedQueue.offer should call 'this.queue.offer' instead of 'this.add'. If you pass a Queue with bounded capacity (ArrayBlockingQueue) the CQ.offer method should return false when the queue is full but will instead throw an IllegalStateException. The current version also is

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

2011-11-13 Thread Jason Mehrens
Darryl, 2. The comparator method is using raw types. The SortedSet.comparator() method spec allows returning of null. Right. But, the return type of the implementation is a raw comparator not Comparator? super E defined by the interface. 4. Only the IAE if statement is need for your

RE: Code Review Request for Bug #4802647

2011-11-30 Thread Jason Mehrens
Brandon, Are there any opinions on this from other Collections experts? http://cr.openjdk.java.net/~mduigou/4802647/0/webrev/ Shouldn't the test include all collections included with the JDK? Any override of these methods could repeat the same (bad) behavior. Jason

RE: Code Review Request for Bug #4802647

2011-12-21 Thread Jason Mehrens
Date: Tue, 20 Dec 2011 10:12:02 +1000 From: david.hol...@oracle.com To: brandon.passan...@oracle.com Subject: Re: Code Review Request for Bug #4802647 CC: core-libs-dev@openjdk.java.net Brandon, I don't see the purpose of NewAbstractSet. It is identical to NewAbstractCollection. I

RE: Code Review Request Bug #7129185:(coll) Please add Collections.emptyNavigableSet()

2012-01-24 Thread Jason Mehrens
addresses comments made by Jason Mehrens to the commit of the fix for bug #4533691, including adding a Collections.emptyNavigableSet method. Tests are included. Webrev, can be found here: http://cr.openjdk.java.net/~dmocek/7129185/webrev.00 Thanks, Darryl

RE: Code Review Request Bug #7129185:(coll) Please add Collections.emptyNavigableSet()

2012-01-24 Thread Jason Mehrens
3. What if I want to create an empty set navigable set with supplied comparator? Extending is not an option. This is the one issue I wanted to discuss...is this necessary? I was thinking about how this would be implemented. You would need to supply a comparator to the emptyNavigableSet.

RE: Code Review Request Bug #7129185:(coll) Please add Collections.emptyNavigableSet()

2012-01-24 Thread Jason Mehrens
Darryl, Per the SortedSet docs: .null if this set uses the natural ordering of its elements. Per the NavigableSet.descendingSet docs: The returned set has an ordering equivalent to Collections.reverseOrder(comparator()) = NavigableSetString fwd = new

RE: Codereview request for 6995537: different behavior in iso-2022-jp encoding between jdk131/140/141 and jdk142/5/6/7

2012-02-09 Thread Jason Mehrens
Sherman, As a workaround, what about allowing a write of empty string or empty char array to call flushBuffer? If you call PrintStream.print() then flushBuffer is called on the internal writers. But if you try the same by doing OuputStreamWriter.write() the flushbuffer call is trapped by a

RE: Codereview request for 6995537: different behavior in iso-2022-jp encoding between jdk131/140/141 and jdk142/5/6/7

2012-02-09 Thread Jason Mehrens
() into osw.flushBuffer (in StreamWriter.implFlushBuffer()). -Sherman On 02/09/2012 10:24 AM, Jason Mehrens wrote: Sherman, As a workaround, what about allowing a write of empty string or empty char array to call flushBuffer? If you call PrintStream.print() then flushBuffer is called

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

2012-02-23 Thread Jason Mehrens
David, For completeness, you might want to link this bug to bug id 6360946 (coll) SetFromMap.equals should perform identity check. Most of the wrapper classes were fixed to include an identity check for that bug. Digging up some old messages from December 2005, the synchXXX wrappers were

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

2012-03-01 Thread Jason Mehrens
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 = Arrays.asList(a); //Evil raw type Collections.sort(l);

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

2012-03-12 Thread Jason Mehrens
I'm not really confident about proposing assertions as lint detection rather than adding explicit checks. We wouldn't (and don't) use optional assertions for array bounds checking. This has clearly been the right choice. I'm still considering my feelings about whether to be hardline and

RE: review request: 4244896: (process) Provide System.getPid(), System.killProcess(String pid)

2012-04-18 Thread Jason Mehrens
Rob, It looks like waitFor is calling Object.wait(long) without owning this objects monitor. If I pass Long.MAX_VALUE to waitFor, shouldn't waitFor return if the early if the process ends? Jason Date: Tue, 17 Apr 2012 15:56:30 +0100 From: rob.mcke...@oracle.com To:

RE: review request: 4244896: (process) Provide System.getPid(), System.killProcess(String pid)

2012-04-20 Thread Jason Mehrens
Rob, 2) As Alan noted, there is really no need for isAlive() if people are happy with the idea of waitFor(long, TimeUnit). I'd appreciate any feedback on this aspect of the fix. Process.isAlive is similar to Future.isDone(). I think isAlive fills that need to have a simple query method

RE: String.subSequence and CR#6924259: Remove offset and count fields from java.lang.String

2012-06-24 Thread Jason Mehrens
Mike, Why not implement subSequence as 'java.nio.CharBuffer.wrap(data, beginIndex, endIndex).asReadOnlyBuffer()' ? Easy to implement and test. The nice thing is that parsers would know what a 'CharBuffer' vs. a sub sequence String internal class. Jason Subject: String.subSequence and

RE: Code review request for 6857789: (reflect) Create common superclass of reflective exceptions

2009-07-10 Thread Jason Mehrens
Joe, Wouldn't LinkageException be a better fit than ReflectiveOperationException? Shorter name and it would mimic the LinkageError inheritance tree introduced in JDK1.0. I.E. LinkageError - NoClassDefFoundError, LinkageException - ClassNotFoundException This request seems dangerous,

RE: What methods should go into a java.util.Objects class in JDK 7?

2009-10-02 Thread Jason Mehrens
Joe, * null safe toString(Object), returning null for a null argument Doesn't String.valueOf do the same thing? http://java.sun.com/javase/6/docs/api/java/lang/String.html#valueOf(java.lang.Object) What about a toIdentityString(Object) instead? Some of the nice properties of an

RE: What methods should go into a java.util.Objects class in JDK 7?

2009-10-02 Thread Jason Mehrens
By toIdentityString, do you mean the String that would be returned by toString if toString is not overridden? -Joe Yep. As in: return o != null ? o.getClass().getName() +'@'+ Integer.toHexString(System.identityHashCode(o)) : null; I suppose the name should be identityToString

RE: What methods should go into a java.util.Objects class in JDK 7?

2009-10-02 Thread Jason Mehrens
I think a better name would be defaultToString since it is the default toString from Object. However, I haven't ever heard anyone else request easier access to the default toString before so I'm not convinced this should go into Objects. -Joe One use case is implementing toString

RE: What methods should go into a java.util.Objects class in JDK 7?

2009-10-07 Thread Jason Mehrens
Hi Stephen, [...] In key places there are multiple options. NIO Path vs File and Calendar vs Date are examples. As you know, Path (resp. Calendar) is just an attempt to correct the mess introduced by File (resp. Date). So yes, there is duplication but this duplication is done to

RE: Objects.toString [Re: What methods should go into a java.util.Objects class in JDK 7?]

2009-10-08 Thread Jason Mehrens
Joe, I'll volunteer some find usage stats from the code base I work on for a living: 565 ObjectUtils.toString(Object) calls. 487 String.valueOf(Object) calls. Hopefully others can contribute their find usage stats. It seems to me that both behaviors are useful. Jason

RE: Need reviewer - adding -ea -esa to testing via jdk/test/Makefile

2009-12-11 Thread Jason Mehrens
Kelly, Are any of the tests run with -Xcheck:jni? If not, it might be something to consider. Jason Mehrens Date: Fri, 11 Dec 2009 11:20:53 -0800 From: kelly.oh...@sun.com Subject: Need reviewer - adding -ea -esa to testing via jdk/test/Makefile To: core-libs-dev@openjdk.java.net

RE: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

2010-02-25 Thread Jason Mehrens
Alan, Shouldn't the loading of 'this.count' into 'len' be only performed if 'h' is zero? Otherwise, when hash is not zero we perform a little unnecessary work every time hashCode is called. Jason Date: Thu, 25 Feb 2010 21:17:37 +0100 From: ulf.zi...@gmx.de To: alan.bate...@sun.com

RE: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

2010-02-27 Thread Jason Mehrens
will see that, am I wrong ? -Ulf Am 27.02.2010 18:50, schrieb Jason Mehrens: Here are two more variants you might want to throw into the benchmark. public int hashCode6() { int h = hash; if (h == 0 count 0) { int off = offset; char val[] = value

RE: Need reviewer for forward port of 6815768 (File.getXXXSpace) and 6815768 (String.hashCode)

2010-03-03 Thread Jason Mehrens
String.hash should only have two known states, zero and the actual computed hash code. http://bugs.sun.com/view_bug.do?bug_id=6611830 Jason Date: Sun, 28 Feb 2010 17:09:15 +0100 From: ulf.zi...@gmx.de To: alan.bate...@sun.com Subject: Re: Need reviewer for forward port of 6815768

RE: java.util.Pair

2010-03-30 Thread Jason Mehrens
Stephen, I'm all for adding support for unmodifiableIterable, unmodifableNavigableMap, and unmodifableNavigableSet. However, I think adding public access to such a iterator decorator goes against the guidelines of the collections design faq (4 and 5):

RE: RFR: 8060132: Handlers configured on abstract nodes in logging.properties are not always properly closed

2014-10-24 Thread Jason Mehrens
The strong reference is not changed to weak if later on all handlers are removed from the logger. The only other solution I can think of to satisfy all of the previous pain points is to go back to keeping a reference to Logger.handlers in LogManager.LogNode and create a

RE: RFR 9 8055330: (process spec) ProcessBuilder.start and Runtime.exec should throw UnsupportedOperationException ...

2015-02-19 Thread Jason Mehrens
Standing with Martin on this, I wanted to note the following from the ProcessBuilder docs: The exact nature of the exception is system-dependent, but it will always be a subclass of IOException The type of exception thrown is the one thing that is defined in the spec. The rest may be vague or

RE: RFR: 8072645: java.util.logging should use java.time to get more precise time stamps

2015-02-16 Thread Jason Mehrens
Subject: Re: RFR: 8072645: java.util.logging should use java.time to get more precise time stamps Hi Jason, On 2/13/15 10:57 PM, Jason Mehrens wrote: Daniel, In the XMLFormatter.format you can get rid of the double call to getNanoAdjustment() since you have stored the value

RE: RFR: 8072645: java.util.logging should use java.time to get more precise time stamps

2015-02-18 Thread Jason Mehrens
Daniel, Was it determined if you were going to modify the existing logger.dtd or create a logger-v2.dtd? If you are going to create a v2 then I think it might make sense to make dtd log manger property for the XMLFormatter instead of useInstant property. So if you are using v2 then instant

RE: RFR: 8072645: java.util.logging should use java.time to get more precise time stamps

2015-02-13 Thread Jason Mehrens
Daniel, In the XMLFormatter.format you can get rid of the double call to getNanoAdjustment() since you have stored the value in the local var 'nanos'. For the new XMLFormatter constructor what do you think about using Properties, FunctionString, String, or perhaps a builder pattern? That

LogManager.readConfiguration doesn't document IAE and NPE.

2015-03-18 Thread Jason Mehrens
Daniel, It occurred to me after reading Brian's patch for https://bugs.openjdk.java.net/browse/JDK-8075362 that the LogManager.readConfiguration methods do not document NPE or IAE that can be triggered by Properties.load. Do we need to file a bug just against logging or should larger bug be

RE: JEP 102 Process Updates revised API draft

2015-03-09 Thread Jason Mehrens
could be removed from Process.getPid() too, right? Which solves that small API wart. -Chris. Roger On 3/9/2015 6:10 AM, Chris Hegarty wrote: On 06/03/15 19:34, Jason Mehrens wrote: Hi Chris, Since getPid can throw UOE that means that compareTo could now throw UOE. Ooh... I don't like

RE: JEP 102 Process Updates revised API draft

2015-03-06 Thread Jason Mehrens
Hi Chris, Since getPid can throw UOE that means that compareTo could now throw UOE. Jason Subject: Re: JEP 102 Process Updates revised API draft From: chris.hega...@oracle.com Date: Fri, 6 Mar 2015 11:59:28 + To: roger.ri...@oracle.com

RE: RFR: 8075810: LogManager.readConfiguration may throw undocumented IllegalArgumentException

2015-03-24 Thread Jason Mehrens
Hi Daniel, Looks good. The only other alternative would be to use java.io.CharConversionException over IOException. We could even consider dropping the cause because the subclass of I/O exception would convey the same meaning. Minor formatting issues with a missing space after the catch

RE: RFR: 7113878: LogManager - namedLoggers should be ConcurrentHashMap instead of Hashtable

2015-03-30 Thread Jason Mehrens
For the test shouldn't you include a test for getLoggerNames to check that if the returned Enumeration is an instanceof Iterator then its remove method must throw an UnsupportedOperationException? That makes it clear as to why you are using Collections.enumeration. Jason

RE: RFR 9 8055330: (process spec) ProcessBuilder.start and Runtime.exec should throw UnsupportedOperationException ...

2015-02-21 Thread Jason Mehrens
. On Fri, Feb 20, 2015 at 3:49 AM, Alan Bateman alan.bate...@oracle.com wrote: On 19/02/2015 21:54, Jason Mehrens wrote: I'm assuming that compatibility is given more weight vs. correcting choices made in the original design. Yes, I think we've spent more than enough time on it. In this case

RE: RFR: 8075810: LogManager.readConfiguration may throw undocumented IllegalArgumentException

2015-03-27 Thread Jason Mehrens
wrote: Thanks for the review Jason! On 24/03/15 18:01, Jason Mehrens wrote: Hi Daniel, Looks good. The only other alternative would be to use java.io.CharConversionException over IOException. We could even consider dropping the cause because the subclass of I/O exception would convey

RE: RFR: 7113878: LogManager - namedLoggers should be ConcurrentHashMap instead of Hashtable

2015-03-26 Thread Jason Mehrens
The snapshot enumeration is a welcomed change. ConcurrentHashMap has legacy Hashtable methods so you can save a little bit by calling namedLoggers.keys() instead of wrapping the key set. Jason Date: Thu, 26 Mar 2015 14:32:23 +0100 From:

RE: [9] RFR of 8042377: BufferedWriter and FilteredOutputStream.close throw IAE if flush and close throw equal exceptions

2015-06-24 Thread Jason Mehrens
. Thanks, Brian On Jun 24, 2015, at 1:03 PM, Jason Mehrens jason_mehr...@hotmail.com wrote: Not sure on this but, isn't it a little risky to import AtomicBoolean into such low level class? I vaguely remember there was an issue with using AtomicXXX in java.lang.Thread. Not sure if this case

  1   2   >