Re: RFR [9] 8130247: fix some new tidy warnings from jaxws and CORBA
Hello,
Looks fine to me; thanks,
-Joe
On 7/1/2015 7:35 AM, alexander stepanov wrote:
Hello,
Could you please review the following fix
http://cr.openjdk.java.net/~avstepan/8130247/webrev.00/
for
https://bugs.openjdk.java.net/browse/JDK-8130247
Just a minor correction for the docs. Some empty trimming p tidy
warnings fixed; some deprecated tt tags were replaced with {@code
} as well.
Thanks,
Alexander
RFR- 8130238: Remove com.sun.org.apache.xalan.internal.xsltc.cmdline
Hi, Please find below a trivial patch that removes com.sun.org.apache.xalan.internal.xsltc.cmdline This package is not used in the JDK. http://cr.openjdk.java.net/~dfuchs/webrev_8130238/webrev.00/ Jaxp tests passed locally (jaxp/test, jdk/test/javax/xml/jaxp) JCK for XML APIs also passed. More thorough testing through jprt under way... best regards, -- daniel
Re: RFR- 8130238: Remove com.sun.org.apache.xalan.internal.xsltc.cmdline
Looks fine Daniel, I doubt there will be any issues with nuking this Best Lance On Jul 1, 2015, at 9:02 AM, Daniel Fuchs daniel.fu...@oracle.com wrote: http://cr.openjdk.java.net/~dfuchs/webrev_8130238/webrev.00/ 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: RFR JDK-8075526: Need a way to read and write time in UTC
Hi, On 2015-06-30 23:11, Xueming Shen wrote: Hi, Please help review and comment on this rfe. Issue: https://bugs.openjdk.java.net/browse/JDK-8075526 webrev: http://cr.openjdk.java.net/~sherman/8075526 I think this looks reasonable. I think we could consolidate the LocalDateTime-xdostime conversions into ZipUtils along with and aligned with the code to convert UTC instants (ZipUtils::dosToJavaTime/javaToDosTime), which I've suggested should be converted into similar code anyhow: http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-February/031716.html The get32S changes seems like an unrelated but welcome optimization. Background info: The title of the RFE is a little mis-leading. All the existing set/get date-time methods actually work with UTC time. Both the old pair public void setTime(long time); public long getTime(); and the newly introduced pair pubic ZipEntry set/getLastModifiedTime(FileTime time); public FileTime getLastModifiedTime(); are to set/get the last modified time in UTC time. However the ZIP specification clearly specifies that the normal date and time fields in the zip file entry (loc and cen) are defined as the date/time in dos format, which unfortunately is a local date-time. Therefor timezone conversion must be applied before/after the utc time can be set into/got from those fields (the UTC timestamps set/get by the new pair are therefor being set into/got from the extended timestamp fields of the optional extra data of each entry, those fields are specified as unix/utc timestamp) We did not have an local-date-time abstract before the java.time.LocalDateTime was introduced in jdk8, the epoc date/time is the only date/time abstract in java vm. The proposed change here is to add yet another pair of set/get modified time methods public void setTimeLocale(LocalDateTime time); public LocalDateTime getTimeLocal(); to use the java.time.LocalDateTime type to set/get the modified time into zip entry's dos timestamp fields directly WITHOUT involving the timezone conversion (implied, with default TimeZone). Other than solving the pack/unpack problem raised in this RFE, it should also help improve the performance when local-date-time is actually desired when interfacing with the ZipEntry by eliminating the un-necessary/implied timezone conversion. For example, in our jar tool, currently we are printing the timestamp for zip entry e as new Date(e.getTime()).toString(); in which we are converting the local-date-time (ms-dos-formatted in zip entry) to utc time by using the default timezone (in ZipEntry), and then converting the utc time (in Date) back to the printable local date time again. It might be desired to format the local-date-time directly without involving the timezone conversion now via the proposed method java.time.format.DateTimeFormatter.ofPattern(EEE MMM dd HH:mm:ss zzz ) .withZone(java.time.ZoneId.systemDefault()); .format(e.getTimeLocal() In above example, we still use the system default timezone, however, it is used purely to output the zone name for the zzz (which the Date.toString() does), not for conversion. if the zzz is not required/needed, it can just be java.time.format.DateTimeFormatter.ofPattern(EEE MMM dd HH:mm:ss ).format(e.getTimeLocal()); Since you're asking for opinions: I think we should try not to propagate the problem we're working around here (that values are printed/represented in a local time with no way of knowing what timezone the time was local to). If we can print the local timezone at little to no cost, why not? Thanks! /Claes Comment/Opinion please. If we agree the approach/webrev, I will submit the CCC before integration. Thanks, -Sherman
Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs
On 30/06/2015 16:05, Jan Lahoda wrote: We need to override these settings for jshell (we use subclasses of WindowsTerminal/UnixTerminal to get e.g. Ctrl-C detection - not sure if we will want to generalize these additional features for jjs as well). Is there a way to override them (just) for jshell using ServiceLoader? Is extending these classes the normal way to customize? That makes it really awkward to cleanly separate the multiple implementations. BTW: I think what you in the current webrev is okay for now. -Alan
RFR [9] 8130247: fix some new tidy warnings from jaxws and CORBA
Hello,
Could you please review the following fix
http://cr.openjdk.java.net/~avstepan/8130247/webrev.00/
for
https://bugs.openjdk.java.net/browse/JDK-8130247
Just a minor correction for the docs. Some empty trimming p tidy
warnings fixed; some deprecated tt tags were replaced with {@code
} as well.
Thanks,
Alexander
Re: RFR: JDK-8080679: Include jline in JDK for Java and JavaScript REPLs
On 1.7.2015 16:43, Alan Bateman wrote: On 30/06/2015 16:05, Jan Lahoda wrote: We need to override these settings for jshell (we use subclasses of WindowsTerminal/UnixTerminal to get e.g. Ctrl-C detection - not sure if we will want to generalize these additional features for jjs as well). Is there a way to override them (just) for jshell using ServiceLoader? Is extending these classes the normal way to customize? That makes it really awkward to cleanly separate the multiple implementations. Subclassing is used in JLine (there's for example NoInterruptUnixTerminal which is a subclass of UnixTerminal). We could use delegation for some of our purposes, but so far it seems we need to be able to use customized Terminals instead of the original ones, as e.g. WindowsTerminal is synthesizing an InputStream (which is then used instead of System.in) which we are intercepting and continuously reading from it awaiting Ctrl-C (when the user's code is running, Ctrl-C will interrupt it). BTW: I think what you in the current webrev is okay for now. Thanks! Jan -Alan
Re: RFR JDK-8075526: Need a way to read and write time in UTC
Hi Sherman, Thanks for solving this!, this has been a long standing issue for pack200. The changes looks generally good, I have also tested the initial prototype with pack200. Some nitpicks: TestLocalTime.java: 0. line 51, s/supoprt/support/ 1. line 63, you have a } out of place 2. line 71, misplaced , 3. line 111: extra LF. Thanks Kumar On 6/30/2015 2:11 PM, Xueming Shen wrote: Hi, Please help review and comment on this rfe. Issue: https://bugs.openjdk.java.net/browse/JDK-8075526 webrev: http://cr.openjdk.java.net/~sherman/8075526 Background info: The title of the RFE is a little mis-leading. All the existing set/get date-time methods actually work with UTC time. Both the old pair public void setTime(long time); public long getTime(); and the newly introduced pair pubic ZipEntry set/getLastModifiedTime(FileTime time); public FileTime getLastModifiedTime(); are to set/get the last modified time in UTC time. However the ZIP specification clearly specifies that the normal date and time fields in the zip file entry (loc and cen) are defined as the date/time in dos format, which unfortunately is a local date-time. Therefor timezone conversion must be applied before/after the utc time can be set into/got from those fields (the UTC timestamps set/get by the new pair are therefor being set into/got from the extended timestamp fields of the optional extra data of each entry, those fields are specified as unix/utc timestamp) We did not have an local-date-time abstract before the java.time.LocalDateTime was introduced in jdk8, the epoc date/time is the only date/time abstract in java vm. The proposed change here is to add yet another pair of set/get modified time methods public void setTimeLocale(LocalDateTime time); public LocalDateTime getTimeLocal(); to use the java.time.LocalDateTime type to set/get the modified time into zip entry's dos timestamp fields directly WITHOUT involving the timezone conversion (implied, with default TimeZone). Other than solving the pack/unpack problem raised in this RFE, it should also help improve the performance when local-date-time is actually desired when interfacing with the ZipEntry by eliminating the un-necessary/implied timezone conversion. For example, in our jar tool, currently we are printing the timestamp for zip entry e as new Date(e.getTime()).toString(); in which we are converting the local-date-time (ms-dos-formatted in zip entry) to utc time by using the default timezone (in ZipEntry), and then converting the utc time (in Date) back to the printable local date time again. It might be desired to format the local-date-time directly without involving the timezone conversion now via the proposed method java.time.format.DateTimeFormatter.ofPattern(EEE MMM dd HH:mm:ss zzz ) .withZone(java.time.ZoneId.systemDefault()); .format(e.getTimeLocal() In above example, we still use the system default timezone, however, it is used purely to output the zone name for the zzz (which the Date.toString() does), not for conversion. if the zzz is not required/needed, it can just be java.time.format.DateTimeFormatter.ofPattern(EEE MMM dd HH:mm:ss ).format(e.getTimeLocal()); Comment/Opinion please. If we agree the approach/webrev, I will submit the CCC before integration. Thanks, -Sherman
Re: RFR [S] 8077242: (str) Optimize AbstractStringBuilder.append(CharSequence, int, int) for String argument
Thank you Martin for review! On 01.07.2015 0:59, Martin Buchholz wrote: This looks good. Because we already have append(String) it *may* be a good idea to add append(String, int, int). I found only a dozen of the places in JDK where this method is applicable, so I thought it doesn't seem to worth a change in the API. See also my failure to add getChars to CharSequence itself. If we (you?) could make that happen, that would eliminate the need for instanceof String etc. Yes, it was my initial thought too: http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-May/033285.html I guess, it's not a problem to add default CharSequence.getChars(). The problem is to call it safely. If there were a way to enforce all the derived classes except a few trusted stick to the default implementation, it would be possible to use getChars in sb.append(). For now, I limited the scope of the change, so that changing sb.append(str.substring(a, b)) to sb.append(str, a, b) will be at least no slower. Sincerely yours, Ivan On Sat, Jun 27, 2015 at 7:23 AM, Ivan Gerasimov ivan.gerasi...@oracle.com mailto:ivan.gerasi...@oracle.com wrote: Hello! AbstractStringBuilder, which is base for StringBuilder and StringBuffer has a method for appending a sub-sequence of a CharSequence. Internally, it copies one char at a time in a loop, picking them up with CharSequence.charAt() method. For the case when the argument is a String, it can be done more efficiently, by a call to String.getChars(). Since String is a final class, it should be safe to pass 'value' to its method. I've also found a few places in JDK where code can be done more efficient, using this optimization. Would you please help review this fix? BUGURL: https://bugs.openjdk.java.net/browse/JDK-8077242 WEBREV: http://cr.openjdk.java.net/~igerasim/8077242/02/webrev/ http://cr.openjdk.java.net/%7Eigerasim/8077242/02/webrev/ Sincerely yours, Ivan
Re: RFR JDK-8075526: Need a way to read and write time in UTC
On 07/01/2015 05:02 AM, Claes Redestad wrote: Hi, On 2015-06-30 23:11, Xueming Shen wrote: Hi, Please help review and comment on this rfe. Issue: https://bugs.openjdk.java.net/browse/JDK-8075526 webrev: http://cr.openjdk.java.net/~sherman/8075526 I think this looks reasonable. I think we could consolidate the LocalDateTime-xdostime conversions into ZipUtils along with and aligned with the code to convert UTC instants (ZipUtils::dosToJavaTime/javaToDosTime), which I've suggested should be converted into similar code anyhow: I would prefer to deal with JDK-8066644 issue separately. One of the concerns of replacing the j.u.Date with the java.time classes was the possible startup time regression that may be triggered by forcing the vm to load lots of java.time classes given all the jvm starts with loading bunch of jar files. As a matter of fact, we spent the time and did the prototype when doing JSR310, it did show startup regression back then... This might no longer be an issue if moving to module system, but it'd be better to have some data first. On the other hand, the proposed change here is for two new methods, no impact to any existing apps. http://mail.openjdk.java.net/pipermail/core-libs-dev/2015-February/031716.html The get32S changes seems like an unrelated but welcome optimization. It's to fix the bug for negative date-time, which I missed when doing the unix timestamp support. The 32-bit date-time need to be signed to suport date-time before epoch. Well, you really should not have a zip file with last modified time before that though :-) Well in theory we still have the 2106 32-bit overflow issue for the unix timestamp, should find a solution later. In above example, we still use the system default timezone, however, it is used purely to output the zone name for the zzz (which the Date.toString() does), not for conversion. if the zzz is not required/needed, it can just be java.time.format.DateTimeFormatter.ofPattern(EEE MMM dd HH:mm:ss ).format(e.getTimeLocal()); Since you're asking for opinions: I think we should try not to propagate the problem we're working around here (that values are printed/represented in a local time with no way of knowing what timezone the time was local to). If we can print the local timezone at little to no cost, why not? We actually might not be able to just change the jar tool to use the new LocalDateTime interface. It will be an incompatible change again :-) As in the past there were regression complains that there are tools over there that have dependency on the output format of the jar tool. Like my example, even with zzz, the j.t.f.DTF will output the zonename for local here now as pt but the Date.toString() go with PDT, because without tz lookup (with either a ldt or an instant) you don't know if it's PDT or PST. Well someone says it really does not matter:-) Thanks for the review. -Sherman
Re: RFR JDK-8075526: Need a way to read and write time in UTC
Hi Kumar, thanks for the comments. Webrev has been updated according. On 7/1/15 2:09 PM, Kumar Srinivasan wrote: Hi Sherman, Thanks for solving this!, this has been a long standing issue for pack200. The changes looks generally good, I have also tested the initial prototype with pack200. Some nitpicks: TestLocalTime.java: 0. line 51, s/supoprt/support/ 1. line 63, you have a } out of place 2. line 71, misplaced , 3. line 111: extra LF. Thanks Kumar On 6/30/2015 2:11 PM, Xueming Shen wrote: Hi, Please help review and comment on this rfe. Issue: https://bugs.openjdk.java.net/browse/JDK-8075526 webrev: http://cr.openjdk.java.net/~sherman/8075526 Background info: The title of the RFE is a little mis-leading. All the existing set/get date-time methods actually work with UTC time. Both the old pair public void setTime(long time); public long getTime(); and the newly introduced pair pubic ZipEntry set/getLastModifiedTime(FileTime time); public FileTime getLastModifiedTime(); are to set/get the last modified time in UTC time. However the ZIP specification clearly specifies that the normal date and time fields in the zip file entry (loc and cen) are defined as the date/time in dos format, which unfortunately is a local date-time. Therefor timezone conversion must be applied before/after the utc time can be set into/got from those fields (the UTC timestamps set/get by the new pair are therefor being set into/got from the extended timestamp fields of the optional extra data of each entry, those fields are specified as unix/utc timestamp) We did not have an local-date-time abstract before the java.time.LocalDateTime was introduced in jdk8, the epoc date/time is the only date/time abstract in java vm. The proposed change here is to add yet another pair of set/get modified time methods public void setTimeLocale(LocalDateTime time); public LocalDateTime getTimeLocal(); to use the java.time.LocalDateTime type to set/get the modified time into zip entry's dos timestamp fields directly WITHOUT involving the timezone conversion (implied, with default TimeZone). Other than solving the pack/unpack problem raised in this RFE, it should also help improve the performance when local-date-time is actually desired when interfacing with the ZipEntry by eliminating the un-necessary/implied timezone conversion. For example, in our jar tool, currently we are printing the timestamp for zip entry e as new Date(e.getTime()).toString(); in which we are converting the local-date-time (ms-dos-formatted in zip entry) to utc time by using the default timezone (in ZipEntry), and then converting the utc time (in Date) back to the printable local date time again. It might be desired to format the local-date-time directly without involving the timezone conversion now via the proposed method java.time.format.DateTimeFormatter.ofPattern(EEE MMM dd HH:mm:ss zzz ) .withZone(java.time.ZoneId.systemDefault()); .format(e.getTimeLocal() In above example, we still use the system default timezone, however, it is used purely to output the zone name for the zzz (which the Date.toString() does), not for conversion. if the zzz is not required/needed, it can just be java.time.format.DateTimeFormatter.ofPattern(EEE MMM dd HH:mm:ss ).format(e.getTimeLocal()); Comment/Opinion please. If we agree the approach/webrev, I will submit the CCC before integration. Thanks, -Sherman
