Re: Review Request JDK-8193325: StackFrameInfo::getByteCodeIndex returns wrong value if bci > 32767

2017-12-12 Thread John Rose
On Dec 12, 2017, at 5:24 PM, David Holmes wrote: > > as per getByteCodeIndex() it needs to be able to store "-1" to indicate > no-bci? Yes, that's the essential requirement, to encode a "none" value. FWIW, one natural way to do that here would be to make the int field @Stable and have the acce

Re: Review Request JDK-8193325: StackFrameInfo::getByteCodeIndex returns wrong value if bci > 32767

2017-12-12 Thread David Holmes
Hi Mandy, On 13/12/2017 10:42 AM, mandy chung wrote: http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8193325/webrev.00/ This fixes the bug in hotspot that sets the bci field as int but it's of short type and also StackFrameInfo to return a proper bci value.  Coleen suggests not to change set

Re: RFR[10] 8190984 : tools/launcher/TestXcheckJNIWarnings.java WARNING was found in the output

2017-12-12 Thread Brent Christian
What David said. :) One other wrinkle: I ProblemList-ed the test a few days ago[1] in jdk/jdk. AFAICT, that change is not yet in jdk/hs, so the changes David's pushing can't be used to take the test back off of the ProblemList. I figure I'll have to file a new issue, to be fixed in jdk/jdk

Re: RFR[10] 8190984 : tools/launcher/TestXcheckJNIWarnings.java WARNING was found in the output

2017-12-12 Thread mandy chung
On 12/12/17 4:18 PM, David Holmes wrote: Hi Mandy, On 13/12/2017 10:12 AM, mandy chung wrote: How do we pick the value 50?  The JNI local refs are deleted in some cases and it's unclear how 50 is computed.  What if new properties are added in the future? IIRC Brent instrumented the localre

Re: RFR[10] 8190984 : tools/launcher/TestXcheckJNIWarnings.java WARNING was found in the output

2017-12-12 Thread David Holmes
On 13/12/2017 10:41 AM, Brent Christian wrote: What David said. :) One other wrinkle: I ProblemList-ed the test a few days ago[1] in I forgot about that :( jdk/jdk.  AFAICT, that change is not yet in jdk/hs, so the changes David's pushing can't be used to take the test back off of the Prob

Review Request JDK-8193325: StackFrameInfo::getByteCodeIndex returns wrong value if bci > 32767

2017-12-12 Thread mandy chung
http://cr.openjdk.java.net/~mchung/jdk10/webrevs/8193325/webrev.00/ This fixes the bug in hotspot that sets the bci field as int but it's of short type and also StackFrameInfo to return a proper bci value.  Coleen suggests not to change set_bci to take jushort but instead do the cast as in the

Re: RFR[10] 8190984 : tools/launcher/TestXcheckJNIWarnings.java WARNING was found in the output

2017-12-12 Thread mandy chung
How do we pick the value 50?  The JNI local refs are deleted in some cases and it's unclear how 50 is computed.  What if new properties are added in the future? Mandy On 12/12/17 4:00 PM, David Holmes wrote: Reviewed! :) Thanks Brent! I guess I can push these both now. David On 13/12/2017

Re: RFR[10] 8190984 : tools/launcher/TestXcheckJNIWarnings.java WARNING was found in the output

2017-12-12 Thread David Holmes
Hi Mandy, On 13/12/2017 10:12 AM, mandy chung wrote: How do we pick the value 50?  The JNI local refs are deleted in some cases and it's unclear how 50 is computed.  What if new properties are added in the future? IIRC Brent instrumented the localref methods to see how many were being create

Re: RFR[10] 8190984 : tools/launcher/TestXcheckJNIWarnings.java WARNING was found in the output

2017-12-12 Thread David Holmes
Reviewed! :) Thanks Brent! I guess I can push these both now. David On 13/12/2017 9:38 AM, Brent Christian wrote: Hi, Please review this small change to the System.initProperties() native method. The tools/launcher/TestXcheckJNIWarnings.java has begun to fail due to this warning being is

RFR[10] 8190984 : tools/launcher/TestXcheckJNIWarnings.java WARNING was found in the output

2017-12-12 Thread Brent Christian
Hi, Please review this small change to the System.initProperties() native method. The tools/launcher/TestXcheckJNIWarnings.java has begun to fail due to this warning being issued: WARNING: JNI local refs: 41, exceeds capacity: 40 at java.lang.System.initProperties(java.base/Native M

Re: RFR(s): 8060192: Add default method Collection.toArray(generator)

2017-12-12 Thread Stuart Marks
On 12/7/17 5:03 PM, Martin Buchholz wrote: (I'm still trying to love this new API) The changes to the jsr166 tck are fine. I'm not convinced that the new implementation for ArrayList is progress.  The current implementation of toArray(T[]) does             // Make a new array of a's runti

Re: RFR of 8180451: ByteArrayInputStream should override readAllBytes, readNBytes, and transferTo

2017-12-12 Thread Brent Christian
Hi, Brian The changes look fine to me. I would have found the test case a little easier to follow if "off"/"len" weren't named so similarly to "offset"/"length", but it's not a big deal. -Brent On 12/11/17 12:47 PM, Brian Burkhalter wrote: https://bugs.openjdk.java.net/browse/JDK-8180451 ht

Re: RFR of 8180451: ByteArrayInputStream should override readAllBytes, readNBytes, and transferTo

2017-12-12 Thread Brian Burkhalter
On Dec 12, 2017, at 11:42 AM, Paul Sandoz wrote: > int len = count - pos > out.write(but, pos, len); > pos = count; > return len; Indeed that is clearer. Thanks, Brian

Re: RFR of 8180451: ByteArrayInputStream should override readAllBytes, readNBytes, and transferTo

2017-12-12 Thread Paul Sandoz
> On 11 Dec 2017, at 12:47, Brian Burkhalter > wrote: > > https://bugs.openjdk.java.net/browse/JDK-8180451 > http://cr.openjdk.java.net/~bpb/8180451/webrev.00 > > 1. Add overrides of readAllBytes(), readNBytes(), and transferTo(). > 2. Simplify parameter checks in read(byte[],int,int). > 3. C

Re: Possible bug in StackFrameInfo#getByteCodeIndex?

2017-12-12 Thread mandy chung
Good catch!  I agree that it should not be final and the constant field should be static. I will take JDK-8193325 and follow up this issue. -1 is ambiguous but should be initialized to a valid index when filled by the VM.  We tried to keep it compact for footprint concern.  Anyway I will propo

Re: RFR (JDK10) 8183743: Umbrella: add overloads that take a Charset parameter

2017-12-12 Thread Joe Wang
Thanks Roger, Alan. I've pushed the changeset after re-ran the core tests. -Joe On 12/12/17, 8:47 AM, Roger Riggs wrote: Hi Joe, Looks good; +1 Regards, Roger On 12/11/2017 3:04 PM, Joe Wang wrote: On 12/11/17, 11:18 AM, Roger Riggs wrote: Hi Joe, The new tests using testng look good, a

Re: Possible bug in StackFrameInfo#getByteCodeIndex?

2017-12-12 Thread Martin Buchholz
Out of curiosity, I looked more at StackFrameInfo, and saw: short bci is final and is only ever assigned to -1 in java code. What's up with that? Ohh, it seems to be magically frobbed in hotspot: void java_lang_StackFrameInfo::set_bci(oop element, int value) { element->int_field_put(_bci_offs

Re: RFR: JDK-8184947:,ZipCoder performance improvements

2017-12-12 Thread Xueming Shen
On 12/12/2017 09:58 AM, Claes Redestad wrote: StringCoding.java: private static void throwMalformed(int nb) { throw new IllegalArgumentException("malformed input length : " + nb); } nb is the number of bytes of the *first* offending chunk of bytes(?); is this information generally useful?

Re: Review Request: JDK-8193192: jdeps --generate-module-info does not look at module path

2017-12-12 Thread Daniel Fuchs
Hi Mandy, I have seen nothing obviously wrong with this patch. Using tokens instead of a bunch of boolean variables is a nice simplification IMO. best regards, -- daniel On 07/12/2017 17:17, mandy chung wrote: jdeps currently finds modules from the module path for analysis based on the value

Re: RFR: JDK-8184947:,ZipCoder performance improvements

2017-12-12 Thread Claes Redestad
Hi Sherman, On 2017-12-11 05:08, Xueming Shen wrote: http://cr.openjdk.java.net/~sherman/8184947/webrev thanks for incorporating my suggestion! It looks pretty good to me.  While many parts is just code that has been moved, this is still a pretty big change, so I hope we can get at least ano

Re: RFR (JDK10) 8183743: Umbrella: add overloads that take a Charset parameter

2017-12-12 Thread Roger Riggs
Hi Joe, Looks good;  +1 Regards, Roger On 12/11/2017 3:04 PM, Joe Wang wrote: On 12/11/17, 11:18 AM, Roger Riggs wrote: Hi Joe, The new tests using testng look good, a few comments:  - BAOS/EncodingTest.java    70:  The message gets printed if the condition is not true; so "results do not

Re: [10]RFR 8190278: ClassCastException is thrown by java.util.Scanner when a NumberFormatProvider is used.

2017-12-12 Thread Roger Riggs
Hi Nishit, Looks fine to me Regards, Roger On 12/11/2017 4:01 PM, Naoto Sato wrote: Looks good to me. Naoto On 12/11/17 1:04 AM, Nishit Jain wrote: Hi, Please review the fix for JDK-8190278 Bug: https://bugs.openjdk.java.net/browse/JDK-8190278 Webrev: http://cr.openjdk.java.net/~nishjain

Any news from JDK-8151981 in Java8 ?

2017-12-12 Thread João Paulo Varandas
Hi guys! We are also experiencing some odd issues here with setCallSiteTargetNormal with Java8. https://bugs.openjdk.java.net/browse/JDK-8151981 Our cenario is: - Web Application using Tomcat; - 8 HTTP Threads; - A single ScriptEngine for the whole application; When a request hits the server, t

Re: RFR(s): 8060192: Add default method Collection.toArray(generator)

2017-12-12 Thread David Lloyd
On Mon, Dec 11, 2017 at 8:00 PM, John Rose wrote: > I submit to you that such a factory is *not* an IntFunction, because > that only creates half of an array (or 0.01% of one), the empty > version that needs to be populated. A natural array factory API > [...] > The interface would look something

Re: Possible bug in StackFrameInfo#getByteCodeIndex?

2017-12-12 Thread Vitaly Davidovich
On Tue, Dec 12, 2017 at 8:45 AM, David Lloyd wrote: > On Mon, Dec 11, 2017 at 9:03 PM, mandy chung > wrote: > > On 12/11/17 6:31 PM, Martin Buchholz wrote: > >> Java has an unsigned 16 bit type. Could bci be of type "char" ? > > > > Yes but I think keeping it short is fine. > > Call me strange

Re: Possible bug in StackFrameInfo#getByteCodeIndex?

2017-12-12 Thread David Lloyd
On Mon, Dec 11, 2017 at 9:03 PM, mandy chung wrote: > On 12/11/17 6:31 PM, Martin Buchholz wrote: >> Java has an unsigned 16 bit type. Could bci be of type "char" ? > > Yes but I think keeping it short is fine. Call me strange if you like, but I never liked using char as anything other than a UT

Re: Possible bug in StackFrameInfo#getByteCodeIndex?

2017-12-12 Thread David Lloyd
Thanks! On Mon, Dec 11, 2017 at 3:53 PM, mandy chung wrote: > I filed https://bugs.openjdk.java.net/browse/JDK-8193325. I can sponsor > this patch for you. > > --- a/src/java.base/share/classes/java/lang/StackFrameInfo.java > +++ b/src/java.base/share/classes/java/lang/StackFrameInfo.java > @@

RE: FW: RFR(M): 8189102: All tools should support -?, -h and --help

2017-12-12 Thread Lindenmaier, Goetz
Hi Alan, Javadoc combines documentation and support of a flag in the way the flag handling is implemented. On the other side, it prints the help message anyways if a wrong flag is presented to it, so if you call it with -help you get the help message. Therefore, in my original change where I tr

JDK-8190919 :StaX buffers do not really close underlying buffers

2017-12-12 Thread Srividya Shamaiah
Joe, Thanks you for your response. I agree with you that the specification says the underlying stream should not be closed. However, it is not clear what was the reason for decided not to close the underlying stream. Also, it is not evident at what point the underlying streams will get closed an