Re: RFR [6943190] TEST_BUG: java/lang/Runtime/exec/ExecWithInput.java hardcodes path to cat
Thank you Alan! This looks okay although I would prefer for a number of these tests to fail if the command is not found (otherwise it is will just hide issues). Now I've got two suggestions that somehow contradict each other. Martin suggested to check for particular OS only as a last resort, but without knowing that we run under Unix, we cannot treat a command absence as an error. I think we can assume that none of the tests which need UnixCommands are for Windows, so I added explicit checking for that. Having made sure the OS is a Unix (i.e. not Windows), the absence of a required command now causes an exception to be thrown. Would you please take a look at the updated webrev? http://cr.openjdk.java.net/~igerasim/6943190/6/webrev/ Sincerely yours, Ivan
Re: JDK 9 RFR of 6375303: Review use of caching in BigDecimal
On 03/20/2014 11:06 AM, Peter Levart wrote: I was thinking about last night, for question: Why is this double-checked non-volatile-then-volatile trick not any faster than pure volatile variant even on ARM platform where volatile read should have some penalty compared to normal read?, might be in the fact that Raspberry Pi is a single-core/single-thread machine. Would anyone with JVM JIT compiler expertise care to share some insight? I suspect that on such platform, the compiler optimizes volatile accesses so that they are performed without otherwise necessary memory fences... Yes, at least C2 is known to not emit memory fences on uniprocessor machines. You need to have a multicore ARM. If you are still interested, contact me privately and I can arrange the access to my personal quad-core Cortex-A9. -Aleksey.
Re: StringBuilder version of java.util.regex.Matcher.append*
On 03/19/2014 06:51 PM, Jeremy Manson wrote: I'm told that the diff didn't make it. I've put it in a Google drive folder... https://drive.google.com/file/d/0B_GaXa6O4K5LY3Y0aHpranM3aEU/edit?usp=sharing Jeremy Hi Jeremy, Your factoring-out of expandReplacement() method exposed an opportunity to further optimize the code. Instead of creating intermediate StringBuilder instance for each expandReplacement() call, this method could append directly to resulting StringBuffer/StringBuilder, like in the following: http://cr.openjdk.java.net/~plevart/jdk9-dev/MatcherWithStringBuilder/webrev.01/ Regards, Peter On Wed, Mar 19, 2014 at 10:41 AM, Jeremy Manson jeremyman...@google.comwrote: Hi folks, We've had this internally for a while, and I keep meaning to bring it up here. The Matcher class has a few public methods that take StringBuffers, and we've found it useful to add similar versions that take StringBuilders. It has two benefits: - Users don't have to convert from one to the other when they want to use the method in question. The symmetry is nice. - The StringBuilder variants are faster (if lock optimizations don't kick in, which happens in the interpreter and the client compiler). For interpreted / client-compiled code, we saw something like a 25% speedup on String.replaceAll(), which calls into this code. Any interest? Diff attached. Jeremy
Re: JDK-8036003: Add variable not to separate debug information.
On 2014-03-19 15:57, Andrew Hughes wrote: I think that's something we must fix ourselves. What we really need from OpenJDK is a way to build with complete debuginfo in both binaries and jarfiles. This was my intent with #2. The jstack/jmap issue needs to be fixed by stripping less debuginfo post-build on libjvm.so. Ok, that sounds great. We provide a way to keep the debuginfo in the binaries, and then you can do your own magic, and everyone is happy. :) I'll re-target the original bug (JDK-8036003) so that it is about implementing such debuginfo handling instead. /Magnus
Re: RFR [8014066] Mistake in documentation of ArrayList#removeRange
Am 19.03.2014 23:37, schrieb Ulf Zibis: Am 19.03.2014 23:32, schrieb Martin Buchholz: No one is as performance obsessed as Ulf. :-D :-D :-D And additionally footprint obsessed -Ulf
Re: JDK-8036003: Add variable not to separate debug information.
On 18/03/2014 6:59 PM, Magnus Ihse Bursie wrote: On 2014-03-18 02:19, Andrew Hughes wrote: Do we need more than just the following three alternatives? #1. No debugging information at all. #2. Debugging information left in the original binaries. #3. Debugging information stripped from the binaries and zipped in separate files. It sounds to me like Oracle want #3, while distros want #2 and I imagine some end users may just want #1 for a faster, smaller build. I'm completely thinking along your lines here. I think we should focus on the end result, not the specific implementations details. I've been thinking about exposing a configure option like this: --with-debug-symbols=none (for #1) --with-debug-symbols=internal (for #2) --with-debug-symbols=zipped (for #3) I don't think this quite works as there are other variations not captured here. Rather than zipped it should just be external. Whether the debuginfo files are zipped or not is then an additional build time option. Additionally we still have to be able to control the degree of stripping that is carried out. It doesn't make sense to me to try and enumerate all possible combinations as top-level configure options. Further, as Dan was saying, this doesn't capture the overlap between internal and external because we still leave some symbols internal even when creating the debuginfo file. So I don't think this proposed categorization works. We still have three aspects: - generating symbols into the object files - stripping symbols from the final binaries - saving symbols into an external form Each of which can potentally be varied (of course if you don't generate symbols in the obj files stripping and saving are non issues). David - but perhaps with better names. I also imagine there might be a use case for yet another option, #4. Debuggin information stripped from the binaries, but not zipped expressed like this --with-debug-symbols=external And then this would be the only user interface needed, and the rest of the current set of not-really-clear flags can be hidden inside the configure script. For compatibility reasons, the default would be --with-debug-symbols=zipped (which is called FDS here), just as we currently do, but distributions would just need to add a --with-debug-symbols=internal to get what they want. /Magnus
Re: RFR [6943190] TEST_BUG: java/lang/Runtime/exec/ExecWithInput.java hardcodes path to cat
On 20/03/2014 07:25, Ivan Gerasimov wrote: Now I've got two suggestions that somehow contradict each other. Martin suggested to check for particular OS only as a last resort, but without knowing that we run under Unix, we cannot treat a command absence as an error. It's always hard to get complete agreement on things like this but for tests that are *nix specific then I would say that it's better to check it at the start and have it just pass on Windows without needlessly checking for programs that don't exist or can't be used by the test. I think we can assume that none of the tests which need UnixCommands are for Windows, so I added explicit checking for that. Having made sure the OS is a Unix (i.e. not Windows), the absence of a required command now causes an exception to be thrown. Would you please take a look at the updated webrev? http://cr.openjdk.java.net/~igerasim/6943190/6/webrev/ This looks okay to me. An alternative would be for findCommand to throw an exception when the command is not found and that would allow most of the ensureCommandAvailables(...) usages to be removed and saves searching for them twice. -Alan
Re: RFR (JAXP): 8035577: Xerces Update: impl/xpath/regex/RangeToken.java
I think this OK.
The comments with the o--o did not do much for me though and found them a bit
confusing but perhaps I need more coffee this morning ?
Also, not sure we need the @author tag but I think its usage varies in the
workspace
Best
Lance
On Mar 19, 2014, at 7:10 PM, David Li wrote:
Hi,
This is an update from Xerces for file impl/xpath/regex/TokenRange.java. For
details, please refer to: https://bugs.openjdk.java.net/browse/JDK-8035577.
Webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8035577/webrev/
Existing tests: JAXP SQE and unit tests passed.
Test cases added for typo fix in RangeToken.intersectRanges. Code also
updated to fix a bug where regular expression intersection returns incorrect
value when first range ends later than second range. Example below. Test
cases have been added to cover any scenarios that the code changes affect.
new RegularExpression((?[b-d][a-r])); - returns [b-d] (Correct)
new RegularExpression((?[a-r][b-d])); - returns [b-de-r] (Incorrect)
Thanks,
David
P.S. Notes on bug fixes.
1) Line 404 removal of while loop.
This fixes a new bug where incorrect results are given when first range ends
later than second range. In the old code we got
(?[a-r][b-d]) - returns [b-de-r]
By removing the while loop, we get [b-d].
This while loop looks like a copy-paste error from subtractRanges. In
subtractRanges we need to keep the leftover portion from the first range, but
this does not apply to intersection.
2) Line 388, addition of src2 += 2;
This code change affects anything of the form (?[a-r][b-eg-j]). The code
execution is diagrammed below.
oo (src1)
o--o o--o (src2)
For the first match we get
oo (src1)
o--o (src2)
Next we want to run src2+=2 to get the second pair of endpoints (since the
first two endpoints are already used). Notice how src1begin has been updated
to this.ranges[src1] = src2end+1, which is directly from the code.
o--o (src1)
o--o (src2)
The src2+=2 statement was left out of the old code, and is added in this
webrev. If we leave out the src2+=2 at line 388, on the next iteration of
the large while loop we will reach case } else if (src2end src1begin) {
which also executes src2+=2. This means the correct final result is
generated, but on a later loop. We want to add the new code because it's
better to have all associated variable updated in the sameloop. In addition,
all the other conditions have similar src1 or src2 updates.
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: StringBuilder version of java.util.regex.Matcher.append*
2009? I do have something similar back to 2009 :-) http://cr.openjdk.java.net/~sherman/regex_replace/webrev/ Then the ball was dropped around the discussion of whether or not the IOE should be thrown. But if we are going to/have to have explicit StringBuilder/Buffer pair anyway, then we can keep the Appendable version as private for now and deal with the StringBuilder and Appendable as two separate issues. -Sherman On 03/20/2014 09:52 AM, Jeremy Manson wrote: That's definitely an improvement - I think that when I wrote this (circa 2009), I didn't think about Appendable. I take it my argument convinced someone? :) Jeremy On Thu, Mar 20, 2014 at 1:32 AM, Peter Levartpeter.lev...@gmail.comwrote: On 03/19/2014 06:51 PM, Jeremy Manson wrote: I'm told that the diff didn't make it. I've put it in a Google drive folder... https://drive.google.com/file/d/0B_GaXa6O4K5LY3Y0aHpranM3aEU/ edit?usp=sharing Jeremy Hi Jeremy, Your factoring-out of expandReplacement() method exposed an opportunity to further optimize the code. Instead of creating intermediate StringBuilder instance for each expandReplacement() call, this method could append directly to resulting StringBuffer/StringBuilder, like in the following: http://cr.openjdk.java.net/~plevart/jdk9-dev/MatcherWithStringBuilder/ webrev.01/ Regards, Peter On Wed, Mar 19, 2014 at 10:41 AM, Jeremy Mansonjeremyman...@google.com wrote: Hi folks, We've had this internally for a while, and I keep meaning to bring it up here. The Matcher class has a few public methods that take StringBuffers, and we've found it useful to add similar versions that take StringBuilders. It has two benefits: - Users don't have to convert from one to the other when they want to use the method in question. The symmetry is nice. - The StringBuilder variants are faster (if lock optimizations don't kick in, which happens in the interpreter and the client compiler). For interpreted / client-compiled code, we saw something like a 25% speedup on String.replaceAll(), which calls into this code. Any interest? Diff attached. Jeremy
Re: RFR [6943190] TEST_BUG: java/lang/Runtime/exec/ExecWithInput.java hardcodes path to cat
Thank you Alan! I think we can assume that none of the tests which need UnixCommands are for Windows, so I added explicit checking for that. Having made sure the OS is a Unix (i.e. not Windows), the absence of a required command now causes an exception to be thrown. Would you please take a look at the updated webrev? http://cr.openjdk.java.net/~igerasim/6943190/6/webrev/ This looks okay to me. An alternative would be for findCommand to throw an exception when the command is not found and that would allow most of the ensureCommandAvailables(...) usages to be removed and saves searching for them twice. I think it is informative to have the prerequisites checking at the beginning of the test. And the results of the search are cached in UnixCommands.nameToCommand map, so they're searched for only once. Sincerely yours, Ivan
Re: RFR: JDK-8033662 DateTimeFormatter parsing ignores withZone()
Hi there, It would be great if I could get a review please. The patch is viewable in plain text at JIRA (for IP reasons): https://bugs.openjdk.java.net/secure/attachment/19216/ParseWithZone.patch The same patch is viewable in a nice format at GitHub https://gist.github.com/jodastephen/9505761 This really needs to make 8u20 IMO, so I need to get it into jdk9 first thanks Stephen On 12 March 2014 12:29, Stephen Colebourne scolebou...@joda.org wrote: This is a request for review of this bug: https://bugs.openjdk.java.net/browse/JDK-8033662 and the duplicate: https://bugs.openjdk.java.net/browse/JDK-8033659 The javadoc of the method java.time.format.DateTimeFormatter::withZone says: If no zone has been parsed, then this override zone will be included in the result of the parse where it can be used to build instants and date-times. However, the implementation doesn't obey this. This is a very unfortunate bug that makes some date-time parsing a lot harder. A second related bug in an egde case - not properly handling a ChronoZonedDateTime from TemporalField.resolve - is also tested for and fixed. Proposed patch: https://gist.github.com/jodastephen/9505761 The patch includes no spec changes. The patch includes new and refactored TCK tests. The new tests for withZone() and withChronology() are based on the spec. I need a reviewer and a committer please. thanks
Re: RFR (JAXP): 8035577: Xerces Update: impl/xpath/regex/RangeToken.java
On 3/20/2014 8:40 AM, Lance Andersen - Oracle wrote:
I think this OK.
The comments with the o--o did not do much for me though and found them a bit
confusing but perhaps I need more coffee this morning ?
Black or white? :-)
It illustrates an intersection. For example, the result of the following
would be o--o:
o--o (src1)
o--o (src2)
-Joe
Also, not sure we need the @author tag but I think its usage varies in the
workspace
Best
Lance
On Mar 19, 2014, at 7:10 PM, David Li wrote:
Hi,
This is an update from Xerces for file impl/xpath/regex/TokenRange.java. For
details, please refer to: https://bugs.openjdk.java.net/browse/JDK-8035577.
Webrevs: http://cr.openjdk.java.net/~joehw/jdk9/8035577/webrev/
Existing tests: JAXP SQE and unit tests passed.
Test cases added for typo fix in RangeToken.intersectRanges. Code also updated
to fix a bug where regular expression intersection returns incorrect value when
first range ends later than second range. Example below. Test cases have been
added to cover any scenarios that the code changes affect.
new RegularExpression((?[b-d][a-r])); - returns [b-d] (Correct)
new RegularExpression((?[a-r][b-d])); - returns [b-de-r] (Incorrect)
Thanks,
David
P.S. Notes on bug fixes.
1) Line 404 removal of while loop.
This fixes a new bug where incorrect results are given when first range ends
later than second range. In the old code we got
(?[a-r][b-d]) - returns [b-de-r]
By removing the while loop, we get [b-d].
This while loop looks like a copy-paste error from subtractRanges. In
subtractRanges we need to keep the leftover portion from the first range, but
this does not apply to intersection.
2) Line 388, addition of src2 += 2;
This code change affects anything of the form (?[a-r][b-eg-j]). The code
execution is diagrammed below.
oo (src1)
o--o o--o (src2)
For the first match we get
oo (src1)
o--o (src2)
Next we want to run src2+=2 to get the second pair of endpoints (since the
first two endpoints are already used). Notice how src1begin has been updated
to this.ranges[src1] = src2end+1, which is directly from the code.
o--o (src1)
o--o (src2)
The src2+=2 statement was left out of the old code, and is added in this webrev. If we leave out the
src2+=2 at line 388, on the next iteration of the large while loop we will reach case } else if
(src2end src1begin) { which also executes src2+=2. This means the correct
final result is generated, but on a later loop. We want to add the new code because it's better to
have all associated variable updated in the sameloop. In addition, all the other conditions have
similar src1 or src2 updates.
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering
1 Network Drive
Burlington, MA 01803
lance.ander...@oracle.com
RFR: 8037507: broken links in javax/sql/rowset/package.html
Hi, Looking for a quick review of the following javadoc clean-up ljanders-mac:rowset ljanders$ hg diff diff -r 95e72182e615 src/share/classes/javax/sql/rowset/package.html --- a/src/share/classes/javax/sql/rowset/package.html Thu Mar 20 16:19:08 2014 + +++ b/src/share/classes/javax/sql/rowset/package.html Thu Mar 20 15:46:30 2014 -0400 @@ -295,14 +295,13 @@ h3a name=relspec4.0 Related Specifications/a/h3 ul -lia href=http://java.sun.com/products/jdbc;JDBC 3.0 Specification/a +lia href=https://jcp.org/en/jsr/detail?id=221;JDBC 4.2 Specification/a lia href=http://www.w3.org/XML/Schema;XML Schema/a -lia href=http://www.syncml.org;SyncML/a /ul h3a name=reldocs5.0 Related Documentation/a/h3 ul -lia href=http://java.sun.com/developer/Books/JDBCTutorial/chapter5.html; +lia href=http://docs.oracle.com/javase/tutorial/jdbc/basics/rowset.html; JDBC RowSet Tutorial/a /ul /body Best, Lance -- 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: Javadoc in 9 seems to treat all interfaces with only one method as functional interfaces
* Daniel Fuchs: While playing with JDK 9 javadoc command I noticed that it seems to treat all single method interfaces as if they were functional interfaces - even though they don't have the @FunctionalInterface annotation. Does it take inheritence into account? If yes, why is this even a bug?
Re: Javadoc in 9 seems to treat all interfaces with only one method as functional interfaces
It's a bug because it was an early decision in JDK 8 that got changed: http://mail.openjdk.java.net/pipermail/lambda-libs-spec-experts/2013-November/002379.html And then the change: http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-January/024165.html On Thu, Mar 20, 2014 at 2:51 PM, Florian Weimer f...@deneb.enyo.de wrote: * Daniel Fuchs: While playing with JDK 9 javadoc command I noticed that it seems to treat all single method interfaces as if they were functional interfaces - even though they don't have the @FunctionalInterface annotation. Does it take inheritence into account? If yes, why is this even a bug? -- Cheers, Paul
Re: Javadoc in 9 seems to treat all interfaces with only one method as functional interfaces
* Paul Benedict: It's a bug because it was an early decision in JDK 8 that got changed: http://mail.openjdk.java.net/pipermail/lambda-libs-spec-experts/2013-November/002379.html And then the change: http://mail.openjdk.java.net/pipermail/core-libs-dev/2014-January/024165.html So it's a functional interface, but the documentation doesn't say so because we don't want it to be a functional interface? That's rather odd and suggests that something went wrong with the design. :-)
Re: RFR: JDK-8033662 DateTimeFormatter parsing ignores withZone()
Hi Stephen, Given the fact that the parser context is no longer public, the parsed from the formatter is either unresolved or resolved, just wonder if we really need those effective chrono and zone fields in Parsed. It appears perfect for me to simply keep these info inside the parser context and return the unresolved and resolved based on the formatter's request, for example http://cr.openjdk.java.net/~sherman/8033662/webrev2/ -Sherman On 03/20/2014 11:24 AM, Stephen Colebourne wrote: Hi there, It would be great if I could get a review please. The patch is viewable in plain text at JIRA (for IP reasons): https://bugs.openjdk.java.net/secure/attachment/19216/ParseWithZone.patch The same patch is viewable in a nice format at GitHub https://gist.github.com/jodastephen/9505761 This really needs to make 8u20 IMO, so I need to get it into jdk9 first thanks Stephen On 12 March 2014 12:29, Stephen Colebournescolebou...@joda.org wrote: This is a request for review of this bug: https://bugs.openjdk.java.net/browse/JDK-8033662 and the duplicate: https://bugs.openjdk.java.net/browse/JDK-8033659 The javadoc of the method java.time.format.DateTimeFormatter::withZone says: If no zone has been parsed, then this override zone will be included in the result of the parse where it can be used to build instants and date-times. However, the implementation doesn't obey this. This is a very unfortunate bug that makes some date-time parsing a lot harder. A second related bug in an egde case - not properly handling a ChronoZonedDateTime from TemporalField.resolve - is also tested for and fixed. Proposed patch: https://gist.github.com/jodastephen/9505761 The patch includes no spec changes. The patch includes new and refactored TCK tests. The new tests for withZone() and withChronology() are based on the spec. I need a reviewer and a committer please. thanks
Re: Review request for 8035807: Convert use of sun.misc.BASE64Encoder/Decoder with java.util.Base64
On 3/19/14 12:28 PM, Xueming Shen wrote: On 03/19/2014 11:37 AM, Mandy Chung wrote: https://bugs.openjdk.java.net/browse/JDK-8035807 Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.00/ This patch converts the last 2 references to sun.misc.BASE64Encoder/Decoder from the jdk repo with java.util.Base64. We should also update the tests and I have filed JDK-8037873 for that. Thanks Mandy The sun.misc.BASE64En/Decoder is MIME type, so it outputs the \r\n per 76 characters during encoding, and ignores/skip \r or \n when decoding. The new Base64.getEncoder/Decoder() returns the basic Base64 coder, which it never inserts line separator when output, and throws exception for any non-base64- alphabet character, including \r and \n. The only disadvantage/incompatibility (j.u.Base64.getMimeDecoer() vs sun.misc.BASE64Decoder) of switching to j.u.Base64 MIME type en/decoder is that the Base64 Mime decoder ignores/skips any non-base64-alphabet (including \r and \n), while sun.misc.BASE64Decoder appears to simply use the init value -1 for any non-base64-alphabet character for decoding. I'm not familiar with the use scenario of ldap's Obj class, so I'm not sure if it matters (if it ever outputs/inputs 76 character data, or even it does,if the difference matters). Btw, except getMimeEncoder(int ...) all other Base64.getXXXEn/Decoder() returns singleton, so the de/encoder cache might not be necessary. Thanks Sherman. Vinnie confirms that it should retain the current behavior as there could be long-lived Java object in LDAP encoded with JDK 8 for example and then retrieved with JDK 9. Here is the updated webrev: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.01/ Thanks Mandy
Re: Review request for 8035807: Convert use of sun.misc.BASE64Encoder/Decoder with java.util.Base64
Obj.java:#482 It appears sun.misc.BASE64Decoder.decodeBuffer(String) uses String's deprecated String.getBytes(int srcBegin, int srcEnd, byte[] dst, int dstBegin). The proposed change now uses the jvm's default charset. It might trigger incompatible behavior if the default charset is not an ASCII compatible charset. But if the Java object in LDAP was encoded with the platform default charset (as the new comment suggested), the old implementation actually did not work on platform that the default encoding is not ASCII compatible, such as the IBM ebcdic. -Sherman On 3/20/14 3:48 PM, Mandy Chung wrote: On 3/19/14 12:28 PM, Xueming Shen wrote: On 03/19/2014 11:37 AM, Mandy Chung wrote: https://bugs.openjdk.java.net/browse/JDK-8035807 Webrev at: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.00/ This patch converts the last 2 references to sun.misc.BASE64Encoder/Decoder from the jdk repo with java.util.Base64. We should also update the tests and I have filed JDK-8037873 for that. Thanks Mandy The sun.misc.BASE64En/Decoder is MIME type, so it outputs the \r\n per 76 characters during encoding, and ignores/skip \r or \n when decoding. The new Base64.getEncoder/Decoder() returns the basic Base64 coder, which it never inserts line separator when output, and throws exception for any non-base64- alphabet character, including \r and \n. The only disadvantage/incompatibility (j.u.Base64.getMimeDecoer() vs sun.misc.BASE64Decoder) of switching to j.u.Base64 MIME type en/decoder is that the Base64 Mime decoder ignores/skips any non-base64-alphabet (including \r and \n), while sun.misc.BASE64Decoder appears to simply use the init value -1 for any non-base64-alphabet character for decoding. I'm not familiar with the use scenario of ldap's Obj class, so I'm not sure if it matters (if it ever outputs/inputs 76 character data, or even it does,if the difference matters). Btw, except getMimeEncoder(int ...) all other Base64.getXXXEn/Decoder() returns singleton, so the de/encoder cache might not be necessary. Thanks Sherman. Vinnie confirms that it should retain the current behavior as there could be long-lived Java object in LDAP encoded with JDK 8 for example and then retrieved with JDK 9. Here is the updated webrev: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8035807/webrev.01/ Thanks Mandy
