Re: RFR [8038982]: java/lang/ref/EarlyTimeout.java failed again
On 15.04.2014 6:23, Mandy Chung wrote: On 4/14/2014 11:26 AM, Ivan Gerasimov wrote: Actually, zero tolerance should be sufficient now even for Windows platform. Measuring the time with nanoTime() should make the inner and outer time intervals consistent. I've added the tolerance just to play safer. I can remove it. That'd be even better! Alright, I removed the tolerance back. So now the only change is how the time interval is measured: http://cr.openjdk.java.net/~igerasim/8038982/1/webrev/ Sincerely yours, Ivan thanks Mandy
Re: RFR [8038982]: java/lang/ref/EarlyTimeout.java failed again
On 15/04/2014 4:10 PM, Ivan Gerasimov wrote: On 15.04.2014 6:23, Mandy Chung wrote: On 4/14/2014 11:26 AM, Ivan Gerasimov wrote: Actually, zero tolerance should be sufficient now even for Windows platform. Measuring the time with nanoTime() should make the inner and outer time intervals consistent. I've added the tolerance just to play safer. I can remove it. That'd be even better! Alright, I removed the tolerance back. So now the only change is how the time interval is measured: http://cr.openjdk.java.net/~igerasim/8038982/1/webrev/ This is the right change to make. I'm a little surprised we are seeing these timing problems though. That said the resolution of timed blocking calls and the resolution of currentTimeMillis() can be quite different on any platform, not just windows. In general nanoTime should always be used to measure elapsed time. Thanks, David Sincerely yours, Ivan thanks Mandy
Re: RFR [8038982]: java/lang/ref/EarlyTimeout.java failed again
Hi guys, Should 'actual' and 'reference' be declared as volatile? I see that they are accessed from main() after joining the threads. Or does joining the threads guarantees that 'main' will see the right values? best regards, -- daniel On 4/15/14 8:48 AM, David Holmes wrote: On 15/04/2014 4:10 PM, Ivan Gerasimov wrote: On 15.04.2014 6:23, Mandy Chung wrote: On 4/14/2014 11:26 AM, Ivan Gerasimov wrote: Actually, zero tolerance should be sufficient now even for Windows platform. Measuring the time with nanoTime() should make the inner and outer time intervals consistent. I've added the tolerance just to play safer. I can remove it. That'd be even better! Alright, I removed the tolerance back. So now the only change is how the time interval is measured: http://cr.openjdk.java.net/~igerasim/8038982/1/webrev/ This is the right change to make. I'm a little surprised we are seeing these timing problems though. That said the resolution of timed blocking calls and the resolution of currentTimeMillis() can be quite different on any platform, not just windows. In general nanoTime should always be used to measure elapsed time. Thanks, David Sincerely yours, Ivan thanks Mandy
Re: RFR: JDK-8038500: (zipfs) Upgrade ZIP provider to be a supported provider
Hi Sherman, Can you confirm that the intent is that this remains only available in a full JRE and not in any Compact Profile? Thanks, David On 15/04/2014 1:48 PM, Xueming Shen wrote: Hi, webrev has been updated to use the name space jdk.nio.zipfs. http://cr.openjdk.java.net/~sherman/8038500/webrev/ -Sherman On 4/14/14 12:15 PM, Xueming Shen wrote: On 4/14/14 11:56 AM, Alan Bateman wrote: On 14/04/2014 18:52, Xueming Shen wrote: Thanks Mandy and Alan for the review. webrev has been updated accordingly to (1) Removed the basic.sh. The tests are now using test.jdk to access the zipfs.jar directly (2) Updated most of the class to package package, only ZipFileSystemProvider (the spi interface) and ZipInfo are public. The ZipInfo is a handy utility sometime I find it useful to simply do java com.sun.nio.zipfs.Info xyz.zip to display all cen and loc tables in details. I may take sometime to find a better home for it later. (3) I have not migrated the test target zipfs.jar to an independent test source (created during test) yet, will definitely later in a separate thread when we move to modules. (4) Yes, I mean to keep Demo there as an interactive regression test. http://cr.openjdk.java.net/~sherman/8038500/webrev/ -Sherman Iris asked me off-list about the name space which makes me wonder if we should use the opportunity to move this to jdk.something. As it's a service provider then nothing should be accessing these classes directly. The only thing that comes to mind is ZipFileAttributeView/ZipFileAttributes where the API provides a type-safe means to access attributes. In the webrev then these are being changed to package-private so I think would break anyone that might be using them anyway. go with jdk.nio.zipfs? I'm fine with that if this is the new directory to go. though it appears we have 1000+ classes at com.sun... -sherman
Re: PING! Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal
Hi Brian, My inclination is if it ain't broke... and AFAICT nothing indicates toString it is particular broken [*], so perhaps just focus on the cleanup aspects and revisit the other when the JMM is updated (and maybe enhanced volatiles is ready)? Paul. [*] although i still question the use of thread locals, but that is a separate issue that requires more investigation and i don't know if if can be improved. On Apr 10, 2014, at 10:45 PM, Brian Burkhalter brian.burkhal...@oracle.com wrote: Second day back from vacation so I guess it’s time to beat this horse again … As there was no response to the message included below I am simply re-posting it. Thanks, Brian On Mar 25, 2014, at 7:19 AM, Brian Burkhalter brian.burkhal...@oracle.com wrote: On Mar 25, 2014, at 1:58 AM, Paul Sandoz paul.san...@oracle.com wrote: This is another example of a stable variable. I would like to re-iterate my scepticism that such changes are necessary in this case (i am not sure if it is possible to create a benchmark that could better exacerbate the concurrent overlap of calls to layoutChars). But, i do agree the discussion has been useful and interesting. I am happy either to leave the toString() code as it is or to change it to the variant with toStringSlow(). There is however other cleanup in the patch to consider. So it would be good to get consensus on the two points: 1) Change toString() to variant using toStringSlow() or leave it as-is. 2) Change non-toString() code as indicated in the patch or leave it as-is. If “as-is” is the answer in both cases, then it’s simply a matter of resolving the enhancement as “not an issue.” Thanks, Brian
Re: RFR [8038982]: java/lang/ref/EarlyTimeout.java failed again
On 15/04/2014 4:54 PM, Daniel Fuchs wrote: Hi guys, Should 'actual' and 'reference' be declared as volatile? I see that they are accessed from main() after joining the threads. Or does joining the threads guarantees that 'main' will see the right values? Yes. If you join() a Thread you are guaranteed to see all writes performed by that Thread before it terminated. (Similarly a newly started Thread sees all writes performed by the Thread that called start() on it.) David best regards, -- daniel On 4/15/14 8:48 AM, David Holmes wrote: On 15/04/2014 4:10 PM, Ivan Gerasimov wrote: On 15.04.2014 6:23, Mandy Chung wrote: On 4/14/2014 11:26 AM, Ivan Gerasimov wrote: Actually, zero tolerance should be sufficient now even for Windows platform. Measuring the time with nanoTime() should make the inner and outer time intervals consistent. I've added the tolerance just to play safer. I can remove it. That'd be even better! Alright, I removed the tolerance back. So now the only change is how the time interval is measured: http://cr.openjdk.java.net/~igerasim/8038982/1/webrev/ This is the right change to make. I'm a little surprised we are seeing these timing problems though. That said the resolution of timed blocking calls and the resolution of currentTimeMillis() can be quite different on any platform, not just windows. In general nanoTime should always be used to measure elapsed time. Thanks, David Sincerely yours, Ivan thanks Mandy
Re: 8020860: cluster Hashtable/Vector field updates for better transactional memory behaviour
On Apr 15, 2014, at 12:54 AM, Mike Duigou mike.dui...@oracle.com wrote: Should we proceed forward despite these understood limitations? My vote is a very soft Yes. I think we can proceed, things are not made any worse for the cases we care about, and i think we can slightly improve things as Martin suggests, plus we can update the documentation on enumerators and iterators. Paul.
Re: i18n dev Improve timezone mapping for AIX platform
Hi Masayoshi, Volker, Thanks a lot for reviewing ! Is it OK for me to push the change into JDK9 directly ? or need another reviewer's approval ? Many thanks Jonathan On Mon, Apr 14, 2014 at 2:03 PM, Masayoshi Okutsu masayoshi.oku...@oracle.com wrote: Looks good to me. Thanks, Masayoshi On 4/11/2014 10:46 PM, Volker Simonis wrote: Hi Jonathan, thank you for fixing all the remaining issues. From my point of view this change looks good now. @Masayoshi: can I please get a final approval from you for pushing the change? I also want to downport this to 8u-dev but I don't think that's a big deal as this only touches AX code. Thank you and best regards, Volker On Thu, Apr 10, 2014 at 11:44 AM, Jonathan Lu luc...@linux.vnet.ibm.comwrote: Hi Volker, Thanks a lot for your comments, I've made another patch, http://cr.openjdk.java.net/~luchsh/JDK-8Masayoshi034220.v4/http://cr.openjdk.java.net/%7Eluchsh/JDK-8034220.v4/ On Fri, Apr 4, 2014 at 9:22 PM, Volker Simonis volker.simo...@gmail.comwrote: Hi Jonathan, sorry, but I found a few more issues: - please use strncpy instead of strcpy in TimeZone_md.c:798 otherwise somebody could easily crash the VM by specifying a TZ with more than 100 characters. Right, I've fix it by using strncpy, and also updated another usage of strcmp(). - you can delete the lines 871-872 because the variables are actually not used and you can also remove the handling for timezone == 0 because that is actually done in getGMTOffsetID() anyway. Nice catch, have deleted those in latest patch. - could you please avoid the usage of strtok. It is not intended for usage in a multithreaded environment (see for example man strtok on AIX). strtok_r is probably overkill, but you could use for example strchr. I've changed it to strtok_r in this patch, although strtok was only used once here, it may still impact other threads. did you check the build on Windows? Yes, both patches got built on Windows. Thank you and best regards, Volker On Fri, Apr 4, 2014 at 10:18 AM, Jonathan Lu luc...@linux.vnet.ibm.com wrote: Hi Volker, Masayoshi, I made another patch which fixed the problems mentioned in last mail, http://cr.openjdk.java.net/~luchsh/JDK-8034220.v3/ Could you pls help to take a look? Many thanks Jonathan On Thu, Apr 3, 2014 at 12:34 AM, Jonathan Lu luc...@linux.vnet.ibm.com wrote: Hi Volker, On 2014年04月02日 23:07, Volker Simonis wrote: Hi Jonathan, thanks for updating the change. Please find my comments inline: On Tue, Apr 1, 2014 at 4:52 PM, Jonathan Lu luc...@linux.vnet.ibm.com wrote: Hi Volker, Masayoshi, Thanks a lot for your review, here's the updated patch, could you pls take a look? http://cr.openjdk.java.net/~luchsh/JDK-8034220.v2/ On Thu, Mar 27, 2014 at 1:48 AM, Volker Simonis volker.simo...@gmail.com wrote: Hi Jonathan, thanks for doing this change. Please find some comments below: 1. please update the copyright year to 2014 in every file you touch Updated in new patch. 2. in CopyFiles.gmk you can simplify the change by joining the windows and aix cases because on Windows OPENJDK_TARGET_OS is the same like OPENJDK_TARGET_OS_API_DIR. So you can write: ifneq ($(findstring $(OPENJDK_TARGET_OS), windows aix), ) TZMAPPINGS_SRC := $(JDK_TOPDIR)/src/$(OPENJDK_TARGET_OS)/lib $(LIBDIR)/tzmappings: $(TZMAPPINGS_SRC)/tzmappings $(call install-file) COPY_FILES += $(LIBDIR)/tzmappings endif I've tried that approach before, but OPENJDK_TARGET_OS_API_DIR is 'solaris' as I observed on my AIX box, solaris/lib is not the directory where tzmappings was stored for AIX. So I'm keeping this change, please fix me if I was wrong about the config. Yes, but my point was to actually use OPENJDK_TARGET_OS for the construction of TZMAPPINGS_SRC as shown in the code snippet above. OPENJDK_TARGET_OS is windows on Windows platforms and aix on AIX and that should be just enough here. That's right, let me try that and also build/test on Windows platform. 3. regarding the changes proposed by Masayoshi: I agree that we should put the AIX timezone mapping code in its own function, but I don't think that getPlatformTimeZoneID() would be the right place. As far as I understand, getPlatformTimeZoneID() is used to get a platform time zone id if the environment variable TZ is not set. I don't know a way how this could be done on AIX (@Jonathan: maybe you know?). If there would be a way to get the time zone from some system files or so, then we should put this code into the AIX version of getPlatformTimeZoneID(). I guess we may try to use /etc/envrionment file, which is basic setting for all processes, see http://publib.boulder.ibm.com/infocenter/aix/v7r1/index.jsp?topic=%2Fcom.ibm.aix.files%2Fdoc%2Faixfiles%2Fenvironment.htm
Re: RFR: JDK-8038500: (zipfs) Upgrade ZIP provider to be a supported provider
Hello Sherman,
The build changes look good to me.
/Erik
On 2014-04-14 19:52, Xueming Shen wrote:
Thanks Mandy and Alan for the review.
webrev has been updated accordingly to
(1) Removed the basic.sh. The tests are now using test.jdk to access
the zipfs.jar directly
(2) Updated most of the class to package package, only
ZipFileSystemProvider (the spi interface)
and ZipInfo are public.
The ZipInfo is a handy utility sometime I find it useful to
simply do java com.sun.nio.zipfs.Info xyz.zip
to display all cen and loc tables in details. I may take sometime
to find a better home for it later.
(3) I have not migrated the test target zipfs.jar to an independent
test source (created during test) yet,
will definitely later in a separate thread when we move to modules.
(4) Yes, I mean to keep Demo there as an interactive regression test.
http://cr.openjdk.java.net/~sherman/8038500/webrev/
-Sherman
On 4/11/14 4:29 PM, Mandy Chung wrote:
On 4/11/2014 3:42 PM, Xueming Shen wrote:
webrev: http://cr.openjdk.java.net/~sherman/8038500/webrev
It's good to see the source of the zip provider moved to the jdk
repo. You have made some public classes to package-private which is
good. I wonder whether a few remaining public classes can be made
package-private (e.g. ZipFileAttributes, ZipInfo etc). Is
JarFileSystemProvider used anywhere? ZipFileSystemProvider is the
only provider listed in the service configuration.
Do you mean to make Demo.java as a regression test?
basic.sh - I wonder if you can get rid of this shell test. Do Basic,
PathOps, ZipFSTester have any relationship? I was thinking if they
can be made as individual java test and constructor the input path of
zipfs.jar.With zipfs.jar part of the jdk build, it will be found
under ${java.home}/lib/ext/zipfs.jar. When we move to modules, this
jar file won't exist. It might be better for the tests to create a
JAR file (one to share by all zipfs tests) to get ready for modules.
Mandy
Covariant overrides on the Buffer Hierachy
Hi, I'd like to have a discussion about tidying up a few core library method signatures in a way that (I think) is backwards compatible. I've been using ByteBuffer quite a lot recently which is designed to be a fluent API. Unfortunately its quite inconvenient to use because there's a hierarchy of classes at play (MappedByteBuffer, ByteBuffer and Buffer) and methods which are inherited from parent classes aren't overridden with covariant return types. For example the following code doesn't compile because clear is defined on Buffer and putInt is defined on ByteBuffer. ByteBuffer buffer = ByteBuffer.allocate(256); ByteBuffer other = buffer.duplicate() .clear() .putInt(0, 4); If clear was overridden in ByteBuffer with ByteBuffer as its return type then this would compile. Now I can appreciate that these classes were introduced in Java 1.4, which was before covariant overriding was possible, but I was wondering what the situation was with fixing it. To my mind, and by all means correct me if I've overlooked something, changing these methods to return their own type rather than the parent type should be backwards compatible at the source and binary levels. The only issue that I'm aware of that is related to this kind of change is the requirement to recompile all the classes in the hierarchy when making a change [0]. If you don't do this its possible for an infinite recursion and eventual stackoverflow to occur. Now as far as I can tell the entire hierarchy of Buffer classes are all either package scoped classes, or they are public classes with package scoped constructors. So I'm not aware of a way to sub class them without the code being part of the JDK. This would mean that the subclasses all need to be recompiled, satisfying the safety criteria. I'd be most appreciative of any feedback on this issue. regards, Richard Warburton http://insightfullogic.com @RichardWarburto http://twitter.com/richardwarburto [0] http://mail.openjdk.java.net/pipermail/core-libs-dev/2012-January/009119.html (thanks to Stuart Marks for pointing this one out to me)
Re: Covariant overrides on the Buffer Hierachy
On 15/04/2014 09:05, Richard Warburton wrote: : The only issue that I'm aware of that is related to this kind of change is the requirement to recompile all the classes in the hierarchy when making a change [0]. If you don't do this its possible for an infinite recursion and eventual stackoverflow to occur. Now as far as I can tell the entire hierarchy of Buffer classes are all either package scoped classes, or they are public classes with package scoped constructors. So I'm not aware of a way to sub class them without the code being part of the JDK. This would mean that the subclasses all need to be recompiled, satisfying the safety criteria. This is a long standing bug to look at this issue [1]. CharBuffer.subSequence was one case that was retrofitted in 7. As you note, buffers are not designed to be extended outside of the java.nio package so this should make a number of options possible. -Alan [1] https://bugs.openjdk.java.net/browse/JDK-4774077
Re: i18n dev Improve timezone mapping for AIX platform
You can push it to jdk9-dev directly. No other reviewer is required. Regards, Volker On Tue, Apr 15, 2014 at 9:48 AM, Jonathan Lu luc...@linux.vnet.ibm.com wrote: Hi Masayoshi, Volker, Thanks a lot for reviewing ! Is it OK for me to push the change into JDK9 directly ? or need another reviewer's approval ? Many thanks Jonathan On Mon, Apr 14, 2014 at 2:03 PM, Masayoshi Okutsu masayoshi.oku...@oracle.com wrote: Looks good to me. Thanks, Masayoshi On 4/11/2014 10:46 PM, Volker Simonis wrote: Hi Jonathan, thank you for fixing all the remaining issues. From my point of view this change looks good now. @Masayoshi: can I please get a final approval from you for pushing the change? I also want to downport this to 8u-dev but I don't think that's a big deal as this only touches AX code. Thank you and best regards, Volker On Thu, Apr 10, 2014 at 11:44 AM, Jonathan Lu luc...@linux.vnet.ibm.com wrote: Hi Volker, Thanks a lot for your comments, I've made another patch, http://cr.openjdk.java.net/~luchsh/JDK-8Masayoshi034220.v4/ On Fri, Apr 4, 2014 at 9:22 PM, Volker Simonis volker.simo...@gmail.com wrote: Hi Jonathan, sorry, but I found a few more issues: - please use strncpy instead of strcpy in TimeZone_md.c:798 otherwise somebody could easily crash the VM by specifying a TZ with more than 100 characters. Right, I've fix it by using strncpy, and also updated another usage of strcmp(). - you can delete the lines 871-872 because the variables are actually not used and you can also remove the handling for timezone == 0 because that is actually done in getGMTOffsetID() anyway. Nice catch, have deleted those in latest patch. - could you please avoid the usage of strtok. It is not intended for usage in a multithreaded environment (see for example man strtok on AIX). strtok_r is probably overkill, but you could use for example strchr. I've changed it to strtok_r in this patch, although strtok was only used once here, it may still impact other threads. did you check the build on Windows? Yes, both patches got built on Windows. Thank you and best regards, Volker On Fri, Apr 4, 2014 at 10:18 AM, Jonathan Lu luc...@linux.vnet.ibm.com wrote: Hi Volker, Masayoshi, I made another patch which fixed the problems mentioned in last mail, http://cr.openjdk.java.net/~luchsh/JDK-8034220.v3/ Could you pls help to take a look? Many thanks Jonathan On Thu, Apr 3, 2014 at 12:34 AM, Jonathan Lu luc...@linux.vnet.ibm.com wrote: Hi Volker, On 2014年04月02日 23:07, Volker Simonis wrote: Hi Jonathan, thanks for updating the change. Please find my comments inline: On Tue, Apr 1, 2014 at 4:52 PM, Jonathan Lu luc...@linux.vnet.ibm.com wrote: Hi Volker, Masayoshi, Thanks a lot for your review, here's the updated patch, could you pls take a look? http://cr.openjdk.java.net/~luchsh/JDK-8034220.v2/ On Thu, Mar 27, 2014 at 1:48 AM, Volker Simonis volker.simo...@gmail.com wrote: Hi Jonathan, thanks for doing this change. Please find some comments below: 1. please update the copyright year to 2014 in every file you touch Updated in new patch. 2. in CopyFiles.gmk you can simplify the change by joining the windows and aix cases because on Windows OPENJDK_TARGET_OS is the same like OPENJDK_TARGET_OS_API_DIR. So you can write: ifneq ($(findstring $(OPENJDK_TARGET_OS), windows aix), ) TZMAPPINGS_SRC := $(JDK_TOPDIR)/src/$(OPENJDK_TARGET_OS)/lib $(LIBDIR)/tzmappings: $(TZMAPPINGS_SRC)/tzmappings $(call install-file) COPY_FILES += $(LIBDIR)/tzmappings endif I've tried that approach before, but OPENJDK_TARGET_OS_API_DIR is 'solaris' as I observed on my AIX box, solaris/lib is not the directory where tzmappings was stored for AIX. So I'm keeping this change, please fix me if I was wrong about the config. Yes, but my point was to actually use OPENJDK_TARGET_OS for the construction of TZMAPPINGS_SRC as shown in the code snippet above. OPENJDK_TARGET_OS is windows on Windows platforms and aix on AIX and that should be just enough here. That's right, let me try that and also build/test on Windows platform. 3. regarding the changes proposed by Masayoshi: I agree that we should put the AIX timezone mapping code in its own function, but I don't think that getPlatformTimeZoneID() would be the right place. As far as I understand, getPlatformTimeZoneID() is used to get a platform time zone id if the environment variable TZ is not set. I don't know a way how this could be done on AIX (@Jonathan: maybe you know?). If there would be a way to get the time zone from some system files or so, then we should put this code into the AIX version of getPlatformTimeZoneID(). I guess we may try to use /etc/envrionment file, which is basic setting for all processes, see
Re: RFR: [8039396] NPE when writing a class descriptor object to a custom ObjectOutputStream
On 11/04/2014 09:01, Ivan Gerasimov wrote: Hello everybody! ObjectStreamClass#forClass() function is allowed to return null, if the local VM does not have the corresponding local class. Because of that, NPE can be encountered during serialization through a subclass of ObjectOutputStream. Would you please help review the fix? BUGURL: https://bugs.openjdk.java.net/browse/JDK-8039396 WEBREV: http://cr.openjdk.java.net/~igerasim/8039396/0/webrev/ The fix looks okay to me, a bit of a corner case but okay. The test looks useful. A minor suggestion is to shorten the lines a bit to make future side-by-side reviews easier. -Alan.
Re: RFR: JDK-8038500: (zipfs) Upgrade ZIP provider to be a supported provider
On 15/04/2014 04:48, Xueming Shen wrote: Hi, webrev has been updated to use the name space jdk.nio.zipfs. http://cr.openjdk.java.net/~sherman/8038500/webrev/ Thanks, I think this makes sense. For further down the road then consideration could be given to make ZipFileAttributes/ZipFileAttributeView public and supported so as to provide type safe access to zip specific attributes. JarFileSystemProvider will also need discussion at some point to decide whether it should be added to the service configuration file so that JAR URLs are supported. -Alan. **
Re: RFR: [8039396] NPE when writing a class descriptor object to a custom ObjectOutputStream
Thank you Alan! I'll reformat the test before pushing to make it fit into the half of the screen. Sincerely yours, Ivan On 15.04.2014 12:56, Alan Bateman wrote: On 11/04/2014 09:01, Ivan Gerasimov wrote: Hello everybody! ObjectStreamClass#forClass() function is allowed to return null, if the local VM does not have the corresponding local class. Because of that, NPE can be encountered during serialization through a subclass of ObjectOutputStream. Would you please help review the fix? BUGURL: https://bugs.openjdk.java.net/browse/JDK-8039396 WEBREV: http://cr.openjdk.java.net/~igerasim/8039396/0/webrev/ The fix looks okay to me, a bit of a corner case but okay. The test looks useful. A minor suggestion is to shorten the lines a bit to make future side-by-side reviews easier. -Alan.
Core Libs Dev[9] Review Request for 8030709: Tidy warnings cleanup for java.lang package
Sorry, the webrev was updated to include one other minor change in javax/script/ScriptEngineFactory.java to avoid creation of separate review request: http://cr.openjdk.java.net/~yan/8030709/webrev.01/ Regards, Alexander On 10.04.2014 12:24, alexander stepanov wrote: BigInteger.java from java.lang was also touched. sorry, from java.math On 10.04.2014 12:23, alexander stepanov wrote: Hello, Could you please review the fix for the following bug: https://bugs.openjdk.java.net/browse/JDK-8030709 Webrev corresponding: http://cr.openjdk.java.net/~yan/8030709/webrev.00/ Just a minor cleanup of javadoc to avoid tidy warnings; no other code affected. BigInteger.java from java.lang was also touched. Thanks. Regards, Alexander
Re: Core Libs Dev[9] Review Request for 8030709: Tidy warnings cleanup for java.lang package
looks ok On Apr 15, 2014, at 9:11 AM, alexander stepanov alexander.v.stepa...@oracle.com wrote: Sorry, the webrev was updated to include one other minor change in javax/script/ScriptEngineFactory.java to avoid creation of separate review request: http://cr.openjdk.java.net/~yan/8030709/webrev.01/ Regards, Alexander On 10.04.2014 12:24, alexander stepanov wrote: BigInteger.java from java.lang was also touched. sorry, from java.math On 10.04.2014 12:23, alexander stepanov wrote: Hello, Could you please review the fix for the following bug: https://bugs.openjdk.java.net/browse/JDK-8030709 Webrev corresponding: http://cr.openjdk.java.net/~yan/8030709/webrev.00/ Just a minor cleanup of javadoc to avoid tidy warnings; no other code affected. BigInteger.java from java.lang was also touched. Thanks. Regards, Alexander 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: Core Libs Dev[9] Review Request for 8030709: Tidy warnings cleanup for java.lang package
thanks! On 15.04.2014 17:24, Lance Andersen wrote: looks ok On Apr 15, 2014, at 9:11 AM, alexander stepanov alexander.v.stepa...@oracle.com mailto:alexander.v.stepa...@oracle.com wrote: Sorry, the webrev was updated to include one other minor change in javax/script/ScriptEngineFactory.java to avoid creation of separate review request: http://cr.openjdk.java.net/~yan/8030709/webrev.01/ http://cr.openjdk.java.net/%7Eyan/8030709/webrev.01/ Regards, Alexander On 10.04.2014 12:24, alexander stepanov wrote: BigInteger.java from java.lang was also touched. sorry, from java.math On 10.04.2014 12:23, alexander stepanov wrote: Hello, Could you please review the fix for the following bug: https://bugs.openjdk.java.net/browse/JDK-8030709 Webrev corresponding: http://cr.openjdk.java.net/~yan/8030709/webrev.00/ Just a minor cleanup of javadoc to avoid tidy warnings; no other code affected. BigInteger.java from java.lang was also touched. Thanks. Regards, Alexander Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 http://oracle.com/us/design/oracle-email-sig-198324.giflance.ander...@oracle.com mailto:lance.ander...@oracle.com
Re: PING! Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal
Hi Paul, On Apr 15, 2014, at 12:23 AM, Paul Sandoz paul.san...@oracle.com wrote: Hi Brian, My inclination is if it ain't broke... and AFAICT nothing indicates toString it is particular broken [*], so perhaps just focus on the cleanup aspects and revisit the other when the JMM is updated (and maybe enhanced volatiles is ready)? Paul. Fine by me. I’ll update the webrev. The other aspects could come under this issue already on file: https://bugs.openjdk.java.net/browse/JDK-8033491. Thanks, Brian [*] although i still question the use of thread locals, but that is a separate issue that requires more investigation and i don't know if if can be improved.
Re: RFR: JDK-8038500: (zipfs) Upgrade ZIP provider to be a supported provider
Hi David, yes, it's the intent to make it only available in full jre, for now. Thanks! -Sherman On 4/15/14 12:22 AM, David Holmes wrote: Hi Sherman, Can you confirm that the intent is that this remains only available in a full JRE and not in any Compact Profile? Thanks, David On 15/04/2014 1:48 PM, Xueming Shen wrote: Hi, webrev has been updated to use the name space jdk.nio.zipfs. http://cr.openjdk.java.net/~sherman/8038500/webrev/ -Sherman On 4/14/14 12:15 PM, Xueming Shen wrote: On 4/14/14 11:56 AM, Alan Bateman wrote: On 14/04/2014 18:52, Xueming Shen wrote: Thanks Mandy and Alan for the review. webrev has been updated accordingly to (1) Removed the basic.sh. The tests are now using test.jdk to access the zipfs.jar directly (2) Updated most of the class to package package, only ZipFileSystemProvider (the spi interface) and ZipInfo are public. The ZipInfo is a handy utility sometime I find it useful to simply do java com.sun.nio.zipfs.Info xyz.zip to display all cen and loc tables in details. I may take sometime to find a better home for it later. (3) I have not migrated the test target zipfs.jar to an independent test source (created during test) yet, will definitely later in a separate thread when we move to modules. (4) Yes, I mean to keep Demo there as an interactive regression test. http://cr.openjdk.java.net/~sherman/8038500/webrev/ -Sherman Iris asked me off-list about the name space which makes me wonder if we should use the opportunity to move this to jdk.something. As it's a service provider then nothing should be accessing these classes directly. The only thing that comes to mind is ZipFileAttributeView/ZipFileAttributes where the API provides a type-safe means to access attributes. In the webrev then these are being changed to package-private so I think would break anyone that might be using them anyway. go with jdk.nio.zipfs? I'm fine with that if this is the new directory to go. though it appears we have 1000+ classes at com.sun... -sherman
Re: JDK 9 RFR of 8039474: sun.misc.CharacterDecoder.decodeBuffer should use getBytes(iso8859-1)
Am 10.04.2014 21:53, schrieb Tim Bell: On 04/10/14 19:26, Ulf Zibis wrote: BTW, where are these links gone: This part of the question I can handle. The six digit Bug numbers came from the legacy OpenJDK bugzilla instance. Before it was shut down, those bug reports were transferred to JBS. In the process, they were assigned new JDK-nnn bug numbers, so you will be able to view them on the new system: In general, if you saved an old Bugzilla ID (six digits, for example 100092), you should be able to find it in JBS by visiting this URL: https://bugs.openjdk.java.net/issues/?jql= And doing a simple search for the string id=100092 Hope this helps- Much thanks Tim. But where are the original attachments e.g. webrevs, patches ? Are they lost forever ? -Ulf
Re: RFR: JDK-8038500: (zipfs) Upgrade ZIP provider to be a supported provider
On 4/14/14 8:48 PM, Xueming Shen wrote: Hi, webrev has been updated to use the name space jdk.nio.zipfs. http://cr.openjdk.java.net/~sherman/8038500/webrev/ This patch looks fine. JarFileSystemProvider is currently not used and it's fine to include it as it will need discussion to decide if this should be added in the service configuration file as Alan suggests. thanks Mandy
Re: Covariant overrides on the Buffer Hierachy
Hi, Am 15.04.2014 10:05, schrieb Richard Warburton: Hi, I'd like to have a discussion about tidying up a few core library method signatures in a way that (I think) is backwards compatible. I've been using ByteBuffer quite a lot recently which is designed to be a fluent API. Unfortunately its quite inconvenient to use because there's a hierarchy of classes at play (MappedByteBuffer, ByteBuffer and Buffer) and methods which are inherited from parent classes aren't overridden with covariant return types. For example the following code doesn't compile because clear is defined on Buffer and putInt is defined on ByteBuffer. ByteBuffer buffer = ByteBuffer.allocate(256); ByteBuffer other = buffer.duplicate() .clear() .putInt(0, 4); If clear was overridden in ByteBuffer with ByteBuffer as its return type then this would compile. See also: http://mail.openjdk.java.net/pipermail/core-libs-dev/2009-April/001512.html http://mail.openjdk.java.net/pipermail/core-libs-dev/2009-April/001365.html http://mail.openjdk.java.net/pipermail/coin-dev/2009-March/001180.html https://bugs.openjdk.java.net/browse/JDK-6472931 https://bugs.openjdk.java.net/browse/JDK-6373386 https://bugs.openjdk.java.net/browse/JDK-6479372 https://bugs.openjdk.java.net/browse/JDK-4774077 https://bugs.openjdk.java.net/browse/JDK-6451131 https://bugs.openjdk.java.net/browse/JDK-5082736 -Ulf
Re: String.indexOf optimization
Hi everyone!
On 04.04.2014 21:13, Martin Buchholz wrote:
Summary:
Many people (myself included) have looked at this problem. It's unlikely
that String.indexOf will change. It's hard to beat the naive
implementation in the typical case.
But we can try to speed up this naive implementation a little bit.
We can separate the special case: When the substring's length == 1.
This special case can be done fast, and in the general case we can now
assume substring's length is at least 2.
Here's the webrev with the implementation of this idea:
http://cr.openjdk.java.net/~igerasim/indexof/0/webrev/
I've done some benchmarking with JMH and found no performance
degradation on my machine.
Of course, more testcases should be created and they should be tried on
different machines to be treated as reliable.
Benchmark Mode ThrCnt Sec Mean
Mean errorUnits
o.b.IndexOfBench.benchIndexOf_1_A avgt 1 20 5
704.7399.104 nsec/op
o.b.IndexOfBench.benchIndexOf_1_B avgt 1 20 5 *573.879*
9.820 nsec/op
o.b.IndexOfBench.benchIndexOf_2_A avgt 1 20 5
668.4559.882 nsec/op
o.b.IndexOfBench.benchIndexOf_2_B avgt 1 20 5 *476.062*
6.063 nsec/op
o.b.IndexOfBench.benchIndexOf_3_A avgt 1 20 5
155.2271.796 nsec/op
o.b.IndexOfBench.benchIndexOf_3_B avgt 1 20 5 *152.850 *
1.512 nsec/op
o.b.IndexOfBench.benchIndexOf_4_A avgt 1 20 5
656.1835.904 nsec/op
o.b.IndexOfBench.benchIndexOf_4_B avgt 1 20 5 *515.178*
9.107 nsec/op
o.b.IndexOfBench.benchIndexOf_5_A avgt 1 20 5
140.6097.305 nsec/op
o.b.IndexOfBench.benchIndexOf_5_B avgt 1 20 5 *129.603*
1.654 nsec/op
o.b.IndexOfBench.benchIndexOf_6_A avgt 1 20 5
127.7131.497 nsec/op
o.b.IndexOfBench.benchIndexOf_6_B avgt 1 20 5 *122.177*
1.186 nsec/op
o.b.IndexOfBench.benchIndexOf_7_A avgt 1 20 5
430.1484.875 nsec/op
o.b.IndexOfBench.benchIndexOf_7_B avgt 1 20 5 *387.338*
10.904 nsec/op
o.b.IndexOfBench.benchIndexOf_8_A avgt 1 20 5
2064.563 28.885 nsec/op
o.b.IndexOfBench.benchIndexOf_8_B avgt 1 20 5 *1858.669*
24.343 nsec/op
Benchmarks ending with A use the current indexOf implementation, with B
use the modified version.
These numbers show from 1.4% up to 28% performance increase.
The full listing is below.
Suggestions about what else to test are welcome!
Sincerely yours,
Ivan
-
/**
* Copyright (c) 2014, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation. Oracle designates this
* particular file as subject to the Classpath exception as provided
* by Oracle in the LICENSE file that accompanied this code.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License
version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/
package org.benches;
import org.openjdk.jmh.annotations.*;
import org.openjdk.jmh.logic.BlackHole;
import java.util.concurrent.TimeUnit;
@BenchmarkMode(Mode.AverageTime)
@OutputTimeUnit(TimeUnit.NANOSECONDS)
@State(Scope.Benchmark)
public class IndexOfBench {
// |||
final static char[] source1 =
abababcabcacabcabcabcabcaccbcabcacbcabcacbcbcabcbcbacbcabcbabacbcbacbcabcabcabcabcabcabcabcacbacbacbacabcabcacbacbcabcbcbcaabdacbacabcabacbacabca.toCharArray();
final static char[] source2 =
acfacafacfacfacfacafcafcacadcacdacaccacacdacacfcafcafcfacdacadcadcadcdacfacfacdacadcacdcfacfacdacdacdcfacdacdacdacgshgshasdabdahghjgwdshacavcavsc.toCharArray();
final static char[] source3 =
tyrtytfytytuytfytuytggfghgdytyftytfdytdshfgjhdfytsfuythgsfhgjhfghtuysdfthgfsdhgystfjhgsfguysthgfgjhgdfjhgsjdghfythgsdfjhggfabduikjhfjhkjhfkjhgkjh.toCharArray();
final static char[] target1 = abd.toCharArray();
final static char[] source4 =
ahhhahahahahhahahahahhahahahhhahahahahahahahahahahhahahahahahahahahallallalalalalalkakakakakakakakakkakmamamamabammamamamamamaakaklalalaoalalalao.toCharArray();
final static char[] source5 =
Re: JDK 9 RFR of 8039474: sun.misc.CharacterDecoder.decodeBuffer should use getBytes(iso8859-1)
On 04/15/14 16:47, Ulf Zibis wrote: But where are the original attachments e.g. webrevs, patches ? Are they lost forever ? No, they are there on the new JBS bug reports. For some reason they are not visible to users outside Oracle. I will see if that can be changed. Regards- Tim
Re: PING! Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal
On Apr 15, 2014, at 9:24 AM, Brian Burkhalter brian.burkhal...@oracle.com wrote: On Apr 15, 2014, at 12:23 AM, Paul Sandoz paul.san...@oracle.com wrote: My inclination is if it ain't broke... and AFAICT nothing indicates toString it is particular broken [*], so perhaps just focus on the cleanup aspects and revisit the other when the JMM is updated (and maybe enhanced volatiles is ready)? I updated the patch to remove the toString() changes: http://cr.openjdk.java.net/~bpb/6375303/webrev.02/ Unless there are objections within the next 24 hours or so I will push this as-is. Fine by me. I’ll update the webrev. The other aspects could come under this issue already on file:https://bugs.openjdk.java.net/browse/JDK-8033491. I linked this issue to 6375303 and included links to the archive discussion thread. Thanks, Brian
Re: 8020860: cluster Hashtable/Vector field updates for better transactional memory behaviour
On Apr 14 2014, at 18:25 , Martin Buchholz marti...@google.com wrote:
I'll retreat to being neutral on the overall idea.
In general, it *is* a best software engineering practice to do all the
reading and computing before doing all the writing at the end.
You'll break anyone who does the crazy thing of intentionally calling
add(null, Integer.MAX_VALUE) just for the side effect of incrementing
modCount. How would _you_ increment modCount without doing any modding?!
Assuming you meant Vector::put(Integer.MAX_VALUE, null). You could use
synchronized(vector) { vector.removeRange(vector.size(), vector.size()) } with
slight higher cost.
You could make a real improvement (more concurrency) for addAll, by moving
the call to toArray out of the synchronized block.
public synchronized boolean addAll(Collection? extends E c) {
-modCount++;
Object[] a = c.toArray();
Done!
It's hardly worth it for e.g. clear, where you are doing nothing but writes
in the loop as well.
public synchronized void clear() {
Entry?,? tab[] = table;
-modCount++;
for (int index = tab.length; --index = 0; )
tab[index] = null;
+modCount++;
count = 0;
}
For consistency with other methods I retained the movement of modCount.
I have updated the webrev with what I hope is the final form:
http://cr.openjdk.java.net/~mduigou/JDK-8020860/1/webrev/
Good to go?
Mike
PS:- I would have liked to rewritten Hashtable::putAll(Map t)
as :
{
t.forEach(this::put)
}
but decided against this since it may change the synchronization on t which
seems risky. ie. some Thread could hold a lock on t which would now deadlock
where the current implementation does not. The forEach() implementation would
have advantages as for some cases it would avoid the need to create Map.Entry
objects in the iterator.
