Re: RFR JDK-6321472: Add CRC-32C API

On 10/21/2014 11:34 PM, Staffan Friberg wrote: I believe it must be <, as it is in the tail loop as well, because end is (off+len or limit) so end is exclusive, similar to subString(begin,end). Makes sense? //Staffan On 10/21/2014 01:46 PM, Peter Levart wrote: Sorry Staffan, another nit...

Re: RFR: 8060130: Simplify the synchronization of defining and getting java.lang.Package

Hi Claes, In the test you should use the ProcessTools from the testlibrary not mess with System.getProperty("java.home") and create your own processes directly. The test should not require a full JDK to run, but should execute on a JRE or any compact profile. Thanks, David On 21/10/2014 11:

Re: RFR JDK-6321472: Add CRC-32C API

Thanks for the review. Updated the @implSpec. Also includes Peter's good catch. Webrev: http://cr.openjdk.java.net/~sfriberg/JDK-6321472/webrev.06 //Staffan On 10/21/2014 02:09 PM, Xueming Shen wrote: Staffan, Thanks for the package.html update. Just wonder if it would be better to use

Re: RFR JDK-6321472: Add CRC-32C API

I believe it must be <, as it is in the tail loop as well, because end is (off+len or limit) so end is exclusive, similar to subString(begin,end). Makes sense? //Staffan On 10/21/2014 01:46 PM, Peter Levart wrote: Sorry Staffan, another nit... 212 for (; off < (end - Long.BYTES

Re: RFR JDK-6321472: Add CRC-32C API

Hi Peter, Thanks for catching! I would like to keep to 8 since the crc32c instruction on SPARC will require it, the plan is to add an intrinsic for it. Thanks, Staffan On 10/21/2014 01:30 PM, Peter Levart wrote: On 10/21/2014 08:49 PM, Staffan Friberg wrote: Hi, Got an offline comment th

Re: RFR: 8060130: Simplify the synchronization of defining and getting java.lang.Package

Hello, one thing I wonder - in the old and in the new case there is nothing in definePackage() guaring the parents from getting to know the new package and causing a conflict (as the atomic insert is only on the specific packages table. Are those (parent and system package) immutable? Gruss Bernd

Re: RFR JDK-6321472: Add CRC-32C API

Staffan, Thanks for the package.html update. Just wonder if it would be better to use buffer.remaining(), instead of the buffer.limit() - buffer.position() in Checksum.udpate(ByteBuffer)'s #implSpec The rest looks fine for me. -Sherman On 10/21/2014 01:11 PM, Staffan Friberg wrote: Con

Re: RFR (M) 8061651 - Interface to the Lookup Index Cache to improve URLClassPath search time

Hi Ioi, Here are some comments from me: -src/share/vm/classfile/classLoader.cpp In ClassLoader::setup_search_path, if canonicalize is true it's not necessary to allocate the 'path' and copy the 'class_path' to 'path'. -src/share/vm/memory/metadataFactory.hpp In free_array, is it possible to d

Re: RFR JDK-6321472: Add CRC-32C API

On 10/21/2014 01:11 PM, Staffan Friberg wrote: Converted. http://cr.openjdk.java.net/~sfriberg/JDK-6321472/webrev.05 Conversion looks fine; thanks, -Joe //Staffan On 10/21/2014 12:34 PM, Joe Darcy wrote: Hi Staffan, If you are updating package.html, please also hg mv the file to be a p

Re: RFR JDK-6321472: Add CRC-32C API

Sorry Staffan, another nit... 212 for (; off < (end - Long.BYTES); off += Long.BYTES) { and 286 for (; off < (end - Long.BYTES); off += Long.BYTES) { I think you could change "off < (end - Long.BYTES)" to "off <= (end - Long.BYTES)". Am I right? Regards, Peter

Re: RFR JDK-6321472: Add CRC-32C API

On 10/21/2014 08:49 PM, Staffan Friberg wrote: Hi, Got an offline comment that the package.html should be update as well to cover CRC-32C. Otherwise there are no code changes in this new webrev. http://cr.openjdk.java.net/~sfriberg/JDK-6321472/webrev.04 Hi Staffan, 198 if (end

Re: RFR (JAXP): 8061686: Size limits in BufferAllocator should have been final

Thanks! Joe On 10/21/2014 12:27 PM, Lance Andersen wrote: +1 On Oct 21, 2014, at 3:09 PM, huizhe wang > wrote: Hi, The three fields in com.sun.xml.internal.stream.util.BufferAllocator are used internally only, should have been private and final. https://bug

Re: RFR JDK-6321472: Add CRC-32C API

Converted. http://cr.openjdk.java.net/~sfriberg/JDK-6321472/webrev.05 //Staffan On 10/21/2014 12:34 PM, Joe Darcy wrote: Hi Staffan, If you are updating package.html, please also hg mv the file to be a package-info.java file with the equivalent javadoc. Thanks, -Joe On 10/21/2014 11:49 A

Re: RFR (M) 8061651 - Interface to the Lookup Index Cache to improve URLClassPath search time

Hi Ioi, On 10/21/14 10:27 AM, Ioi Lam wrote: Please review this fix: http://cr.openjdk.java.net/~iklam/8061651-lookup-index-open-v1/ This looks better than the preliminary webrev you sent me offline earlier. I only review the jdk repo change. Launcher.java line 290: it can be made a final

Re: RFR JDK-6321472: Add CRC-32C API

Hi Staffan, If you are updating package.html, please also hg mv the file to be a package-info.java file with the equivalent javadoc. Thanks, -Joe On 10/21/2014 11:49 AM, Staffan Friberg wrote: Hi, Got an offline comment that the package.html should be update as well to cover CRC-32C. Ot

Re: RFR (JAXP): 8061686: Size limits in BufferAllocator should have been final

+1 On Oct 21, 2014, at 3:09 PM, huizhe wang wrote: > Hi, > > The three fields in com.sun.xml.internal.stream.util.BufferAllocator are used > internally only, should have been private and final. > > https://bugs.openjdk.java.net/browse/JDK-8061686 > http://cr.openjdk.java.net/~joehw/jdk9/806168

RFR (JAXP): 8061686: Size limits in BufferAllocator should have been final

Hi, The three fields in com.sun.xml.internal.stream.util.BufferAllocator are used internally only, should have been private and final. https://bugs.openjdk.java.net/browse/JDK-8061686 http://cr.openjdk.java.net/~joehw/jdk9/8061686/webrev/ Tests passed. Thanks, Joe

Re: RFR JDK-6321472: Add CRC-32C API

Hi, Got an offline comment that the package.html should be update as well to cover CRC-32C. Otherwise there are no code changes in this new webrev. http://cr.openjdk.java.net/~sfriberg/JDK-6321472/webrev.04 //Staffan On 10/21/2014 10:28 AM, Staffan Friberg wrote: Hi Peter, Thanks for the

Re: RFR JDK-6321472: Add CRC-32C API

Hi Peter, Thanks for the comments.. 217 if (Unsafe.ADDRESS_SIZE == 4) { 218 // On 32 bit platforms read two ints instead of a single 64bit long When you're reading from byte[] using Unsafe (updateBytes), you have the option of reading 64bit values on 6

RFR (M) 8061651 - Interface to the Lookup Index Cache to improve URLClassPath search time

Please review this fix: http://cr.openjdk.java.net/~iklam/8061651-lookup-index-open-v1/ Bug: Add an interface to the JVM's Class/Resource Lookup Index Cache for improving sun.misc.URLClassPath search time https://bugs.openjdk.java.net/browse/JDK-8061651 Summary of fix: Some J2EE env

Re: RFR: 8061244 Use of stack-less ClassNotFoundException thrown from (URL)ClassLoader.findClass()

On 10/21/2014 12:37 PM, Peter Levart wrote: http://cr.openjdk.java.net/~plevart/jdk9-dev/ClassLoader.CNFE/CLBench2.java Just an observation... Running this benchmark for about a minute in sampling profiler for small call-stack depth (calling loadClass() public method on XLoader directly

Re: RFR: 8060130: Simplify the synchronization of defining and getting java.lang.Package

Hi Mandy, thanks for the review! On 10/15/2014 03:07 AM, Mandy Chung wrote: Claes, Peter, Thanks for the revised webrev and Peter's thorough review. webrev.05 looks much better. My comment is mostly minor. ClassLoader.java line 1582-1586 - I suggest to get rid of the "oldpkg" variable (

Re: RFR: 8061244 Use of stack-less ClassNotFoundException thrown from (URL)ClassLoader.findClass()

On 10/21/2014 01:03 PM, Stanimir Simeonoff wrote: I like the changes, esp the black listing which is long overdue in JDK (middleware containers have been doing that for over a decade). Not sure if there is any specification governing if two consecutive attempts to load the same class name may re

Re: RFR: 8061244 Use of stack-less ClassNotFoundException thrown from (URL)ClassLoader.findClass()

I like the changes, esp the black listing which is long overdue in JDK (middleware containers have been doing that for over a decade). Not sure if there is any specification governing if two consecutive attempts to load the same class name may respectively fail and succeed - if the black listing p

Re: RFR: 8061244 Use of stack-less ClassNotFoundException thrown from (URL)ClassLoader.findClass()

Hi, I guess we are not happy with stack-traces that only reveal call stack up to the public ClassLoader.loadClass() method and hide everything happening after that even if paired with a more descriptive message. So I improved the strategy of throwing stack-less exception. New strategy throws