Re: JDK 9 RFR of problem listing of IncludeLocalesPluginTest.java
On 01/06/2016 23:10, joe darcy wrote: Hello, Of Late, particularly after the push of JDK-8145136, we've seen frequent timeouts of the recently modified test IncludeLocalesPluginTest.java (JDK-8158272). I'd like to problem list the test until this issues are addressed. In addition, it is seems suspect to me that test does not run with the standard amount of memory and its turning on of verbose GC logging is unusual. It's possible that the CLDR update added more languages and pushed this test over the edge. Excluding it temporarily should be fine while it is tracked down. -Alan
RFR: 8114827: JDK 9 multi-release enabled jar tool
Hi, Please review the following changeset that makes it easier to create multi-release jar files with jar tool. webrev: http://cr.openjdk.java.net/~sdrach/8114827/webrev.01/ issue: https://bugs.openjdk.java.net/browse/JDK-8114827 The changeset is the implementation of a new command line option, —release n, that indicates all following files and directories are placed in the META-INF/versions/n directory of a multi-release jar file. The new command line syntax is jar [OPTION...] [ [--release VERSION] [-C dir] files] … An example is jar --create --file mr.jar README -C foo classes --release 9 -C foo9 classes Foo.class This will put README and all the files under foo/classes in the base (or root) directory of a jar file and put Foo.class and all the files under foo9/classes in the META-INF/versions/9 directory of the jar file.
Re: JDK 9 RFR of problem listing of IncludeLocalesPluginTest.java
+1 Naoto On 6/1/16 3:10 PM, joe darcy wrote: Hello, Of Late, particularly after the push of JDK-8145136, we've seen frequent timeouts of the recently modified test IncludeLocalesPluginTest.java (JDK-8158272). I'd like to problem list the test until this issues are addressed. In addition, it is seems suspect to me that test does not run with the standard amount of memory and its turning on of verbose GC logging is unusual. Thanks, -Joe diff -r f6af8273cffa test/ProblemList.txt --- a/test/ProblemList.txtWed Jun 01 13:47:55 2016 -0700 +++ b/test/ProblemList.txtWed Jun 01 14:56:40 2016 -0700 @@ -397,5 +397,6 @@ # core_tools +tools/jlink/plugins/IncludeLocalesPluginTest.java 8158272 generic-all + -
JDK 9 RFR of problem listing of IncludeLocalesPluginTest.java
Hello, Of Late, particularly after the push of JDK-8145136, we've seen frequent timeouts of the recently modified test IncludeLocalesPluginTest.java (JDK-8158272). I'd like to problem list the test until this issues are addressed. In addition, it is seems suspect to me that test does not run with the standard amount of memory and its turning on of verbose GC logging is unusual. Thanks, -Joe diff -r f6af8273cffa test/ProblemList.txt --- a/test/ProblemList.txtWed Jun 01 13:47:55 2016 -0700 +++ b/test/ProblemList.txtWed Jun 01 14:56:40 2016 -0700 @@ -397,5 +397,6 @@ # core_tools +tools/jlink/plugins/IncludeLocalesPluginTest.java 8158272 generic-all + -
Re: RFR[9]: 8147585: Annotations with lambda expressions has parameters result in wrong behavior.
Hi, I think this was caught by the verifier before 8 since you couldn't have concrete or private methods in an interface. Now you can (though not in Java for private ones). My spider sense tells me there might be something lurking here (though it was a while since this was in my L1 cache). It is not likely, but I'm not 100% sure that it is impossible to make javac produce a bridge when compiling an annotation type for example, so why not remove synthetic methods as well? Spending some time with ASM to do a bunch of tests not compilable in java might be useful, there should also be some frameworks in langtools to produce invalid classfiles IIRC. cheers /Joel On Tue, 31 May 2016 at 17:49 shilpi.rast...@oracle.com < shilpi.rast...@oracle.com> wrote: > Thanks Paul!! > Please see http://cr.openjdk.java.net/~srastogi/8147585/webrev.03/ > > Thanks, > Shilpi > > On 5/31/2016 7:57 PM, Paul Sandoz wrote: > > >"Returns an array containing Method objects reflecting all the declared > methods of the class or interface represented by this Class object, > including public, protected, default (package) access, and private methods, > but excluding inherited methods." > >
Re: Review request: JDK-8157892: StackFrame::getFileName returns null when a source file exists for native methods
The changes look fine to me, Mandy. -Brent On 06/01/2016 11:46 AM, Mandy Chung wrote: http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8157892/webrev.00/ This webrev addresses two issues [1][2]. It’s a simple fix to return proper filename that may be available for native method and fix getByteCodeIndex to check for a native method and returns proper value. Mandy [1] JDK-8157892: StackFrame::getFileName returns null when a source file exists for native methods [2] JDK-8157977: getByteCodeIndex method from StackFrame does not return negative number when StackFrame is a native frame
Review request: JDK-8157892: StackFrame::getFileName returns null when a source file exists for native methods
http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8157892/webrev.00/ This webrev addresses two issues [1][2]. It’s a simple fix to return proper filename that may be available for native method and fix getByteCodeIndex to check for a native method and returns proper value. Mandy [1] JDK-8157892: StackFrame::getFileName returns null when a source file exists for native methods [2] JDK-8157977: getByteCodeIndex method from StackFrame does not return negative number when StackFrame is a native frame
Re: JDK 9 RFR of JDK-5040830: (ann) please improve toString() for annotations containing exception proxies
Hi Joe,
I noticed you chose the old format for a type mismatch ("class java
...") and print the type name in the middle of the comment instead of
before, why?
Other than that, looks good!
cheers
/Joel
On Tue, May 31, 2016 at 9:43 PM, joe darcy wrote:
> Hello,
>
> Some background, when everything is going well, the toString form of an
> annotation looks something like
>
> // Old non-erroneous annotation
>
> @DangerousAnnotation(utopia=BRIGADOON,
> thirtyTwoBitsAreNotEnough=42,
> classy=interface Fleeting,
> classies=[class java.lang.Object, int],
> moreClassies=[])
>
> with newlines added for clarity. However, there are various kinds of
> problems that can occur with the data in an annotation and in those cases an
> exception proxy is created so that if the corresponding method is called an
> exception is thrown rather than the data being returned. In these cases the
> string form of an annotation with
> exceptions-that-would-be-thrown-if-methods-are-called looks like:
>
> // Current erroneous annotation
>
> @DangerousAnnotation(utopia=sun.reflect.annotation.EnumConstantNotPresentExceptionProxy@766a8c91,
> thirtyTwoBitsAreNotEnough=sun.reflect.annotation.AnnotationTypeMismatchExceptionProxy@1f40866a,
> classy=sun.reflect.annotation.TypeNotPresentExceptionProxy@3de4f72b,
> classies=[class java.lang.Object, int],
> moreClassies=[])
>
> Having the proxy implementation leak through to the string representation in
> this case is not helpful and the string can be made more informative. In
> addition, for Class-related values, the current form doesn't use the syntax
> which is legal in source code for annotations. Class-valued annotations are
> set using Class literal syntax, "Foo.class" rather than "class Foo", and
> arrays of Class-valued item should use braces ("{}") rather than brackets,
> ("[]").
>
> For example, it would be better to have annotation string representations
> which looked like:
>
> // New non-erroneous annotation
>
> @DangerousAnnotation(utopia=BRIGADOON,
> thirtyTwoBitsAreNotEnough=42,
> classy=Fleeting.class,
> classies={java.lang.Object.class, int.class},
> moreClassies={})
>
> // New erroneous annotation
>
> @DangerousAnnotation(utopia=BRIGADOON /* Warning: constant not present! */,
> thirtyTwoBitsAreNotEnough=/* Warning type mismatch! "class
> java.lang.Integer[42]" */,
> classy=Fleeting.class /* Warning: type not present! */,
> classies={java.lang.Object.class, int.class},
> moreClassies={})
>
> Please review the code to implement these behavioral changes:
>
> 5040830: (ann) please improve toString() for annotations containing
> exception proxies
> http://cr.openjdk.java.net/~darcy/5040830.2
>
> (If you have erroneous Class-value items in an array of Class values, then
> this throws an exception since the exception proxy cannot be stored into the
> Class[].)
>
> Thanks,
>
> -Joe
>
Re: JDK 9 RFR of JDK-5040830: (ann) please improve toString() for annotations containing exception proxies
Looks good,
Paul.
> On 31 May 2016, at 21:43, joe darcy wrote:
>
> Hello,
>
> Some background, when everything is going well, the toString form of an
> annotation looks something like
>
> // Old non-erroneous annotation
>
> @DangerousAnnotation(utopia=BRIGADOON,
> thirtyTwoBitsAreNotEnough=42,
> classy=interface Fleeting,
> classies=[class java.lang.Object, int],
> moreClassies=[])
>
> with newlines added for clarity. However, there are various kinds of problems
> that can occur with the data in an annotation and in those cases an exception
> proxy is created so that if the corresponding method is called an exception
> is thrown rather than the data being returned. In these cases the string form
> of an annotation with exceptions-that-would-be-thrown-if-methods-are-called
> looks like:
>
> // Current erroneous annotation
>
> @DangerousAnnotation(utopia=sun.reflect.annotation.EnumConstantNotPresentExceptionProxy@766a8c91,
> thirtyTwoBitsAreNotEnough=sun.reflect.annotation.AnnotationTypeMismatchExceptionProxy@1f40866a,
> classy=sun.reflect.annotation.TypeNotPresentExceptionProxy@3de4f72b,
> classies=[class java.lang.Object, int],
> moreClassies=[])
>
> Having the proxy implementation leak through to the string representation in
> this case is not helpful and the string can be made more informative. In
> addition, for Class-related values, the current form doesn't use the syntax
> which is legal in source code for annotations. Class-valued annotations are
> set using Class literal syntax, "Foo.class" rather than "class Foo", and
> arrays of Class-valued item should use braces ("{}") rather than brackets,
> ("[]").
>
> For example, it would be better to have annotation string representations
> which looked like:
>
> // New non-erroneous annotation
>
> @DangerousAnnotation(utopia=BRIGADOON,
> thirtyTwoBitsAreNotEnough=42,
> classy=Fleeting.class,
> classies={java.lang.Object.class, int.class},
> moreClassies={})
>
> // New erroneous annotation
>
> @DangerousAnnotation(utopia=BRIGADOON /* Warning: constant not present! */,
> thirtyTwoBitsAreNotEnough=/* Warning type mismatch! "class
> java.lang.Integer[42]" */,
> classy=Fleeting.class /* Warning: type not present! */,
> classies={java.lang.Object.class, int.class},
> moreClassies={})
>
> Please review the code to implement these behavioral changes:
>
>5040830: (ann) please improve toString() for annotations containing
> exception proxies
>http://cr.openjdk.java.net/~darcy/5040830.2
>
> (If you have erroneous Class-value items in an array of Class values, then
> this throws an exception since the exception proxy cannot be stored into the
> Class[].)
>
> Thanks,
>
> -Joe
>
RE: RFR: Implement RandomAccess spliterator
Thanks Paul!
Your edits look great to me.
For our particular use case, both 1) and 2) are totally fine and no problem.
2) is an interesting corner case, but as you mentioned, it's quite hard to
imagine some RandomAccess implementation takes an approach to introduce side
effects in its indexed-accessor. So it sounds ok to me as a default behavior.
Thanks,
Hiroshi
-Original Message-
From: Paul Sandoz [mailto:paul.san...@oracle.com]
Sent: Wednesday, June 01, 2016 10:26 PM
To: Ito, Hiroshi [Tech]
Cc: core-libs-dev@openjdk.java.net; Chan, Sunny [Tech]; Raab, Donald [Tech]
Subject: Re: RFR: Implement RandomAccess spliterator
Hi Hiroshi,
I created an issue and made some edits:
http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8158365-list-spliterator-rnd-access/webrev/
Let me know what you think.
Specification-wise we need to describe how the default implementation should
behave without diving into specific implementation details.
It now becomes clearer that these issues introduce behavioural changes:
1) A List implementing RandomAccess but not extending from AbstractList may
result in different fail-fast behaviour; and
2) A List implementing RandomAccess may be operated on concurrently via
List.get when it is the source of a stream pipeline executed in parallel.
I think 1) is ok.
2) is obviously the primary motivation for this change, but it could
potentially affect third-party List implementations whose indexed-accessor
performs side-effects. That seems a questionable thing to do for something that
implements RandomAccess, but there is a lot of odd code out there. Sorry, i
should have raised this point earlier. On reflection i think your case trumps
that of such odd code. Thoughts?
Thanks,
Paul.
> On 31 May 2016, at 05:36, Ito, Hiroshi wrote:
>
> Hi Paul,
>
> Thank you very much for your feedback, and apologize for the delay in
> response..
>
> I have incorporated your comments in the attached patch. Please kindly take a
> look and let me know if you have further feedback.
>
> - Modified @implSpec to add a description in case the list implements
> RandomAccess.
> - Added modification checking in RandomAccessSpliterator, only when the list
> implements AbstractList.
>
> Sorry for the confusion about SubList. It was all about the concurrent
> modification checking, so adding the change above pretty much addressed my
> issue earlier.
>
> Thanks,
> Hiroshi
>
> -Original Message-
> From: Paul Sandoz [mailto:paul.san...@oracle.com]
> Sent: Thursday, May 12, 2016 8:46 PM
> To: Ito, Hiroshi [Tech]
> Cc: core-libs-dev@openjdk.java.net; Chan, Sunny [Tech]; Raab, Donald
> [Tech]
> Subject: Re: RFR: Implement RandomAccess spliterator
>
> Hi Hiroshi,
>
> This is a good example of what seems like a small feature and yet
> there are some unexpected complexities :-)
>
>
> We will need to refine the implementation specification of List.spliterator,
> which currently states:
>
> * @implSpec
> * The default implementation creates a
> * late-binding
> spliterator
> * from the list's {@code Iterator}. The spliterator inherits the
> * fail-fast properties of the list's iterator.
>
>
> Since the existing implementation is derived from the iterator:
>
> @Override
> default Spliterator spliterator() {
> return Spliterators.spliterator(this, Spliterator.ORDERED); }
>
> concurrent modification checking will occur. The RandomAccessSpliterator
> should also support modification checking, which i think requires it be an
> inner class to check co-mod state.
>
>
> I am struggling to understand the points you make about the spliterator of a
> sub-list of a Vector being required to be an iterator-based implementation.
> Since AbstractList.SubList can access a Vector's elements through
> List.get/set why cannot RandomAccessSpliterator?
>
> If we want to support random access spliterators on sub-lists i think we
> would anyway need to override the spliterator method to check for concurrent
> modification (as is the case of the iterator method).
>
> Paul.
>
>> On 11 May 2016, at 11:25, Ito, Hiroshi wrote:
>>
>> Hi,
>>
>> Please kindly review the attached patch for RandomAccessSpliterator
>> implementation.
>>
>> Currently default implementation of spliterator is an IteratorSpliterator
>> which is not optimal for RandomAccess data structures (besides ArrayList and
>> Vector). This patch is to provide a default RandomAccessSpliterator
>> implementation for RandomAccess data structure.
>>
>> The figures below demonstrate the performance difference before and after
>> the change. Note the significant performance improvement in test
>> SelectTest.parallel_lazy_streams_gsc (parallel streams performance test for
>> non-JDK Lists that implement RandomAccess but don't yet implement their own
>> spliterators).
>>
>> Benchmark code:
>> https://github.com/goldmansachs/gs-collections/blob/master/jmh-tests/
>> src/main/java/com/gs/collections/impl/jmh/Selec
Re: RFR: 8151832: Improve exception messages in exceptions thrown by jigsaw code
On 01/06/16 10:21, Alan Bateman wrote: On 31/05/2016 18:57, Seán Coffey wrote: new webrev : http://cr.openjdk.java.net/~coffeys/webrev.8151832.v2/webrev/ Also in JprtPath.checkPath then I assume path.getClass() is enough as the toString is specified to return a useful string. In JrtPath then "nul" has been renamed to "null". I'm not sure why this has changed. If it is confusing the NUL (one L) should be fine. nul looked odd/wrong. I'll replace with NUL then. Jim might want to comment on the jimage updates. In most cases then hitting any of these means the JDK is hosed. That is, if we have a bug here or the jimage container file is corrupted then it will likely not start. Ok - will pass this by Jim. In StackTraceElementCompositeData then I'm not sure if printing the CompositeType adds anything useful. I might be better to extract wording from the table in ThreadInfo.from to make it clear that the stackTrace attribute is missing attributes. This reminds me, I suspect this table might need to be updated for JDK 9. I will create a bug for that. CompositeType.toString() is pretty comprehensive and iterates through the instance's keyset. I thought the extra output would hint at what went wrong. I could print both Objects if you want a better comparison. Or I can start delving into the ThreadInfo.from table if you think that's a more correct approach. regards, Sean. -Alan
Re: RFR: Implement RandomAccess spliterator
Hi Hiroshi,
I created an issue and made some edits:
http://cr.openjdk.java.net/~psandoz/jdk9/JDK-8158365-list-spliterator-rnd-access/webrev/
Let me know what you think.
Specification-wise we need to describe how the default implementation should
behave without diving into specific implementation details.
It now becomes clearer that these issues introduce behavioural changes:
1) A List implementing RandomAccess but not extending from AbstractList may
result in different fail-fast behaviour; and
2) A List implementing RandomAccess may be operated on concurrently via
List.get when it is the source of a stream pipeline executed in parallel.
I think 1) is ok.
2) is obviously the primary motivation for this change, but it could
potentially affect third-party List implementations whose indexed-accessor
performs side-effects. That seems a questionable thing to do for something that
implements RandomAccess, but there is a lot of odd code out there. Sorry, i
should have raised this point earlier. On reflection i think your case trumps
that of such odd code. Thoughts?
Thanks,
Paul.
> On 31 May 2016, at 05:36, Ito, Hiroshi wrote:
>
> Hi Paul,
>
> Thank you very much for your feedback, and apologize for the delay in
> response..
>
> I have incorporated your comments in the attached patch. Please kindly take a
> look and let me know if you have further feedback.
>
> - Modified @implSpec to add a description in case the list implements
> RandomAccess.
> - Added modification checking in RandomAccessSpliterator, only when the list
> implements AbstractList.
>
> Sorry for the confusion about SubList. It was all about the concurrent
> modification checking, so adding the change above pretty much addressed my
> issue earlier.
>
> Thanks,
> Hiroshi
>
> -Original Message-
> From: Paul Sandoz [mailto:paul.san...@oracle.com]
> Sent: Thursday, May 12, 2016 8:46 PM
> To: Ito, Hiroshi [Tech]
> Cc: core-libs-dev@openjdk.java.net; Chan, Sunny [Tech]; Raab, Donald [Tech]
> Subject: Re: RFR: Implement RandomAccess spliterator
>
> Hi Hiroshi,
>
> This is a good example of what seems like a small feature and yet there are
> some unexpected complexities :-)
>
>
> We will need to refine the implementation specification of List.spliterator,
> which currently states:
>
> * @implSpec
> * The default implementation creates a
> * late-binding spliterator
> * from the list's {@code Iterator}. The spliterator inherits the
> * fail-fast properties of the list's iterator.
>
>
> Since the existing implementation is derived from the iterator:
>
> @Override
> default Spliterator spliterator() {
> return Spliterators.spliterator(this, Spliterator.ORDERED);
> }
>
> concurrent modification checking will occur. The RandomAccessSpliterator
> should also support modification checking, which i think requires it be an
> inner class to check co-mod state.
>
>
> I am struggling to understand the points you make about the spliterator of a
> sub-list of a Vector being required to be an iterator-based implementation.
> Since AbstractList.SubList can access a Vector's elements through
> List.get/set why cannot RandomAccessSpliterator?
>
> If we want to support random access spliterators on sub-lists i think we
> would anyway need to override the spliterator method to check for concurrent
> modification (as is the case of the iterator method).
>
> Paul.
>
>> On 11 May 2016, at 11:25, Ito, Hiroshi wrote:
>>
>> Hi,
>>
>> Please kindly review the attached patch for RandomAccessSpliterator
>> implementation.
>>
>> Currently default implementation of spliterator is an IteratorSpliterator
>> which is not optimal for RandomAccess data structures (besides ArrayList and
>> Vector). This patch is to provide a default RandomAccessSpliterator
>> implementation for RandomAccess data structure.
>>
>> The figures below demonstrate the performance difference before and after
>> the change. Note the significant performance improvement in test
>> SelectTest.parallel_lazy_streams_gsc (parallel streams performance test for
>> non-JDK Lists that implement RandomAccess but don't yet implement their own
>> spliterators).
>>
>> Benchmark code:
>> https://github.com/goldmansachs/gs-collections/blob/master/jmh-tests/src/main/java/com/gs/collections/impl/jmh/SelectTest.java
>>
>> With IteratorSpliterator as default:
>>
>> Benchmark Mode CntScoreError Units
>> SelectTest.parallel_lazy_jdk thrpt 20 172.671 ± 14.231 ops/s
>> SelectTest.parallel_lazy_streams_gsc thrpt 20 20.662 ± 0.493 ops/s
>> SelectTest.serial_lazy_jdkthrpt 20 89.384 ± 4.431 ops/s
>> SelectTest.serial_lazy_streams_gscthrpt 20 80.831 ± 7.875 ops/s
>>
>> With RandomAccessSpliterator as default:
>>
>> Benchmark Mode CntScoreError Units
>> SelectTest.parallel_lazy_jdk thrpt 20 174.190 ± 16.870 ops/s
>>
Re: PING: RFR: JDK-4347142: Need method to set Password protection to Zip entries
On 06/01/2016 01:55 PM, Yasumasa Suenaga wrote: >2. We think that we can apply AE-1 and AE-2 format from WinZip [1]. > This format uses extra data field in standard ZIP file format for > encryption header. > Thus we can handle encrypted/unencrypted ZIP archive with keeping > compatibility. Aha! OK, that makes very good sense. Andrew.
Re: PING: RFR: JDK-4347142: Need method to set Password protection to Zip entries
Hi Andrew, Thank you for your comment. We will not modify ZIP file format, so I think we do not need to negotiate for ZIP file format. We want to propose to JEP as below: 1. We do not need to modify current java.util.zip implementation for unencrypted data. 2. We think that we can apply AE-1 and AE-2 format from WinZip [1]. This format uses extra data field in standard ZIP file format for encryption header. Thus we can handle encrypted/unencrypted ZIP archive with keeping compatibility. 3. For extensibility, the encryption engine should be pluggable like Java Cryptography Architecture (JCA). If so, any Java developer can add custom encryption engine. (e.g. traditional ZIP encryption engine) Thanks, Yasumasa [1] http://www.winzip.com/aes_info.htm On 2016/05/31 5:17, Andrew Haley wrote: On 30/05/16 13:44, Yasumasa Suenaga wrote: We had aimed to handle *traditional* zip encryption at first. However, we also want to handle *strong* zip encryption. (e.g. AES) OpenJDK does not control the format of a zip file. To make such a change we'd have to negotiate with the other users of the .ZIP format. Andrew.
Re: RFR: 8151876: (tz) Support tzdata2016d
That test edit looks fine Ramanand and will ensure that the test runs
remain green. I'll push this change for you now.
Regards,
Sean.
On 01/06/16 13:08, Ramanand Patil wrote:
Hi all,
As mentioned by point 4 in my first review thread, one test case is
still failing for which a separate JBS bug is created.(
https://bugs.openjdk.java.net/browse/JDK-8157792 ).
Below edit is needed for green tests and issue will be revisited via
JDK-8157792.
--- a/test/sun/util/calendar/zi/TestZoneInfo310.java
+++ b/test/sun/util/calendar/zi/TestZoneInfo310.java
@@ -164,6 +164,10 @@
}
for (String zid : zids_new) {
+ if (zid.equals("Asia/Oral") || zid.equals("Asia/Qyzylorda")) {
+ // JDK-8157792 tracking this issue
+ continue;
+ }
ZoneInfoOld zi = toZoneInfoOld(TimeZone.getTimeZone(zid));
ZoneInfoOld ziOLD = (ZoneInfoOld)ZoneInfoOld.getTimeZone(zid);
if (! zi.equalsTo(ziOLD)) {
Regards,
Ramanand.
*From:*Seán Coffey
*Sent:* Tuesday, May 31, 2016 3:05 PM
*To:* Masayoshi Okutsu; Ramanand Patil; i18n-...@openjdk.java.net;
core-libs-dev@openjdk.java.net
*Subject:* Re: RFR: 8151876: (tz) Support tzdata2016d
Masayoshi,
I still think the test adds value. At minimum it identifies timezones
which don't have a localised string in the JRE provider.
We need to start another discussion about how best to represent
timezone names for newly added timezones. At the moment, short and
long formats will be of "GMT±hh:mm" format. I suggest we use the IANA
timezone name for the long format name in future (e.g. "Asia/Tomsk"
instead of "GMT+06:00")
For the sake of getting this into the JDK code lines, I suggest we go
ahead with your suggestion to remove the test for now. I've also
reviewed this Ramanand. Your edits look fine (including test removal
for now)
Regards,
Sean.
On 31/05/2016 07:06, Masayoshi Okutsu wrote:
The CheckDisplayNames.java change is different from what I
suggested and doesn't make sense. But we no longer need the test.
I'd suggest CheckDisplayNames.java be removed. Otherwise, the fix
looks OK to me.
Masayoshi
On 5/31/2016 3:03 AM, Ramanand Patil wrote:
Hi Masayoshi and All,
Here is the updated Webrev:
http://cr.openjdk.java.net/~rpatil/8151876/webrev.01/
;
core-libs-dev@openjdk.java.net
*Subject:* Re: RFR: 8151876: (tz) Support tzdata2016d
CLDR locale data defines time zone names, like this (in en.xml):
Almaty Time
Almaty Standard
Time
Almaty Summer
Time
The CLDR converter tool tries to fill in the missing short
names from the legacy TimeZoneNames data. Removing existing
names causes some unexpected behavior. I think JDK-8157814 is
an example of the unexpected behavior. And the suggested fix
in JDK-8157814 looks to me like a workaround.
I still think the existing names should be kept unchanged for
this fix.
Thanks,
Masayoshi
On 5/28/2016 12:04 AM, Seán Coffey wrote:
I guess there's a low risk of timezone not being
identified if being parsed in through a formatter. Isn't
such an approach discouraged though ? short IDs were
already subject to change in tzdata releases. Ramanand
found one issue by removal of these resource strings (so
far) and that's captured in JDK-8157814
There's a decision to be made about how to use the
GMT±hh:mm format for new releases given IANA's new short
ID identifier mechanism. I think that could be discussed
as a separate issue. I'd like to see us follow a similar
approach to IANA and use GMT identifiers on new timezones
and perhaps even consider using the IANA long name format
also for the getDisplayName(..) c
RE: RFR: 8151876: (tz) Support tzdata2016d
Hi all,
As mentioned by point 4 in my first review thread, one test case is still
failing for which a separate JBS bug is created.(
https://bugs.openjdk.java.net/browse/JDK-8157792 ).
Below edit is needed for green tests and issue will be revisited via
JDK-8157792.
--- a/test/sun/util/calendar/zi/TestZoneInfo310.java
+++ b/test/sun/util/calendar/zi/TestZoneInfo310.java
@@ -164,6 +164,10 @@
}
for (String zid : zids_new) {
+ if (zid.equals("Asia/Oral") || zid.equals("Asia/Qyzylorda")) {
+ // JDK-8157792 tracking this issue
+ continue;
+ }
ZoneInfoOld zi = toZoneInfoOld(TimeZone.getTimeZone(zid));
ZoneInfoOld ziOLD = (ZoneInfoOld)ZoneInfoOld.getTimeZone(zid);
if (! zi.equalsTo(ziOLD)) {
Regards,
Ramanand.
From: Seán Coffey
Sent: Tuesday, May 31, 2016 3:05 PM
To: Masayoshi Okutsu; Ramanand Patil; i18n-...@openjdk.java.net;
core-libs-dev@openjdk.java.net
Subject: Re: RFR: 8151876: (tz) Support tzdata2016d
Masayoshi,
I still think the test adds value. At minimum it identifies timezones which
don't have a localised string in the JRE provider.
We need to start another discussion about how best to represent timezone names
for newly added timezones. At the moment, short and long formats will be of
"GMT±hh:mm" format. I suggest we use the IANA timezone name for the long format
name in future (e.g. "Asia/Tomsk" instead of "GMT+06:00")
For the sake of getting this into the JDK code lines, I suggest we go ahead
with your suggestion to remove the test for now. I've also reviewed this
Ramanand. Your edits look fine (including test removal for now)
Regards,
Sean.
On 31/05/2016 07:06, Masayoshi Okutsu wrote:
The CheckDisplayNames.java change is different from what I suggested and
doesn't make sense. But we no longer need the test. I'd suggest
CheckDisplayNames.java be removed. Otherwise, the fix looks OK to me.
Masayoshi
On 5/31/2016 3:03 AM, Ramanand Patil wrote:
Hi Masayoshi and All,
Here is the updated Webrev: HYPERLINK
"http://cr.openjdk.java.net/%7Erpatil/8151876/webrev.01/"http://cr.openjdk.java.net/~rpatil/8151876/webrev.01/
As suggested by Masayoshi, I have kept the existing names unchanged.
Also, this patch contains extra test case fixes for
1. java/util/TimeZone/CheckDisplayNames.java
2. java/util/TimeZone/Bug8149452.java
The exclusion for the newly added TimeZones is added in these test cases where
the entries are not present in the resources and the Short/Long display names
fallback to the GMT±hh:mm format.
Regards,
Ramanand.
From: Masayoshi Okutsu
Sent: Saturday, May 28, 2016 10:58 AM
To: Seán Coffey; Ramanand Patil; HYPERLINK
"mailto:i18n-...@openjdk.java.net"i18n-...@openjdk.java.net; HYPERLINK
"mailto:core-libs-dev@openjdk.java.net"core-libs-dev@openjdk.java.net
Subject: Re: RFR: 8151876: (tz) Support tzdata2016d
CLDR locale data defines time zone names, like this (in en.xml):
Almaty Time
Almaty Standard
Time
Almaty Summer Time
The CLDR converter tool tries to fill in the missing short names from the
legacy TimeZoneNames data. Removing existing names causes some unexpected
behavior. I think JDK-8157814 is an example of the unexpected behavior. And the
suggested fix in JDK-8157814 looks to me like a workaround.
I still think the existing names should be kept unchanged for this fix.
Thanks,
Masayoshi
On 5/28/2016 12:04 AM, Seán Coffey wrote:
I guess there's a low risk of timezone not being identified if being parsed in
through a formatter. Isn't such an approach discouraged though ? short IDs were
already subject to change in tzdata releases. Ramanand found one issue by
removal of these resource strings (so far) and that's captured in JDK-8157814
There's a decision to be made about how to use the GMT±hh:mm format for new
releases given IANA's new short ID identifier mechanism. I think that could be
discussed as a separate issue. I'd like to see us follow a similar approach to
IANA and use GMT identifiers on new timezones and perhaps even consider using
the IANA long name format also for the getDisplayName(..) calls that can be
made. e.g. "Asia/Almaty" instead of "Alma-Ata Time"
Regards,
Sean.
On 27/05/16 15:24, Masayoshi Okutsu wrote:
This change seems to beyond my proposal that the "GMT±hh:mm" format is used for
*new* zones with the "±hh" format. But this change removes *existing* zones
which have changed to use the "±hh" format in tzdata. Can this cause any
compatibility issues?
And have we agreed to use the "GMT±hh:mm" format?
Thanks,
Masayoshi
On 5/27/2016 10:19 PM, Seán Coffey wrote:
Looks fine to me Ramanand. the recen
Re: RFR: 8151832: Improve exception messages in exceptions thrown by jigsaw code
On 31/05/2016 18:57, Seán Coffey wrote: new webrev : http://cr.openjdk.java.net/~coffeys/webrev.8151832.v2/webrev/ Also in JprtPath.checkPath then I assume path.getClass() is enough as the toString is specified to return a useful string. In JrtPath then "nul" has been renamed to "null". I'm not sure why this has changed. If it is confusing the NUL (one L) should be fine. Jim might want to comment on the jimage updates. In most cases then hitting any of these means the JDK is hosed. That is, if we have a bug here or the jimage container file is corrupted then it will likely not start. In StackTraceElementCompositeData then I'm not sure if printing the CompositeType adds anything useful. I might be better to extract wording from the table in ThreadInfo.from to make it clear that the stackTrace attribute is missing attributes. This reminds me, I suspect this table might need to be updated for JDK 9. I will create a bug for that. -Alan
