Integrated: 8287187: Utilize HashMap.newHashMap() in CLDRConverter

2022-05-26 Thread Naoto Sato
On Wed, 25 May 2022 16:43:59 GMT, Naoto Sato  wrote:

> Refactoring the leftover self-calculations of the optimized `HashMap` initial 
> value with `newHashMap()` method. Also replaced some string literals using 
> text blocks for better readability. Confirmed that the output resource bundle 
> sources are effectively (sans indentation) the same as the original ones.

This pull request has now been integrated.

Changeset: c10749a6
Author:    Naoto Sato 
URL:   
https://git.openjdk.java.net/jdk/commit/c10749a6a70977fbd6cd33b298410d212276fcf1
Stats: 65 lines in 1 file changed: 8 ins; 1 del; 56 mod

8287187: Utilize HashMap.newHashMap() in CLDRConverter

Reviewed-by: joehw

-

PR: https://git.openjdk.java.net/jdk/pull/8887


Re: RFR: 8287187: Utilize HashMap.newHashMap() in CLDRConverter [v2]

2022-05-25 Thread Naoto Sato
> Refactoring the leftover self-calculations of the optimized `HashMap` initial 
> value with `newHashMap()` method. Also replaced some string literals using 
> text blocks for better readability. Confirmed that the output resource bundle 
> sources are effectively (sans indentation) the same as the original ones.

Naoto Sato has updated the pull request incrementally with one additional 
commit since the last revision:

  minor fixup

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/8887/files
  - new: https://git.openjdk.java.net/jdk/pull/8887/files/faab3ea1..c2cc3391

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=8887=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=8887=00-01

  Stats: 2 lines in 1 file changed: 1 ins; 1 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8887.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8887/head:pull/8887

PR: https://git.openjdk.java.net/jdk/pull/8887


RFR: 8287187: Utilize HashMap.newHashMap() in CLDRConverter

2022-05-25 Thread Naoto Sato
Refactoring the leftover self-calculations of the optimized `HashMap` initial 
value with `newHashMap()` method. Also replaced some string literals using text 
blocks for better readability. Confirmed that the output resource bundle 
sources are effectively (sans indentation) the same as the original ones.

-

Commit messages:
 - 8287187: Utilize HashMap.newHashMap() in CLDRConverter

Changes: https://git.openjdk.java.net/jdk/pull/8887/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8887=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8287187
  Stats: 65 lines in 1 file changed: 8 ins; 1 del; 56 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8887.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8887/head:pull/8887

PR: https://git.openjdk.java.net/jdk/pull/8887


Integrated: 8286399: Address possibly lossy conversions in JDK Build Tools

2022-05-16 Thread Naoto Sato
On Fri, 13 May 2022 17:05:43 GMT, Naoto Sato  wrote:

> Applied required casts for the upcoming warning. Verified by cherry-picking 
> Adam's patch.

This pull request has now been integrated.

Changeset: c044cb83
Author:    Naoto Sato 
URL:   
https://git.openjdk.java.net/jdk/commit/c044cb8346bb8fbba46db1debe921cf96885ada0
Stats: 5 lines in 2 files changed: 0 ins; 0 del; 5 mod

8286399: Address possibly lossy conversions in JDK Build Tools

Reviewed-by: rriggs, joehw

-

PR: https://git.openjdk.java.net/jdk/pull/8706


Re: RFR: 8286399: Address possibly lossy conversions in JDK Build Tools

2022-05-13 Thread Naoto Sato
On Fri, 13 May 2022 22:11:17 GMT, Joe Wang  wrote:

>> Applied required casts for the upcoming warning. Verified by cherry-picking 
>> Adam's patch.
>
> make/jdk/src/classes/build/tools/generatebreakiteratordata/RuleBasedBreakIteratorBuilder.java
>  line 1278:
> 
>> 1276: state[numCategories] |= (short) END_STATE_FLAG;
>> 1277: if (sawEarlyBreak) {
>> 1278: state[numCategories] |= LOOKAHEAD_STATE_FLAG;
> 
> Does this need a cast as well? and also other cases, e.g. line 1019: 
> state[numCategories] = DONT_LOOP_FLAG;?

No, it doesn't. `LOOKAHEAD_STATE_FLAG` is defined as `0x2000` which is within 
the range of `short`. OTOH, `END_STATE_FLAG` is `0x8000` thus a lossy 
conversion.

-

PR: https://git.openjdk.java.net/jdk/pull/8706


RFR: 8286399: Address possibly lossy conversions in JDK Build Tools

2022-05-13 Thread Naoto Sato
Applied required casts for the upcoming warning. Verified by cherry-picking 
Adam's patch.

-

Commit messages:
 - 8286399: Address possibly lossy conversions in JDK Build Tools

Changes: https://git.openjdk.java.net/jdk/pull/8706/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8706=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8286399
  Stats: 5 lines in 2 files changed: 0 ins; 0 del; 5 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8706.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8706/head:pull/8706

PR: https://git.openjdk.java.net/jdk/pull/8706


Integrated: 8283324: CLDRConverter run time increased by 3x

2022-04-21 Thread Naoto Sato
On Mon, 18 Apr 2022 23:16:18 GMT, Naoto Sato  wrote:

> Fixing performance regression caused by the fix to 
> https://bugs.openjdk.java.net/browse/JDK-8176706. The fix introduced extra 
> looping through the resource map multiple times which was not necessary. The 
> execution time of the tool now got back on par with close to JDK18.

This pull request has now been integrated.

Changeset: f6e9ca0c
Author:    Naoto Sato 
URL:   
https://git.openjdk.java.net/jdk/commit/f6e9ca0cbe671502b6b3b1d0f8fd86f0928f64ea
Stats: 16 lines in 2 files changed: 10 ins; 0 del; 6 mod

8283324: CLDRConverter run time increased by 3x

Reviewed-by: ihse

-

PR: https://git.openjdk.java.net/jdk/pull/8288


RFR: 8283324: CLDRConverter run time increased by 3x

2022-04-20 Thread Naoto Sato
Fixing performance regression caused by the fix to 
https://bugs.openjdk.java.net/browse/JDK-8176706. The fix introduced extra 
looping through the resource map multiple times which was not necessary. The 
execution time of the tool now got back on par with close to JDK18.

-

Commit messages:
 - 8283324: CLDRConverter run time increased by 3x

Changes: https://git.openjdk.java.net/jdk/pull/8288/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8288=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8283324
  Stats: 16 lines in 2 files changed: 10 ins; 0 del; 6 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8288.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8288/head:pull/8288

PR: https://git.openjdk.java.net/jdk/pull/8288


Integrated: 8265315: Support for CLDR version 41

2022-04-11 Thread Naoto Sato
On Thu, 7 Apr 2022 21:20:20 GMT, Naoto Sato  wrote:

> This is to upgrade the CLDR data from version 39 to version 41 which was 
> released yesterday. The vast majority of the changes are basically replacing 
> the CLDR data, along with tools/testcase alignments. Here is the link to CLDR 
> v41's release notes: https://cldr.unicode.org/index/downloads/cldr-41

This pull request has now been integrated.

Changeset: 523899e3
Author:    Naoto Sato 
URL:   
https://git.openjdk.java.net/jdk/commit/523899e36c543343283ab0b1f5cfcba805e7b918
Stats: 132400 lines in 859 files changed: 96406 ins; 4216 del; 31778 mod

8265315: Support for CLDR version 41

Reviewed-by: joehw, iris, ihse

-

PR: https://git.openjdk.java.net/jdk/pull/8150


Re: RFR: 8265315: Support for CLDR version 41 [v2]

2022-04-08 Thread Naoto Sato
> This is to upgrade the CLDR data from version 39 to version 41 which was 
> released yesterday. The vast majority of the changes are basically replacing 
> the CLDR data, along with tools/testcase alignments. Here is the link to CLDR 
> v41's release notes: https://cldr.unicode.org/index/downloads/cldr-41

Naoto Sato has updated the pull request with a new target base due to a merge 
or a rebase. The pull request now contains 22 commits:

 - Merge branch 'master' into cldr+
 - Merge branch 'master' into cldr+
 - CLDR v41 final
 - CLDR v41 beta2
 - Merge branch 'master' into cldr+
 - CLDR v41 alpha4
 - Merge branch 'master' into cldr+
 - Update copyright year to 2022
 - CLDR release v40
 - Merge branch 'master' into cldr
 - ... and 12 more: https://git.openjdk.java.net/jdk/compare/a8c87526...9ef22a6d

-

Changes: https://git.openjdk.java.net/jdk/pull/8150/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8150=01
  Stats: 132400 lines in 859 files changed: 96406 ins; 4216 del; 31778 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8150.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8150/head:pull/8150

PR: https://git.openjdk.java.net/jdk/pull/8150


Re: RFR: 8265315: Support for CLDR version 41

2022-04-08 Thread Naoto Sato
On Thu, 7 Apr 2022 21:20:20 GMT, Naoto Sato  wrote:

> This is to upgrade the CLDR data from version 39 to version 41 which was 
> released yesterday. The vast majority of the changes are basically replacing 
> the CLDR data, along with tools/testcase alignments. Here is the link to CLDR 
> v41's release notes: https://cldr.unicode.org/index/downloads/cldr-41

I measured the execution time of `CLDRConverter`. Each run measures an average 
execution time for 5 invocations of `CLDRConverter.main()`, and I ran it 5 
times for `java.base` and `jdk.localedata` modules. All average values are in 
seconds.

- Before the fix
  - java.base: 11.536 - 12.836
  - jdk.localedata: 19.764 - 22.584
- After the fix
  - java.base: 11.666 - 12.833
  - jdk.localedata: 19.183 - 20.838
 
Looking at the results, I don't see any notable tool execution time difference 
with this change.

-

PR: https://git.openjdk.java.net/jdk/pull/8150


RFR: 8265315: Support for CLDR version 41

2022-04-07 Thread Naoto Sato
This is to upgrade the CLDR data from version 39 to version 41 which was 
released yesterday. The vast majority of the changes are basically replacing 
the CLDR data, along with tools/testcase alignments. Here is the link to CLDR 
v41's release notes: https://cldr.unicode.org/index/downloads/cldr-41

-

Commit messages:
 - Merge branch 'master' into cldr+
 - CLDR v41 final
 - CLDR v41 beta2
 - Merge branch 'master' into cldr+
 - CLDR v41 alpha4
 - Merge branch 'master' into cldr+
 - Update copyright year to 2022
 - CLDR release v40
 - Merge branch 'master' into cldr
 - CLDR tip
 - ... and 11 more: https://git.openjdk.java.net/jdk/compare/526e7349...ee6985a4

Changes: https://git.openjdk.java.net/jdk/pull/8150/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=8150=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8265315
  Stats: 132401 lines in 859 files changed: 96406 ins; 4216 del; 31779 mod
  Patch: https://git.openjdk.java.net/jdk/pull/8150.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/8150/head:pull/8150

PR: https://git.openjdk.java.net/jdk/pull/8150


Integrated: 8283277: ISO 4217 Amendment 171 Update

2022-03-21 Thread Naoto Sato
On Thu, 17 Mar 2022 18:10:17 GMT, Naoto Sato  wrote:

> This is to incorporate the ISO 4217 amendment 171 for Sierra Leonean LEONE 
> redenomination (removing 3 zeros). Its effective date is 4/1, but I went 
> ahead as JDK19 won't be released by 4/1.

This pull request has now been integrated.

Changeset: c4dc58e1
Author:    Naoto Sato 
URL:   
https://git.openjdk.java.net/jdk/commit/c4dc58e12e197562dce90c0027aa74c29047cea6
Stats: 17 lines in 6 files changed: 3 ins; 0 del; 14 mod

8283277: ISO 4217 Amendment 171 Update

Reviewed-by: iris, joehw

-

PR: https://git.openjdk.java.net/jdk/pull/7857


RFR: 8283277: ISO 4217 Amendment 171 Update

2022-03-17 Thread Naoto Sato
This is to incorporate the ISO 4217 amendment 171 for Sierra Leonean LEONE 
redenomination (removing 3 zeros). Its effective date is 4/1, but I went ahead 
as JDK19 won't be released by 4/1.

-

Commit messages:
 - 8283277: ISO 4217 Amendment 171 Update

Changes: https://git.openjdk.java.net/jdk/pull/7857/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7857=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8283277
  Stats: 17 lines in 6 files changed: 3 ins; 0 del; 14 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7857.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7857/head:pull/7857

PR: https://git.openjdk.java.net/jdk/pull/7857


Re: RFR: 8257733: Move module-specific data from make to respective module [v9]

2022-03-17 Thread Naoto Sato
On Thu, 17 Mar 2022 00:12:38 GMT, Magnus Ihse Bursie  wrote:

>> A lot (but not all) of the data in make/data is tied to a specific module. 
>> For instance, the publicsuffixlist is used by java.base, and fontconfig by 
>> java.desktop. (A few directories, like mainmanifest, is *actually* used by 
>> make for the whole build.) 
>> 
>> These data files should move to the module they belong to. The are, after 
>> all, "source code" for that module that is "compiler" into resulting 
>> deliverables, for that module. (But the "source code" language is not Java 
>> or C, but typically a highly domain specific language or data format, and 
>> the "compilation" is, often, a specialized transformation.) 
>> 
>> This misplacement of the data directory is most visible at code review time. 
>> When such data is changed, most of the time build-dev (or the new build 
>> label) is involved, even though this has nothing to do with the build. While 
>> this is annoying, a worse problem is if the actual team that needs to review 
>> the patch (i.e., the team owning the module) is missed in the review.
>> 
>> ### Modules reviewed
>> 
>> - [x] java.base
>> - [x] java.desktop
>> - [x] jdk.compiler
>> - [x] java.se
>
> Magnus Ihse Bursie has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Fix typos

LGTM. Thanks for the change, Magnus!

-

Marked as reviewed by naoto (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1611


Re: RFR: 8257733: Move module-specific data from make to respective module [v6]

2022-03-16 Thread Naoto Sato
On Wed, 16 Mar 2022 21:56:53 GMT, Magnus Ihse Bursie  wrote:

>> make/modules/jdk.charsets/Gensrc.gmk line 32:
>> 
>>> 30: # Generate files using the charsetmapping tool
>>> 31: #
>>> 32: CHARSET_DATA_DIR := $(TOPDIR)/src/java.base/share/data/charsetmapping
>> 
>> Is it intentional to leave `java.base` literal here, or should it be 
>> replaced with `$(MODULE_SRC)`? I see this inconsistency in other tools' 
>> `gensrc.gmk` too
>
> This is part of the weirdness of charsetmapping that Alan talks about. The 
> charsetmapping data is shared between java.base and jdk.charsets in a way 
> that makes it non-trivial to disentangle. 
> 
> So this reference to java.base is quite intentional -- replacing it with 
> $(MODULE_SRC) would have pointed to src/jdk.charsets instead of 
> src/java.base, which would have been incorrect.

Thanks for reminding me. Then yes, I'd agree with Alan. It'd be much simpler to 
exclude this from this PR.

-

PR: https://git.openjdk.java.net/jdk/pull/1611


Re: RFR: 8257733: Move module-specific data from make to respective module [v6]

2022-03-16 Thread Naoto Sato
On Tue, 15 Mar 2022 23:50:20 GMT, Magnus Ihse Bursie  wrote:

>> A lot (but not all) of the data in make/data is tied to a specific module. 
>> For instance, the publicsuffixlist is used by java.base, and fontconfig by 
>> java.desktop. (A few directories, like mainmanifest, is *actually* used by 
>> make for the whole build.) 
>> 
>> These data files should move to the module they belong to. The are, after 
>> all, "source code" for that module that is "compiler" into resulting 
>> deliverables, for that module. (But the "source code" language is not Java 
>> or C, but typically a highly domain specific language or data format, and 
>> the "compilation" is, often, a specialized transformation.) 
>> 
>> This misplacement of the data directory is most visible at code review time. 
>> When such data is changed, most of the time build-dev (or the new build 
>> label) is involved, even though this has nothing to do with the build. While 
>> this is annoying, a worse problem is if the actual team that needs to review 
>> the patch (i.e., the team owning the module) is missed in the review.
>> 
>> ### Modules reviewed
>> 
>> - [x] java.base
>> - [x] java.desktop
>> - [x] jdk.compiler
>> - [x] java.se
>
> Magnus Ihse Bursie has updated the pull request with a new target base due to 
> a merge or a rebase. The pull request now contains 12 commits:
> 
>  - Merge branch 'master' into shuffle-data-reborn
>  - Fix merge
>  - Merge tag 'jdk-19+13' into shuffle-data-reborn
>
>Added tag jdk-19+13 for changeset 5df2a057
>  - Move characterdata templates to share/classes/java/lang.
>  - Update comment refering to "make" dir
>  - Move new symbols to jdk.compiler
>  - Merge branch 'master' into shuffle-data
>  - Move macosxicons from share to macosx
>  - Move to share/data, and move jdwp.spec to java.se
>  - Update references in test
>  - ... and 2 more: 
> https://git.openjdk.java.net/jdk/compare/83d77186...598f740f

Looks good, with minor comments. Also I am assuming you will modify the 
copyright year for these files before the merge.

make/modules/jdk.charsets/Gensrc.gmk line 32:

> 30: # Generate files using the charsetmapping tool
> 31: #
> 32: CHARSET_DATA_DIR := $(TOPDIR)/src/java.base/share/data/charsetmapping

Is it intentional to leave `java.base` literal here, or should it be replaced 
with `$(MODULE_SRC)`? I see this inconsistency in other tools' `gensrc.gmk` too

-

Marked as reviewed by naoto (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1611


Re: RFR: JDK-8280902 ResourceBundle::getBundle may throw NPE when invoked by JNI code with no caller frame

2022-03-02 Thread Naoto Sato
On Wed, 2 Mar 2022 18:56:40 GMT, Tim Prinzing  wrote:

> The caller class returned by Reflection::getCallerClass was used to gain 
> access to it's module in most cases and class loader in one case. I added a 
> method to translate the caller class to caller module so that the decision of 
> what module represents the caller with no stack frame is made in a single 
> place. Calls made to caller.getModule() were replaced with 
> getCallerModule(caller) which returns the system class loader unnamed module 
> if the caller is null.
> 
> The one place a class loader was produced from the caller in getBundleImpl it 
> was rewritten to route through the getCallerModule method:
> 
> final ClassLoader loader = (caller != null) ?
> caller.getClassLoader() : getLoader(getCallerModule(caller));
> 
> A JNI test was added which calls getBundle to fetch a test bundle from a 
> location added to the classpath, fetches a string out of the bundle and 
> verifies it, and calls clearCache.
> 
> The javadoc was updated for the caller sensitive methods changed.

Looks good to me. I believe a CSR is needed for this change.

-

Marked as reviewed by naoto (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7663


Re: RFR: 8176706: Additional Date-Time Formats [v3]

2022-02-10 Thread Naoto Sato
On Thu, 10 Feb 2022 22:20:48 GMT, Roger Riggs  wrote:

>> Naoto Sato has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Fixed LocalizedPrinterParser.toString() to reflect requestedTemplate
>
> src/java.base/share/classes/sun/util/locale/provider/LocaleResources.java 
> line 690:
> 
>> 688: private String resolveInputSkeleton(String type) {
>> 689: var regionToSkeletonMap = inputSkeletons.get(type);
>> 690: return regionToSkeletonMap.getOrDefault(locale.getLanguage() + 
>> "-" + locale.getCountry(),
> 
> This structure computes all the defaults even if the value isn't needed 
> (because the value has to be passed to the `getOrDefault` method.  Perhaps 
> performance isn't an issue.

Indeed, yes. I thought this was simple enough, and as you said, not 
performance-critical.

-

PR: https://git.openjdk.java.net/jdk/pull/7340


Re: RFR: 8176706: Additional Date-Time Formats [v4]

2022-02-10 Thread Naoto Sato
> Following the prior discussion [1], here is the PR for the subject 
> enhancement. CSR has also been updated according to the suggestion.
> 
> [1] 
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2022-January/085175.html

Naoto Sato has updated the pull request incrementally with one additional 
commit since the last revision:

  Addressing review comments

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7340/files
  - new: https://git.openjdk.java.net/jdk/pull/7340/files/41408ff3..af3c0d62

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7340=03
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7340=02-03

  Stats: 33 lines in 5 files changed: 1 ins; 2 del; 30 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7340.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7340/head:pull/7340

PR: https://git.openjdk.java.net/jdk/pull/7340


Re: RFR: 8176706: Additional Date-Time Formats [v3]

2022-02-08 Thread Naoto Sato
> Following the prior discussion [1], here is the PR for the subject 
> enhancement. CSR has also been updated according to the suggestion.
> 
> [1] 
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2022-January/085175.html

Naoto Sato has updated the pull request incrementally with one additional 
commit since the last revision:

  Fixed LocalizedPrinterParser.toString() to reflect requestedTemplate

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7340/files
  - new: https://git.openjdk.java.net/jdk/pull/7340/files/059132f5..41408ff3

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7340=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7340=01-02

  Stats: 3 lines in 1 file changed: 1 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7340.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7340/head:pull/7340

PR: https://git.openjdk.java.net/jdk/pull/7340


Re: RFR: 8176706: Additional Date-Time Formats [v2]

2022-02-08 Thread Naoto Sato
On Mon, 7 Feb 2022 21:22:12 GMT, Roger Riggs  wrote:

>> Naoto Sato has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Modified per suggestions on the PR
>
> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java 
> line 5162:
> 
>> 5160: 
>> 5161: formatter = new 
>> DateTimeFormatterBuilder().appendPattern(pattern).toFormatter(locale);
>> 5162: DateTimeFormatter old = 
>> FORMATTER_CACHE.putIfAbsent(key, formatter);
> 
> .computeIfAbsent(key, () -> {}) might be a bit cleaner/clearer here and 
> avoid the race a few lines of code below. (a slight improvement in old code).

Good point. Changed to `computeIfAbsent()`.

-

PR: https://git.openjdk.java.net/jdk/pull/7340


Re: RFR: 8176706: Additional Date-Time Formats [v2]

2022-02-08 Thread Naoto Sato
On Tue, 8 Feb 2022 00:39:04 GMT, Joe Wang  wrote:

>> Naoto Sato has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Modified per suggestions on the PR
>
> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java 
> line 254:
> 
>> 252: public static String getLocalizedDateTimePattern(String 
>> requestedTemplate,
>> 253:  Chronology chrono, 
>> Locale locale) {
>> 254: Objects.requireNonNull(locale, "requestedTemplate");
> 
> Typo, "locale" should have been requestedTemplate.

Good catch!

> src/java.base/share/classes/sun/util/locale/provider/JavaTimeDateTimePatternImpl.java
>  line 77:
> 
>> 75: LocaleProviderAdapter lpa = 
>> LocaleProviderAdapter.getResourceBundleBased();
>> 76: // CLDR's 'u'/'U' are not supported in the JDK. Replace them 
>> with 'y' instead
>> 77: final var modifiedSkeleton = 
>> requestedTemplate.replaceAll("[uU]", "y");
> 
> Seems to me requestedTemplate needs to be validated when it gets passed to 
> getLocalizedDateTimePattern,  similar as to appendLocalized

This was a left over which should be removed. In fact, CLDR specific pattern 
symbols should not be accepted as the requested template. Removed the 
substitutions. As to the validation, it will be performed in the following 
`LocaleResources.getLocalizedPattern()` method.

-

PR: https://git.openjdk.java.net/jdk/pull/7340


Re: RFR: 8176706: Additional Date-Time Formats [v2]

2022-02-08 Thread Naoto Sato
> Following the prior discussion [1], here is the PR for the subject 
> enhancement. CSR has also been updated according to the suggestion.
> 
> [1] 
> https://mail.openjdk.java.net/pipermail/core-libs-dev/2022-January/085175.html

Naoto Sato has updated the pull request incrementally with one additional 
commit since the last revision:

  Modified per suggestions on the PR

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/7340/files
  - new: https://git.openjdk.java.net/jdk/pull/7340/files/f9311dce..059132f5

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=7340=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=7340=00-01

  Stats: 80 lines in 4 files changed: 23 ins; 37 del; 20 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7340.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7340/head:pull/7340

PR: https://git.openjdk.java.net/jdk/pull/7340


RFR: 8176706: Additional Date-Time Formats

2022-02-03 Thread Naoto Sato
Following the prior discussion [1], here is the PR for the subject enhancement. 
CSR has also been updated according to the suggestion.

[1] 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2022-January/085175.html

-

Commit messages:
 - Removed trailing space
 - Merge branch 'master' into skeleton
 - Tidying up
 - Some more tests
 - Brought back the part that was mistakenly removed
 - Merge branch 'master' into skeleton
 - Reverted IllegalArgumentException back
 - Further cleanups
 - Removed exceptions from ofLocalizedPattern() method, as they are lazily 
thrown on format()
 - Cleanup
 - ... and 19 more: https://git.openjdk.java.net/jdk/compare/cda9c301...f9311dce

Changes: https://git.openjdk.java.net/jdk/pull/7340/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=7340=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8176706
  Stats: 777 lines in 12 files changed: 733 ins; 9 del; 35 mod
  Patch: https://git.openjdk.java.net/jdk/pull/7340.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/7340/head:pull/7340

PR: https://git.openjdk.java.net/jdk/pull/7340


Integrated: 8268081: Upgrade Unicode Data Files to 14.0.0

2022-01-12 Thread Naoto Sato
On Wed, 5 Jan 2022 22:42:38 GMT, Naoto Sato  wrote:

> Please review the changes for upgrading the Unicode support in the JDK, from 
> version 13 to version 14. Corresponding CSR has also been drafted.

This pull request has now been integrated.

Changeset: 0a094d7c
Author:    Naoto Sato 
URL:   
https://git.openjdk.java.net/jdk/commit/0a094d7c286ed0b5a35c517391e48c603cb43a68
Stats: 3443 lines in 41 files changed: 2353 ins; 101 del; 989 mod

8268081: Upgrade Unicode Data Files to 14.0.0

Reviewed-by: joehw, iris, lancea

-

PR: https://git.openjdk.java.net/jdk/pull/6974


RFR: 8268081: Support for Unicode 14

2022-01-05 Thread Naoto Sato
Please review the changes for upgrading the Unicode support in the JDK, from 
version 13 to version 14. Corresponding CSR has also been drafted.

-

Commit messages:
 - Amend unicode.md and icu.md files
 - Minor fixup
 - Merge branch 'master' into unicode
 - Copyright year to 2022
 - ICU4J 70.1
 - 18 -> 19
 - Merge branch 'master' into unicode
 - Unicode 14.0.0 (final)
 - UCD ver. 14.0 (beta) / Unicode Text Segmentation rev. 38 (draft)
 - ICU4J 69.1

Changes: https://git.openjdk.java.net/jdk/pull/6974/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=6974=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8268081
  Stats: 3443 lines in 41 files changed: 2353 ins; 101 del; 989 mod
  Patch: https://git.openjdk.java.net/jdk/pull/6974.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/6974/head:pull/6974

PR: https://git.openjdk.java.net/jdk/pull/6974


Re: RFR: 8275766: (tz) Update Timezone Data to 2021e

2021-10-28 Thread Naoto Sato
On Thu, 28 Oct 2021 01:02:27 GMT, Yoshiki Sato  wrote:

> Please review the integration of tzdata2021e (including tzdata2021d) to the 
> JDK.
> The fix has passed all relevant JTREG regression tests and JCK tests. 
> 
> 8275754: (tz) Update Timezone Data to 2021d
> 8275849: TestZoneInfo310.java fails with tzdata2021e

Marked as reviewed by naoto (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/6144


Integrated: 8274407: (tz) Update Timezone Data to 2021c

2021-10-07 Thread Naoto Sato
On Wed, 6 Oct 2021 01:24:49 GMT, Naoto Sato  wrote:

> This PR is to upgrade the time zone data in the JDK to IANA's tzdata2021c 
> level. Note that the tz data is "as is", as released by IANA. No `merged 
> links` are retracted.
> The PR also fixes two issues along with the 2021c upgrade.

This pull request has now been integrated.

Changeset: 8ca08461
Author:Naoto Sato 
URL:   
https://git.openjdk.java.net/jdk/commit/8ca084617f331b6af934179f3f776c8158da5bba
Stats: 635 lines in 13 files changed: 228 ins; 329 del; 78 mod

8274407: (tz) Update Timezone Data to 2021c
8274467: TestZoneInfo310.java fails with tzdata2021b
8274468: TimeZoneTest.java fails with tzdata2021b

Reviewed-by: rriggs, iris, coffeys

-

PR: https://git.openjdk.java.net/jdk/pull/5832


Re: RFR: 8274407: (tz) Update Timezone Data to 2021c

2021-10-06 Thread Naoto Sato
On Wed, 6 Oct 2021 19:43:06 GMT, Sean Coffey  wrote:

>> This PR is to upgrade the time zone data in the JDK to IANA's tzdata2021c 
>> level. Note that the tz data is "as is", as released by IANA. No `merged 
>> links` are retracted.
>> The PR also fixes two issues along with the 2021c upgrade.
>
> pre-1970 data for some time zones is lost or becomes inaccurate with this 
> tzdata release. Do we plan to release-note that point  ?

Good point, @coffeys. I'll make sure they are described in the release note.

-

PR: https://git.openjdk.java.net/jdk/pull/5832


RFR: 8274407: (tz) Update Timezone Data to 2021c

2021-10-05 Thread Naoto Sato
This PR is to upgrade the time zone data in the JDK to IANA's tzdata2021c 
level. Note that the tz data is "as is", as released by IANA. No `merged links` 
are retracted.
The PR also fixes two issues along with the 2021c upgrade.

-

Commit messages:
 - Fix for Asia/Amman test case error
 - Test fix for Samoa not observing DST
 - tzdata2021c
 - 8274407: (tz) Update Timezone Data to 2021b

Changes: https://git.openjdk.java.net/jdk/pull/5832/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5832=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8274407
  Stats: 635 lines in 13 files changed: 228 ins; 329 del; 78 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5832.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5832/head:pull/5832

PR: https://git.openjdk.java.net/jdk/pull/5832


Integrated: 8274658: ISO 4217 Amendment 170 Update

2021-10-04 Thread Naoto Sato
On Fri, 1 Oct 2021 18:57:28 GMT, Naoto Sato  wrote:

> This is to incorporate the ISO 4217 amendment #170, which has been released 
> today, effective immediately.

This pull request has now been integrated.

Changeset: f2404d60
Author:    Naoto Sato 
URL:   
https://git.openjdk.java.net/jdk/commit/f2404d60de2b58c590bf885f5cce50c289073673
Stats: 15 lines in 6 files changed: 3 ins; 0 del; 12 mod

8274658: ISO 4217 Amendment 170 Update

Reviewed-by: lancea, iris

-

PR: https://git.openjdk.java.net/jdk/pull/5790


RFR: 8274658: ISO 4217 Amendment #170 Update

2021-10-01 Thread Naoto Sato
This is to incorporate the ISO 4217 amendment #170, which has been released 
today, effective immediately.

-

Commit messages:
 - 8274658: ISO 4217 Amendment #170 Update

Changes: https://git.openjdk.java.net/jdk/pull/5790/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5790=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8274658
  Stats: 15 lines in 6 files changed: 3 ins; 0 del; 12 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5790.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5790/head:pull/5790

PR: https://git.openjdk.java.net/jdk/pull/5790


Re: RFR: 8272805: Avoid looking up standard charsets [v2]

2021-08-22 Thread Naoto Sato
On Sun, 22 Aug 2021 23:02:06 GMT, Sergey Bylokhov  wrote:

>> This is the continuation of JDK-8233884, JDK-8271456, and JDK-8272120.
>> 
>> In many places standard charsets are looked up via their names, for example:
>> absolutePath.getBytes("UTF-8");
>> 
>> This could be done more efficiently(up to x20 time faster) with use of 
>> java.nio.charset.StandardCharsets:
>> absolutePath.getBytes(StandardCharsets.UTF_8);
>> 
>> The later variant also makes the code cleaner, as it is known not to throw 
>> UnsupportedEncodingException in contrary to the former variant.
>> 
>> This change includes:
>>  * demo/utils
>>  * jdk.xx packages
>>  * Some places were missed in the previous changes. I have found it by 
>> tracing the calls to the Charset.forName() by executing tier1,2,3 and 
>> desktop tests.
>> 
>> Some performance discussion: https://github.com/openjdk/jdk/pull/5063
>> 
>> Code excluded in this fix: the Xerces library(should be fixed upstream), 
>> J2DBench(should be compatible to 1.4), some code in the network(the change 
>> there are not straightforward, will do it later).
>> 
>> Tested by the tier1/tier2/tier3 tests on Linux/Windows/macOS.
>
> Sergey Bylokhov has updated the pull request with a new target base due to a 
> merge or a rebase. The incremental webrev excludes the unrelated changes 
> brought in by the merge/rebase. The pull request contains 14 additional 
> commits since the last revision:
> 
>  - Update the usage of Files.readAllLines()
>  - Update datatransfer
>  - Merge branch 'master' into standard-encodings-in-non-public-modules
>  - Merge branch 'master' into standard-encodings-in-non-public-modules
>  - Fix related imports
>  - Merge branch 'master' into standard-encodings-in-non-public-modules
>  - Cleanup UnsupportedEncodingException
>  - Update PacketStream.java
>  - Rollback TextTests, should be compatible with jdk1.4
>  - Rollback TextRenderTests, should be compatible with jdk1.4
>  - ... and 4 more: 
> https://git.openjdk.java.net/jdk/compare/c262b06f...e7127644

Looks good.

-

Marked as reviewed by naoto (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/5210


Integrated: 8264792: The NumberFormat for locale sq_XK formats price incorrectly.

2021-08-09 Thread Naoto Sato
On Fri, 6 Aug 2021 16:39:34 GMT, Naoto Sato  wrote:

> Please review the fix to the subject issue. The root cause of this problem is 
> that the currency for the country code `XK` is undefined because the country 
> code is user-defined in the ISO 3166 standard. However, it is commonly used 
> to represent the region `Kosovo`, which CLDR supports and subsequently is 
> supported by the JDK since JDK9. Adding the data `EUR` for the country code 
> `XK` will fix the issue.

This pull request has now been integrated.

Changeset: 41dc795d
Author:Naoto Sato 
URL:   
https://git.openjdk.java.net/jdk/commit/41dc795d6c08af84aa6544cc5a5704dcf99386cf
Stats: 13 lines in 3 files changed: 6 ins; 0 del; 7 mod

8264792: The NumberFormat for locale sq_XK formats price incorrectly.

Reviewed-by: joehw, iris

-

PR: https://git.openjdk.java.net/jdk/pull/5033


RFR: 8264792: The NumberFormat for locale sq_XK formats price incorrectly.

2021-08-06 Thread Naoto Sato
Please review the fix to the subject issue. The root cause of this problem is 
that the currency for the country code `XK` is undefined because the country 
code is user-defined in the ISO 3166 standard. However, it is commonly used to 
represent the region `Kosovo`, which CLDR supports and subsequently is 
supported by the JDK since JDK9. Adding the data `EUR` for the country code 
`XK` will fix the issue.

-

Commit messages:
 - 8264792: The NumberFormat for locale sq_XK formats price incorrectly.

Changes: https://git.openjdk.java.net/jdk/pull/5033/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=5033=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8264792
  Stats: 13 lines in 3 files changed: 6 ins; 0 del; 7 mod
  Patch: https://git.openjdk.java.net/jdk/pull/5033.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/5033/head:pull/5033

PR: https://git.openjdk.java.net/jdk/pull/5033


Re: RFR: JDK-8266254: Update to use jtreg 6 [v2]

2021-06-02 Thread Naoto Sato
On Wed, 2 Jun 2021 20:15:55 GMT, Jonathan Gibbons  wrote:

>> Please review the change to update to using jtreg 6. 
>> 
>> The primary change is to the jib-profiles.js file, which specifies the 
>> version of jtreg to use, for those systems that rely on this file. In 
>> addition, the `requiredVersion` has been updated in the various `TEST.ROOT` 
>> files.
>> 
>> All the tests that could be updated ahead of time have been updated. There 
>> are a few tests remaining that need to be done at this time, because of the 
>> change in the module name for TestNG 7.3. It changed from a default of 
>> `testng` to and explicit `org.testng`.
>
> Jonathan Gibbons has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update copyright years

Marked as reviewed by naoto (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/4315


Re: RFR: JDK-8266254: Update to use jtreg 6

2021-06-02 Thread Naoto Sato
On Wed, 2 Jun 2021 16:13:48 GMT, Jonathan Gibbons  wrote:

> Please review the change to update to using jtreg 6. 
> 
> The primary change is to the jib-profiles.js file, which specifies the 
> version of jtreg to use, for those systems that rely on this file. In 
> addition, the `requiredVersion` has been updated in the various `TEST.ROOT` 
> files.
> 
> All the tests that could be updated ahead of time have been updated. There 
> are a few tests remaining that need to be done at this time, because of the 
> change in the module name for TestNG 7.3. It changed from a default of 
> `testng` to and explicit `org.testng`.

Some of the modified files have copyright year left unchanged. `2021` needs to 
be appended.

-

PR: https://git.openjdk.java.net/jdk/pull/4315


Re: RFR: 8266459: Implement JEP 411: Deprecate the Security Manager for Removal

2021-05-18 Thread Naoto Sato
On Mon, 17 May 2021 18:23:41 GMT, Weijun Wang  wrote:

> Please review this implementation of [JEP 
> 411](https://openjdk.java.net/jeps/411).
> 
> The code change is divided into 3 commits. Please review them one by one.
> 
> 1. 
> https://github.com/openjdk/jdk/commit/576161d15423f58281e384174d28c9f9be7941a1
>  The essential change for this JEP, including the `@Deprecate` annotations 
> and spec change. It also update the default value of the 
> `java.security.manager` system property to "disallow", and necessary test 
> change following this update.
> 2. 
> https://github.com/openjdk/jdk/commit/26a54a835e9f84aa528740a7c5c35d07355a8a66
>  Manual changes to several files so that the next commit can be generated 
> programatically.
> 3. 
> https://github.com/openjdk/jdk/commit/eb6c566ff9207974a03a53335e0e697cffcf0950
>  Automatic changes to other source files to avoid javac warnings on 
> deprecation for removal
> 
> The 1st and 2nd commits should be reviewed carefully. The 3rd one is 
> generated programmatically, see the comment below for more details. If you 
> are only interested in a portion of the 3rd commit and would like to review 
> it as a separate file, please comment here and I'll generate an individual 
> webrev.
> 
> Due to the size of this PR, no attempt is made to update copyright years for 
> any file to minimize unnecessary merge conflict.
> 
> Furthermore, since the default value of `java.security.manager` system 
> property is now "disallow", most of the tests calling 
> `System.setSecurityManager()` need to launched with 
> `-Djava.security.manager=allow`. This is covered in a different PR at 
> https://github.com/openjdk/jdk/pull/4071.

Reviewed i18n/java.time/charset. They all look good.

-

Marked as reviewed by naoto (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/4073


RFR: 8263202: Update Hebrew/Indonesian/Yiddish ISO 639 language codes to current

2021-05-17 Thread Naoto Sato
Please review the changes to the subject issue. java.util.Locale class has a 
long-standing issue for those obsolete ISO 639 languages where its 
normalization ends up in the obsolete codes. This change intends to flip the 
normalization towards the current codes, providing a system property for 
compatibility behavior. ResourceBundle class is also modified to load either 
obsolete/current bundles. For more detail, take a look at the CSR.

-

Commit messages:
 - Added more ResourceBundleProvider tests
 - ResourceBundleProvider and test cases modifications
 - Eliminated some duplicated code
 - Changed ResourceBundle javadoc
 - Locale class description
 - renamed old resource files
 - inital commit

Changes: https://git.openjdk.java.net/jdk/pull/4069/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=4069=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8263202
  Stats: 382 lines in 35 files changed: 239 ins; 41 del; 102 mod
  Patch: https://git.openjdk.java.net/jdk/pull/4069.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/4069/head:pull/4069

PR: https://git.openjdk.java.net/jdk/pull/4069


Integrated: 8258795: Update IANA Language Subtag Registry to Version 2021-05-11

2021-05-13 Thread Naoto Sato
On Wed, 12 May 2021 16:28:54 GMT, Naoto Sato  wrote:

> Please review the changes to the subject issue. This is to incorporate the 
> latest language subtag registry definition into the JDK.

This pull request has now been integrated.

Changeset: a259ab4a
Author:    Naoto Sato 
URL:   
https://git.openjdk.java.net/jdk/commit/a259ab4a8d163ff924ba56c5da5395cec0d8c350
Stats: 340 lines in 2 files changed: 327 ins; 0 del; 13 mod

8258795: Update IANA Language Subtag Registry to Version 2021-05-11

Reviewed-by: joehw

-

PR: https://git.openjdk.java.net/jdk/pull/3998


RFR: 8258795: Update IANA Language Subtag Registry to Version 2021-05-11

2021-05-12 Thread Naoto Sato
Please review the changes to the subject issue. This is to incorporate the 
latest language subtag registry definition into the JDK.

-

Commit messages:
 - Renaming the test case
 - LSR 2021-05-11
 - LSR 2021-03-05
 - LSR 2021-02-23
 - LSR 2020-12-18

Changes: https://git.openjdk.java.net/jdk/pull/3998/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3998=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8258795
  Stats: 340 lines in 2 files changed: 327 ins; 0 del; 13 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3998.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3998/head:pull/3998

PR: https://git.openjdk.java.net/jdk/pull/3998


Re: RFR: 8264678: Incomplete comment in build.tools.generatecharacter.GenerateCharacter

2021-04-28 Thread Naoto Sato
On Wed, 28 Apr 2021 15:44:47 GMT, Claes Redestad  wrote:

> I'm not exactly sure what I intended to say in this partial comment. Removing 
> it.

Marked as reviewed by naoto (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/3766


Integrated: 8265375: Bootcycle builds fail with StackOverflowError in cldrconverter

2021-04-16 Thread Naoto Sato
On Fri, 16 Apr 2021 21:10:42 GMT, Naoto Sato  wrote:

> Please review the fix to the tier4 build failure. The piece of code that made 
> into `CLDRLocaleProviderAdapter.java` was also needed in the build tool 
> counterpart (`CLDRConverter`).

This pull request has now been integrated.

Changeset: ff499701
Author:    Naoto Sato 
URL:   https://git.openjdk.java.net/jdk/commit/ff499701
Stats: 5 lines in 1 file changed: 4 ins; 0 del; 1 mod

8265375: Bootcycle builds fail with StackOverflowError in cldrconverter

Reviewed-by: joehw

-

PR: https://git.openjdk.java.net/jdk/pull/3553


Re: RFR: 8265375: Bootcycle builds fail with StackOverflowError in cldrconverter [v2]

2021-04-16 Thread Naoto Sato
> Please review the fix to the tier4 build failure. The piece of code that made 
> into `CLDRLocaleProviderAdapter.java` was also needed in the build tool 
> counterpart (`CLDRConverter`).

Naoto Sato has updated the pull request incrementally with one additional 
commit since the last revision:

  Copyright year update.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/3553/files
  - new: https://git.openjdk.java.net/jdk/pull/3553/files/91326786..e54cb6ec

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=3553=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=3553=00-01

  Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3553.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3553/head:pull/3553

PR: https://git.openjdk.java.net/jdk/pull/3553


RFR: 8265375: Bootcycle builds fail with StackOverflowError in cldrconverter

2021-04-16 Thread Naoto Sato
Please review the fix to the tier4 build failure. The piece of code that made 
into `CLDRLocaleProviderAdapter.java` was also needed in the build tool 
counterpart (`CLDRConverter`).

-

Commit messages:
 - 8265375: Bootcycle builds fail with StackOverflowError in cldrconverter

Changes: https://git.openjdk.java.net/jdk/pull/3553/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3553=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8265375
  Stats: 4 lines in 1 file changed: 4 ins; 0 del; 0 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3553.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3553/head:pull/3553

PR: https://git.openjdk.java.net/jdk/pull/3553


Integrated: 8258794: Support for CLDR version 39

2021-04-15 Thread Naoto Sato
On Wed, 14 Apr 2021 21:13:51 GMT, Naoto Sato  wrote:

> Please review the changes to support CLDR version 39. The vast majority of 
> the changes are purely data changes from Unicode. The only change affected in 
> logic was in `CLDRLocaleProviderAdapter.java`, where it needed to deal with 
> CLDR's Norwegian language code switch 
> (https://unicode-org.atlassian.net/browse/CLDR-2698)

This pull request has now been integrated.

Changeset: f6e54f2f
Author:Naoto Sato 
URL:   https://git.openjdk.java.net/jdk/commit/f6e54f2f
Stats: 26326 lines in 815 files changed: 761 ins; 23140 del; 2425 mod

8258794: Support for CLDR version 39

Reviewed-by: joehw, erikj

-

PR: https://git.openjdk.java.net/jdk/pull/3502


Re: RFR: 8258794: Support for CLDR version 39

2021-04-14 Thread Naoto Sato
On Wed, 14 Apr 2021 21:13:51 GMT, Naoto Sato  wrote:

> Please review the changes to support CLDR version 39. The vast majority of 
> the changes are purely data changes from Unicode. The only change affected in 
> logic was in `CLDRLocaleProviderAdapter.java`, where it needed to deal with 
> CLDR's Norwegian language code switch 
> (https://unicode-org.atlassian.net/browse/CLDR-2698)

Thanks, Joe.

> Naoto, are you testing GitHub ('s ability to handle a large number of files) 
> ;-)

CLDR itself has been hosted on GitHub too, so it shouldn't be a problem 

-

PR: https://git.openjdk.java.net/jdk/pull/3502


RFR: 8258794: Support for CLDR version 39

2021-04-14 Thread Naoto Sato
Please review the changes to support CLDR version 39. The vast majority of the 
changes are purely data changes from Unicode. The only change affected in logic 
was in `CLDRLocaleProviderAdapter.java`, where it needed to deal with CLDR's 
Norwegian language code switch 
(https://unicode-org.atlassian.net/browse/CLDR-2698)

-

Commit messages:
 - 8258794: Support for CLDR version 39
 - CLDR 38.1

Changes: https://git.openjdk.java.net/jdk/pull/3502/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=3502=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8258794
  Stats: 26326 lines in 815 files changed: 761 ins; 23140 del; 2425 mod
  Patch: https://git.openjdk.java.net/jdk/pull/3502.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/3502/head:pull/3502

PR: https://git.openjdk.java.net/jdk/pull/3502


Re: RFR: 8264779: Fix doclint warnings in java/nio [v3]

2021-04-07 Thread Naoto Sato
On Wed, 7 Apr 2021 18:54:26 GMT, Conor Cleary  wrote:

>> This fix addresses the following warnings which were generated by building 
>> JDK API documentation with the `-Xdoclint:all` option enabled:
>> 
>> ./build/linux-x64/support/gensrc/java.base/java/nio/charset/IllegalCharsetNameException.java:47:
>>  warning: no comment
>> private String charsetName;
>>^
>> ./open/src/java.base/share/classes/java/nio/charset/MalformedInputException.java:44:
>>  warning: no comment
>> private int inputLength;
>> ^
>> ./open/src/java.base/share/classes/java/nio/charset/UnmappableCharacterException.java:44:
>>  warning: no comment
>> private int inputLength;
>> ^
>> ./build/linux-x64/support/gensrc/java.base/java/nio/charset/UnsupportedCharsetException.java:47:
>>  warning: no comment
>> private String charsetName;
>>^
>> ./open/src/java.base/share/classes/java/nio/file/DirectoryIteratorException.java:81:
>>  warning: no @param for s
>> private void readObject(ObjectInputStream s)
>>  ^
>> ./open/src/java.base/share/classes/java/nio/file/DirectoryIteratorException.java:81:
>>  warning: no @throws for java.lang.ClassNotFoundException
>> private void readObject(ObjectInputStream s)
>>  ^
>> ./open/src/java.base/share/classes/java/nio/file/FileSystemException.java:43:
>>  warning: no comment
>> private final String file;
>>  ^
>> ./open/src/java.base/share/classes/java/nio/file/FileSystemException.java:44:
>>  warning: no comment
>> private final String other;
>>  ^
>> ./open/src/java.base/share/classes/java/nio/file/InvalidPathException.java:43:
>>  warning: no comment
>> private int index;
>> ^
>> ./open/src/java.base/share/classes/java/nio/file/InvalidPathException.java:42:
>>  warning: no comment
>> private String input;
>>^
>> ./open/src/java.base/share/classes/java/nio/file/attribute/UserPrincipalNotFoundException.java:43:
>>  warning: no comment
>> private final String name;
>>  ^
>> Changes to 
>> [`genExceptions.sh`](https://github.com/openjdk/jdk/commit/b729d8ed7970737a8a2d4e8aa788df33789faea2)
>>  and the two 
>> [`exceptions`](https://github.com/openjdk/jdk/commit/b729d8ed7970737a8a2d4e8aa788df33789faea2)
>>  templates are to address the warnings concerned with 
>> `UnsupportedCharsetException.java` and `IllegalCharsetNameException.java` 
>> which are generated when `make jdk-image` is run. A CSR will be filed in due 
>> course with respect to these changes.
>
> Conor Cleary has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8264779: Shortened parameter comment

Marked as reviewed by naoto (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/3376


Re: RFR: 8264779: Fix doclint warnings in java/nio [v2]

2021-04-07 Thread Naoto Sato
On Wed, 7 Apr 2021 18:04:48 GMT, Alan Bateman  wrote:

>> It could, maybe something like "The length of the input byte (or character) 
>> sequence." would work?
>
> or just "The length of the input" so that it is consistent with the 
> description of the getInputLength method.

That sounds good.

-

PR: https://git.openjdk.java.net/jdk/pull/3376


Re: RFR: 8264779: Fix doclint warnings in java/nio [v2]

2021-04-07 Thread Naoto Sato
On Wed, 7 Apr 2021 17:58:58 GMT, Conor Cleary  wrote:

>> src/java.base/share/classes/java/nio/exceptions line 31:
>> 
>>> 29: PACKAGE=java.nio
>>> 30: # This year should only change if the generated source is modified.
>>> 31: COPYRIGHT_YEARS="2000, 2021,"
>> 
>> Does not seem necessary, as I don't see any changes in java.nio package.
>
> This makes changes to the copyright headers of of the generated source files 
> `UnsupportedCharsetException.java` and `IllegalCharsetNameException.java`. 
> These can be found under a jdk build in `support/gensrc` and the headers 
> update after running `make jdk-image`. Perhaps the build is out of date? 
> These changes are harder to track as the source is not checked in.

That's right. Sorry for the false alarm.

-

PR: https://git.openjdk.java.net/jdk/pull/3376


Re: RFR: 8264779: Fix doclint warnings in java/nio [v2]

2021-04-07 Thread Naoto Sato
On Wed, 7 Apr 2021 14:40:48 GMT, Conor Cleary  wrote:

>> This fix addresses the following warnings which were generated by building 
>> JDK API documentation with the `-Xdoclint:all` option enabled:
>> 
>> ./build/linux-x64/support/gensrc/java.base/java/nio/charset/IllegalCharsetNameException.java:47:
>>  warning: no comment
>> private String charsetName;
>>^
>> ./open/src/java.base/share/classes/java/nio/charset/MalformedInputException.java:44:
>>  warning: no comment
>> private int inputLength;
>> ^
>> ./open/src/java.base/share/classes/java/nio/charset/UnmappableCharacterException.java:44:
>>  warning: no comment
>> private int inputLength;
>> ^
>> ./build/linux-x64/support/gensrc/java.base/java/nio/charset/UnsupportedCharsetException.java:47:
>>  warning: no comment
>> private String charsetName;
>>^
>> ./open/src/java.base/share/classes/java/nio/file/DirectoryIteratorException.java:81:
>>  warning: no @param for s
>> private void readObject(ObjectInputStream s)
>>  ^
>> ./open/src/java.base/share/classes/java/nio/file/DirectoryIteratorException.java:81:
>>  warning: no @throws for java.lang.ClassNotFoundException
>> private void readObject(ObjectInputStream s)
>>  ^
>> ./open/src/java.base/share/classes/java/nio/file/FileSystemException.java:43:
>>  warning: no comment
>> private final String file;
>>  ^
>> ./open/src/java.base/share/classes/java/nio/file/FileSystemException.java:44:
>>  warning: no comment
>> private final String other;
>>  ^
>> ./open/src/java.base/share/classes/java/nio/file/InvalidPathException.java:43:
>>  warning: no comment
>> private int index;
>> ^
>> ./open/src/java.base/share/classes/java/nio/file/InvalidPathException.java:42:
>>  warning: no comment
>> private String input;
>>^
>> ./open/src/java.base/share/classes/java/nio/file/attribute/UserPrincipalNotFoundException.java:43:
>>  warning: no comment
>> private final String name;
>>  ^
>> Changes to 
>> [`genExceptions.sh`](https://github.com/openjdk/jdk/commit/b729d8ed7970737a8a2d4e8aa788df33789faea2)
>>  and the two 
>> [`exceptions`](https://github.com/openjdk/jdk/commit/b729d8ed7970737a8a2d4e8aa788df33789faea2)
>>  templates are to address the warnings concerned with 
>> `UnsupportedCharsetException.java` and `IllegalCharsetNameException.java` 
>> which are generated when `make jdk-image` is run. A CSR will be filed in due 
>> course with respect to these changes.
>
> Conor Cleary has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   8264779: Simplified comments

src/java.base/share/classes/java/nio/charset/MalformedInputException.java line 
45:

> 43: 
> 44: /**
> 45:  * The length of the input byte sequence.

Should this comment also refer to the character sequence?

src/java.base/share/classes/java/nio/charset/exceptions line 31:

> 29: PACKAGE=java.nio.charset
> 30: # This year should only change if the generated source is modified.
> 31: COPYRIGHT_YEARS="2000, 2021,"

Do those modifications to the exception sources end up in a change in 
`generated` sources?

src/java.base/share/classes/java/nio/exceptions line 31:

> 29: PACKAGE=java.nio
> 30: # This year should only change if the generated source is modified.
> 31: COPYRIGHT_YEARS="2000, 2021,"

Does not seem necessary, as I don't see any changes in java.nio package.

-

PR: https://git.openjdk.java.net/jdk/pull/3376


Re: RFR: 8263677: Improve Character.isLowerCase/isUpperCase lookups [v2]

2021-03-16 Thread Naoto Sato
On Tue, 16 Mar 2021 21:02:28 GMT, Claes Redestad  wrote:

>> This patch changes the otherLowercase / otherUppercase bits to be set if 
>> either the codepoint is of type LOWERCASE_LETTER and UPPERCASE_LETTER, or 
>> the Unicode Other_Lowercase / Other_Uppercase property is set. This 
>> simplifies the lookup in Character.isLowerCase/isUpperCase to a single table 
>> lookup, which appears to be healthy for performance.
>> 
>> I also took the opportunity to clean up the somewhat dated GenerateCharacter 
>> utility class.
>> 
>> Testing: tier1-3
>
> Claes Redestad has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Roger review + additional cleanups

Marked as reviewed by naoto (Reviewer).

-

PR: https://git.openjdk.java.net/jdk/pull/3028


Re: RFR: 8257733: Move module-specific data from make to respective module [v2]

2020-12-16 Thread Naoto Sato
On Thu, 10 Dec 2020 23:07:52 GMT, Naoto Sato  wrote:

>> Magnus Ihse Bursie has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Move to share/data, and move jdwp.spec to java.se
>
> Reviewed changes to `characterdata`, `charsetmapping`, `cldr`, `currency`, 
> `lsrdata`, `tzdata`, and `unicodedata` with minor comment. Looks good overall.

>I also think that the person most qualified to judge about charsetmapping is 
>@naotoj, which has already accepted this review as it is.

Although I am the current RE for the charsets component, I succeeded it from 
Alan/Sherman, so I would like to hear Alan's opinion on this.

-

PR: https://git.openjdk.java.net/jdk/pull/1611


Re: RFR: 8253497: Core Libs Terminology Refresh

2020-12-14 Thread Naoto Sato
On Mon, 14 Dec 2020 19:36:48 GMT, Brent Christian  wrote:

> This is part of an effort in the JDK to replace archaic/non-inclusive words 
> with more neutral terms (see JDK-8253315 for details).
> 
> Here are the changes covering core libraries code and tests.  Terms were 
> changed as follows:
> 1. grandfathered -> legacy
> 2. blacklist -> filter or reject
> 3. whitelist -> allow or accept
> 4. master -> coordinator
> 5. slave -> worker
> 
> Addressing similar issues in upstream 3rd party code is out of scope of this 
> PR.  Such changes will be picked up from their upstream sources.

Looks good to me.

test/jdk/java/lang/ClassLoader/Assert.java line 65:

> 63: 
> 64: int switchSource = 0;
> 65: if (args.length == 0) { // This is  the coordinator version

Extra space between "is" and "the."

-

Marked as reviewed by naoto (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1771


Re: RFR: 8257733: Move module-specific data from make to respective module [v2]

2020-12-10 Thread Naoto Sato
On Mon, 7 Dec 2020 14:27:45 GMT, Magnus Ihse Bursie  wrote:

>> A lot (but not all) of the data in make/data is tied to a specific module. 
>> For instance, the publicsuffixlist is used by java.base, and fontconfig by 
>> java.desktop. (A few directories, like mainmanifest, is *actually* used by 
>> make for the whole build.) 
>> 
>> These data files should move to the module they belong to. The are, after 
>> all, "source code" for that module that is "compiler" into resulting 
>> deliverables, for that module. (But the "source code" language is not Java 
>> or C, but typically a highly domain specific language or data format, and 
>> the "compilation" is, often, a specialized transformation.) 
>> 
>> This misplacement of the data directory is most visible at code review time. 
>> When such data is changed, most of the time build-dev (or the new build 
>> label) is involved, even though this has nothing to do with the build. While 
>> this is annoying, a worse problem is if the actual team that needs to review 
>> the patch (i.e., the team owning the module) is missed in the review.
>> 
>> ### Modules reviewed
>> 
>> - [ ] java.base
>> - [ ] java.desktop
>> - [x] jdk.compiler
>> - [ ] java.se
>
> Magnus Ihse Bursie has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Move to share/data, and move jdwp.spec to java.se

Reviewed changes to `characterdata`, `charsetmapping`, `cldr`, `currency`, 
`lsrdata`, `tzdata`, and `unicodedata` with minor comment. Looks good overall.

test/jdk/java/util/Locale/LSRDataTest.java line 57:

> 55: // path to the lsr file from the make folder, this test relies on the
> 56: // relative path to the file in the make folder, considering
> 57: // test and make will always exist in the same jdk layout

The comment refers to the "make" folder. (line 55 as well).

-

Marked as reviewed by naoto (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1611


Re: RFR: 8257733: Move module-specific data from make to respective module [v2]

2020-12-08 Thread Naoto Sato
On Tue, 8 Dec 2020 17:33:16 GMT, Alan Bateman  wrote:

>> The mapping and nr tables, and the *-X.java.template files in 
>> make/data/charsetmapping are used to generate source code for the java.base 
>> and jdk.charsets modules. The stdcs-$OS files configure the package and 
>> module that each charset go into. If the tables used to generate the source 
>> files are moved to src/java.base then make/modules/jdk.charsets/Gensrc.gmk 
>> will probably need a new home too.
>
>> @AlanBateman The process of modularization was not fully completed with 
>> Project Jigsaw, and a few ugly warts remained. I was under the impression 
>> that these should be addressed in follow-up fixes, but this was 
>> unfortunately never done. Charsets and cldrconverter were both split between 
>> a core portion in java.base and the rest in jdk.charsets and jdk.localedata, 
>> respectively, but the split was never handled properly, but just "duct 
>> taped" in place.
> 
> This is a complicated area of the build, not really a Project Jigsaw issue. 
> It's complicated because the source code for the charsets is generated at 
> build time and the set of non-standard charsets included in java.base varies 
> by platform, e.g. there's are several IBMxxx charsets in java.base when 
> building on AIX that are not interesting to include in java.base on other 
> platforms. This means we can't split up the mapping tables in 
> make/data/charsetmapping and put them in different directories. If you are 
> moving them into the src tree then src/java.base (as you have it) is best but 
> will still have the ugly wart that some of these mapping tables will be used 
> to generate code for the jdk.charsets module.

> @AlanBateman The process of modularization was not fully completed with 
> Project Jigsaw, and a few ugly warts remained. I was under the impression 
> that these should be addressed in follow-up fixes, but this was unfortunately 
> never done. Charsets and cldrconverter were both split between a core portion 
> in java.base and the rest in jdk.charsets and jdk.localedata, respectively, 
> but the split was never handled properly, but just "duct taped" in place.
> 
> I chose to put the data files used for both java.base and the "additional" 
> modules in java.base, based on the comment that Naoto made in 
> https://mail.openjdk.java.net/pipermail/build-dev/2020-March/027044.html:
> 
> > As to charsetmapping and cldrconverter, I believe they can reside in
> > java.base, as jdk.charsets and jdk.localedata modules depend on it.
> 
> Of course it would be preferable to make a proper split, but that requires 
> work done by the component teams to break the modules apart.
> 
> Specifically for make/modules/jdk.charsets/Gensrc.gmk; the code in that file 
> is more or less duplicated in 
> make/modules/java.base/gensrc/GensrcCharsetMapping.gmk, since the same data 
> set is processed twice, once for java.base and once for jdk.charsets. I don't 
> think that means that make/modules/jdk.charsets/Gensrc.gmk should move to any 
> other place.

I still stand by what I wrote above. It's best to put data in java.base for 
charsets/localedata. Otherwise we would have to duplicate some in each modules 
source directory.

-

PR: https://git.openjdk.java.net/jdk/pull/1611


Integrated: 8247432: Update IANA Language Subtag Registry to Version 2020-09-29

2020-11-23 Thread Naoto Sato
On Fri, 20 Nov 2020 17:55:55 GMT, Naoto Sato  wrote:

> Hi,
> 
> Please review the changes to the subject issue. This is to incorporate the 
> latest language subtag registry definition into the JDK.

This pull request has now been integrated.

Changeset: ae0ca743
Author:    Naoto Sato 
URL:   https://git.openjdk.java.net/jdk/commit/ae0ca743
Stats: 52 lines in 2 files changed: 44 ins; 5 del; 3 mod

8247432: Update IANA Language Subtag Registry to Version 2020-09-29

Reviewed-by: joehw

-

PR: https://git.openjdk.java.net/jdk/pull/1357


RFR: 8247432: Update IANA Language Subtag Registry to Version 2020-09-29

2020-11-20 Thread Naoto Sato
Hi,

Please review the changes to the subject issue. This is to incorporate the 
latest language subtag registry definition into the JDK.

-

Commit messages:
 - LSR 2020-09-29
 - LSR 2020-07-17

Changes: https://git.openjdk.java.net/jdk/pull/1357/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1357=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8247432
  Stats: 52 lines in 2 files changed: 44 ins; 5 del; 3 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1357.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1357/head:pull/1357

PR: https://git.openjdk.java.net/jdk/pull/1357


Re: RFR: 8251317: Support for CLDR version 38

2020-11-17 Thread Naoto Sato
On Tue, 17 Nov 2020 23:19:23 GMT, Naoto Sato  wrote:

> Hi,
> 
> Please review the changes for upgrading the CLDR data to version 38. The vast 
> majority of the changes are simply the changes in CLDR upstream, and others 
> are mainly test changes due to the locale data change.

Looks like the generated webrev is empty, possibly due to this issue 
(https://bugs.openjdk.java.net/browse/SKARA-732). Here is the one manually 
generated:
https://cr.openjdk.java.net/~naoto/cldr/webrev.00/

-

PR: https://git.openjdk.java.net/jdk/pull/1279


RFR: 8251317: Support for CLDR version 38

2020-11-17 Thread Naoto Sato
Hi,

Please review the changes for upgrading the CLDR data to version 38. The vast 
majority of the changes are simply the changes in CLDR upstream, and others are 
mainly test changes due to the locale data change.

-

Commit messages:
 - Updated the version in `cldr.md` files
 - Merge branch 'master' into cldr
 - 8251317: Support for CLDR version 38

Changes: https://git.openjdk.java.net/jdk/pull/1279/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=1279=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8251317
  Stats: 57338 lines in 209 files changed: 41958 ins; 4932 del; 10448 mod
  Patch: https://git.openjdk.java.net/jdk/pull/1279.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/1279/head:pull/1279

PR: https://git.openjdk.java.net/jdk/pull/1279


Re: RFR: 8255226: (tz) Upgrade time-zone data to tzdata2020d

2020-11-02 Thread Naoto Sato
On Tue, 3 Nov 2020 00:00:26 GMT, Kiran Sidhartha Ravikumar 
 wrote:

>> My question is why it is failing. Have you figured it? The existing 
>> exceptions are either negative DST or last rule beyond 2037, which javazic 
>> cannot handle. The changes introduced with 2020d does not meet either of 
>> them. Unless we know why it is failing, we cannot be sure we can exclude it.
>
> Thanks for getting back Naoto, I believe this is a long-standing issue - 
> https://bugs.openjdk.java.net/browse/JDK-8227293.
> 
> Looking back at the integration of tzdata2019a/tzdata2019b, the same issue 
> was addressed by updating the source code - 
> https://hg.openjdk.java.net/jdk/jdk/rev/79036e5e744b#l13.1. 
> 
> Here is some history to the issue - 
> https://mail.openjdk.java.net/pipermail/i18n-dev/2019-July/002887.html
> 
> Please let me know your thoughts

Should we then remove the hack in the ZoneInfoFile.java that excludes 
Gaza/Hebron instead?

-

PR: https://git.openjdk.java.net/jdk/pull/1012


Re: RFR: 8255226: (tz) Upgrade time-zone data to tzdata2020d

2020-11-02 Thread Naoto Sato
On Mon, 2 Nov 2020 18:06:28 GMT, Kiran Sidhartha Ravikumar 
 wrote:

>> test/jdk/sun/util/calendar/zi/TestZoneInfo310.java line 201:
>> 
>>> 199: zid.equals("Iran") || // last rule mismatch
>>> 200: zid.equals("Asia/Gaza") || // last rule mismatch
>>> 201: zid.equals("Asia/Hebron")) { // last rule mismatch
>> 
>> Can you explain why these zones are failing? The criteria for excluding 
>> failing tests here is that the zone has negative dst and last rules beyond 
>> 2037, and I don't think those newly excluded zones suffice those.
>
> It's probably these last rule what is causing the issue
> 
> Rule Palestine2020max -   Mar Sat>=24 0:001:00
> S
> Rule Palestine2020max -   Oct Sat>=24 1:000   
> -
> 
> The failure seen is 
> 
> **
> Asia/Gaza : Asia/Gaza
> -
> SimpleTimeZone (NG)
>
> stz=java.util.SimpleTimeZone[id=Asia/Gaza,offset=720,dstSavings=360,useDaylight=true,startYear=0,startMode=2,startMonth=2,startDay=-1,startDayOfWeek=7,startTime=0,startTimeMode=0,endMode=2,endMonth=9,endDay=-1,endDayOfWeek=7,endTime=360,endTimeMode=0]
>   
> stz0=java.util.SimpleTimeZone[id=Asia/Gaza,offset=720,dstSavings=360,useDaylight=true,startYear=0,startMode=3,startMonth=2,startDay=24,startDayOfWeek=7,startTime=0,startTimeMode=0,endMode=3,endMonth=9,endDay=24,endDayOfWeek=7,endTime=360,endTimeMode=0]

My question is why it is failing. Have you figured it? The existing exceptions 
are either negative DST or last rule beyond 2037, which javazic cannot handle. 
The changes introduced with 2020d does not meet either of them. Unless we know 
why it is failing, we cannot be sure we can exclude it.

-

PR: https://git.openjdk.java.net/jdk/pull/1012


Re: RFR: 8255226: (tz) Upgrade time-zone data to tzdata2020d

2020-11-02 Thread Naoto Sato
On Mon, 2 Nov 2020 16:29:07 GMT, Kiran Sidhartha Ravikumar 
 wrote:

> Hi Guys,
> 
> Please review the integration of tzdata2020d to JDK.
> 
> Details regarding the change can be viewed at - 
> https://mm.icann.org/pipermail/tz-announce/2020-October/62.html
> Bug: https://bugs.openjdk.java.net/browse/JDK-8255226
> 
> TestZoneInfo310.java test failure is addressed along with it. The last rule 
> affects "Asia/Gaza" and "Asia/Hebron" and therefore excluded from the test.
> 
> Regression Tests pass along with JCK.
> 
> Please let me know if the changes are good to push.
> 
> Thanks,
> Kiran

The test case needs copyright year change to 2020.

test/jdk/sun/util/calendar/zi/TestZoneInfo310.java line 201:

> 199: zid.equals("Iran") || // last rule mismatch
> 200: zid.equals("Asia/Gaza") || // last rule mismatch
> 201: zid.equals("Asia/Hebron")) { // last rule mismatch

Can you explain why these zones are failing? The criteria for excluding failing 
tests here is that the zone has negative dst and last rules beyond 2037, and I 
don't think those newly excluded zones suffice those.

-

Changes requested by naoto (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1012


Re: RFR: 8247781: Day periods support [v3]

2020-10-30 Thread Naoto Sato
> Hi,
> 
> Please review the changes for the subject issue. This is to enhance the 
> java.time package to support day periods, such as "in the morning", defined 
> in CLDR. It will add a new pattern character 'B' and its supporting builder 
> method. The motivation and its spec are in this CSR:
> 
> https://bugs.openjdk.java.net/browse/JDK-8254629
> 
> Naoto

Naoto Sato has updated the pull request incrementally with one additional 
commit since the last revision:

  Fixed exception messages.

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/938/files
  - new: https://git.openjdk.java.net/jdk/pull/938/files/6e1eade7..29c47afc

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=938=02
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=938=01-02

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/jdk/pull/938.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/938/head:pull/938

PR: https://git.openjdk.java.net/jdk/pull/938


Re: RFR: 8247781: Day periods support [v2]

2020-10-30 Thread Naoto Sato
On Fri, 30 Oct 2020 10:33:28 GMT, Stephen Colebourne  
wrote:

>> Naoto Sato has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Addressed the following comments:
>>   - https://github.com/openjdk/jdk/pull/938#discussion_r515003422
>>   - https://github.com/openjdk/jdk/pull/938#discussion_r515005296
>>   - https://github.com/openjdk/jdk/pull/938#discussion_r515008862
>>   - https://github.com/openjdk/jdk/pull/938#discussion_r515030268
>>   - https://github.com/openjdk/jdk/pull/938#discussion_r515030880
>>   - https://github.com/openjdk/jdk/pull/938#discussion_r515032002
>>   - https://github.com/openjdk/jdk/pull/938#discussion_r515036803
>>   - https://github.com/openjdk/jdk/pull/938#discussion_r515037626
>>   - https://github.com/openjdk/jdk/pull/938#discussion_r515038069
>>   - https://github.com/openjdk/jdk/pull/938#discussion_r515039056
>
> test/jdk/java/time/test/java/time/format/TestDateTimeFormatterBuilder.java 
> line 981:
> 
>> 979: 
>> 980: {"B", "Text(DayPeriod,SHORT)"},
>> 981: {"BB", "Text(DayPeriod,SHORT)"},
> 
> "BB" and "BBB" are not defined to do anything in the CSR. Java should match 
> CLDR/LDML rules here.

Fixed.

> test/jdk/java/time/test/java/time/format/TestDateTimeFormatterBuilder.java 
> line 540:
> 
>> 538: builder.appendDayPeriodText(TextStyle.FULL);
>> 539: DateTimeFormatter f = builder.toFormatter();
>> 540: assertEquals(f.toString(), "Text(DayPeriod,FULL)");
> 
> This should be "DayPeriod(FULL)", because an end user might create a 
> `TemporalField` with the name "DayPeriod" and the toString should be unique.

Fixed.

> src/java.base/share/classes/java/time/format/Parsed.java line 352:
> 
>> 350: (fieldValues.containsKey(MINUTE_OF_HOUR) ? 
>> fieldValues.get(MINUTE_OF_HOUR) : 0);
>> 351: if (!dayPeriod.includes(mod)) {
>> 352: throw new DateTimeException("Conflict found: " + 
>> changeField + " conflict with day period");
> 
> "conflict with day period" -> "conflicts with day period"
> 
> Should also include `changeValue` and ideally the valid range

Fixed.

> src/java.base/share/classes/java/time/format/Parsed.java line 472:
> 
>> 470: }
>> 471: if (dayPeriod != null) {
>> 472: if (fieldValues.containsKey(HOUR_OF_DAY)) {
> 
> Are we certain that the CLDR data does not contain day periods based on 
> minutes as well as hours? This logic does not check against MINUTE_OF_HOUR 
> for example. The logic also conflicts with the spec Javadoc that says 
> MINUTE_OF_HOUR is validated.

MINUTE_OF_HOUR without HOUR_OF_DAY/HOUR_OF_AMPM may not make any sense, so it 
is only validated in updateCheckDayPeriodConflict.

> src/java.base/share/classes/java/time/format/Parsed.java line 500:
> 
>> 498: }
>> 499: }
>> 500: }
> 
> Looking at the existing logic, the `AMPM_OF_DAY` field is completely ignored 
> if there is no `HOUR_OF_AMPM` field. Thus, there is no validation to check 
> `AMPM_OF_DAY` against `HOUR_OF_DAY`. This seems odd. (AMPM_OF_DAY = 0 and 
> HOUR_OF_DAY=18 does not look like it throws an exception, when it probably 
> should).
> 
> On solution would be for `AMPM_OF_DAY` to be resolved to a day period at line 
> 427, checking for conflicts with any parsed day period. (a small bug fix 
> behavioural change)

There are cases where a period crosses midnight, e.g., 23:00-04:00 so it cannot 
validate am/pm, so I decided to just override ampm with dayperiod without 
validating.

> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java 
> line 1489:
> 
>> 1487: Objects.requireNonNull(style, "style");
>> 1488: if (style != TextStyle.FULL && style != TextStyle.SHORT && 
>> style != TextStyle.NARROW) {
>> 1489: throw new IllegalArgumentException("Style must be either 
>> full, short, or narrow");
> 
> Nothing in the Javadoc describes this behaviour. It would make more sense to 
> map FULL_STANDALONE to FULL and so on and not throw an exception.

Fixed.

> src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java 
> line 1869:
> 
>> 1867: } else if (cur == 'B') {
>> 1868: switch (count) {
>> 1869: case 1, 2, 3 -> 
>> appendDayPeriodText(TextStyle.SHORT);
> 
> I think this sho

Re: RFR: 8247781: Day periods support [v2]

2020-10-30 Thread Naoto Sato
> Hi,
> 
> Please review the changes for the subject issue. This is to enhance the 
> java.time package to support day periods, such as "in the morning", defined 
> in CLDR. It will add a new pattern character 'B' and its supporting builder 
> method. The motivation and its spec are in this CSR:
> 
> https://bugs.openjdk.java.net/browse/JDK-8254629
> 
> Naoto

Naoto Sato has updated the pull request incrementally with one additional 
commit since the last revision:

  Addressed the following comments:
  - https://github.com/openjdk/jdk/pull/938#discussion_r515003422
  - https://github.com/openjdk/jdk/pull/938#discussion_r515005296
  - https://github.com/openjdk/jdk/pull/938#discussion_r515008862
  - https://github.com/openjdk/jdk/pull/938#discussion_r515030268
  - https://github.com/openjdk/jdk/pull/938#discussion_r515030880
  - https://github.com/openjdk/jdk/pull/938#discussion_r515032002
  - https://github.com/openjdk/jdk/pull/938#discussion_r515036803
  - https://github.com/openjdk/jdk/pull/938#discussion_r515037626
  - https://github.com/openjdk/jdk/pull/938#discussion_r515038069
  - https://github.com/openjdk/jdk/pull/938#discussion_r515039056

-

Changes:
  - all: https://git.openjdk.java.net/jdk/pull/938/files
  - new: https://git.openjdk.java.net/jdk/pull/938/files/7ac5fa03..6e1eade7

Webrevs:
 - full: https://webrevs.openjdk.java.net/?repo=jdk=938=01
 - incr: https://webrevs.openjdk.java.net/?repo=jdk=938=00-01

  Stats: 99 lines in 3 files changed: 48 ins; 2 del; 49 mod
  Patch: https://git.openjdk.java.net/jdk/pull/938.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/938/head:pull/938

PR: https://git.openjdk.java.net/jdk/pull/938


RFR: 8247781: Day periods support

2020-10-29 Thread Naoto Sato
Hi,

Please review the changes for the subject issue. This is to enhance the 
java.time package to support day periods, such as "in the morning", defined in 
CLDR. It will add a new pattern character 'B' and its supporting builder 
method. The motivation and its spec are in this CSR:

https://bugs.openjdk.java.net/browse/JDK-8254629

Naoto

-

Commit messages:
 - 8247781: Day periods support

Changes: https://git.openjdk.java.net/jdk/pull/938/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk=938=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8247781
  Stats: 1022 lines in 17 files changed: 838 ins; 87 del; 97 mod
  Patch: https://git.openjdk.java.net/jdk/pull/938.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/938/head:pull/938

PR: https://git.openjdk.java.net/jdk/pull/938


Re: RFR: 8254982: (tz) Upgrade time-zone data to tzdata2020c [v2]

2020-10-20 Thread Naoto Sato
On Tue, 20 Oct 2020 11:39:36 GMT, Kiran Sidhartha Ravikumar 
 wrote:

>> Hi Guys,
>> 
>> Please review the integration of tzdata2020c to JDK.
>> 
>> Details regarding the change can be viewed at - 
>> https://mm.icann.org/pipermail/tz-announce/2020-October/60.html
>> Bug: https://bugs.openjdk.java.net/browse/JDK-8254982
>> 
>> Along with it, there is a test fix for 
>> https://bugs.openjdk.java.net/browse/JDK-8254865, With tzdata2020b, the test
>> fails for the mentioned zones expecting "PST" but JDK has Zone names for 
>> "MST" for JRE locale provider. Since the
>> purpose of the test is to get any GMT-08:00 time zone, I have excluded those 
>> zones from the test.  Please let me know
>> if the changes are good to push.
>> Thanks,
>> Kiran
>
> Kiran Sidhartha Ravikumar has updated the pull request with a new target base 
> due to a merge or a rebase. The
> incremental webrev excludes the unrelated changes brought in by the 
> merge/rebase. The pull request contains three
> additional commits since the last revision:
>  - 8254982: Adding comments for removal of zones in BasicDateTime.java
>  - Merge remote-tracking branch 'origin/master' into JDK-8254982
>  - 8254982: (tz) Upgrade time-zone data to tzdata2020c

Looks good. Thank you for the changes.

-

Marked as reviewed by naoto (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/747


Re: RFR: 8254982: (tz) Upgrade time-zone data to tzdata2020c

2020-10-19 Thread Naoto Sato
On Mon, 19 Oct 2020 18:44:28 GMT, Kiran Sidhartha Ravikumar 
 wrote:

> Hi Guys,
> 
> Please review the integration of tzdata2020c to JDK.
> 
> Details regarding the change can be viewed at - 
> https://mm.icann.org/pipermail/tz-announce/2020-October/60.html
> Bug: https://bugs.openjdk.java.net/browse/JDK-8254982
> 
> Along with it, there is a test fix for 
> https://bugs.openjdk.java.net/browse/JDK-8254865, With tzdata2020b, the test
> fails for the mentioned zones expecting "PST" but JDK has Zone names for 
> "MST" for JRE locale provider. Since the
> purpose of the test is to get any GMT-08:00 time zone, I have excluded those 
> zones from the test.  Please let me know
> if the changes are good to push.
> Thanks,
> Kiran

test/jdk/java/util/Formatter/BasicDateTime.java line 1695:

> 1693: list.remove("America/Dawson");
> 1694: list.remove("America/WhiteHorse");
> 1695: list.remove("Canada/Yukon");

I'd explicitly mention why these time zones are removed from the test.

-

PR: https://git.openjdk.java.net/jdk/pull/747


Re: RFR: 8254177: (tz) Upgrade time-zone data to tzdata2020b

2020-10-12 Thread Naoto Sato
On Mon, 12 Oct 2020 09:32:38 GMT, Kiran Sidhartha Ravikumar 
 wrote:

> Hi Guys,
> 
> Please review the patch for tzdata2020b integration into JDK.
> 
> Release details can be found here:
> 
> https://mm.icann.org/pipermail/tz-announce/2020-October/59.html
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8254177
> 
> I had the pacificnew, systemv files removed from JDK repo and so TimeZoneName 
> classes, make file and few test files
> need to be updated to incorporate the change.
> The patch has passed all the related testing including JCK.
> 
> Thanks,
> Kiran

Looks good to me.
I was wondering about the consequence of removing the "US/Pacific-New" zone 
names, but since the time zone should never
have been introduced in the first place (it didn't become the law), I think no 
real world application would have used
it.

-

Marked as reviewed by naoto (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/602


Re: RFR [16/java.xml] 8251561: Fix doclint warnings in the java.xml package

2020-08-25 Thread naoto . sato

+1

Naoto

On 8/25/20 11:47 AM, Joe Wang wrote:

Cc-ing build-dev@openjdk.java.net (makefile change: make/Docs.gmk)
Updated webrev: http://cr.openjdk.java.net/~joehw/jdk16/8251561/webrev_04/

Thanks Roger! Please see inline comments.

On 8/25/20 8:09 AM, Roger Riggs wrote:

Hi Joe,

Eliminating the checking for warnings in org.w3c is fine. Please be 
more specific in the comment.


"Ignore the doclint warnings in the W3C DOM package"


Updated.
org/xml/...: If we're suppressing the warnings for org/xml/... then 
the files changes are unnecessary?


Did you mean org/w3c? We're only suppressing the warnings for org/w3c 
where the DOM package is in, but not org/xml where SAX is in.




Remove the addition of -Xmaxwarns, it should stay the default.


Done.


Since its a makefile change, please copy build-dev@openjdk.java.net.


Cc-ed


The first line comments should terminate with a period ".".

javax/xml/stream/FactoryConfigurationError.java:40
javax/xml/stream/XMLStreamException.java:41
javax/xml/xpath/XPathException.java:44  And capitable "Serializable".


javax/xml/stream/events/Attribute.java: 50; add "normalized" to the 
@return line so it is the same as the first line.

For simple get methods, the @return mimics the first line.

javax/xml/stream/events/NotationDeclaration.java:43  add "notation"


Updated webrev including all of the above:

http://cr.openjdk.java.net/~joehw/jdk16/8251561/webrev_04/





Thanks, Roger

p.s.  There is lots of other cleanup of the javadoc, using @code 
around true, false,
missing periods at the end of first sentences, etc.  But that's a 
different task.


Created a bug to keep track of this:
https://bugs.openjdk.java.net/browse/JDK-8252328

Thanks,
Joe




On 8/24/20 5:44 PM, Joe Wang wrote:
Hi all,  adding Roger's comment for the make file to webrev_02 (the 
only change to webrev_01 is Docs.gmk):


http://cr.openjdk.java.net/~joehw/jdk16/8251561/webrev_02/

Thanks,
Joe

On 8/21/20 12:49 PM, naoto.s...@oracle.com wrote:

+1

Naoto

On 8/21/20 12:24 PM, Lance Andersen wrote:

Hi Joe,

This looks OK.




On Aug 21, 2020, at 2:23 PM, Joe Wang  wrote:

Pelase review a patch to add the missing @return, @throws, @param 
statements in the java.xml package (excluding the DOM component).


JBS: https://bugs.openjdk.java.net/browse/JDK-8251561
CSR: https://bugs.openjdk.java.net/browse/JDK-8251995

webrev: http://cr.openjdk.java.net/~joehw/jdk16/8251561/webrev_01/

Thanks,
Joe



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: RFR: JDK-8243985 Make source generation by generatecharacter reproducible

2020-04-28 Thread naoto . sato

Looks good.

Naoto

On 4/28/20 2:19 AM, Magnus Ihse Bursie wrote:
For no really good reason, the generatecharacter buildtool outputs the 
current time in the generated source. While this has no effect on the 
generated class files, it makes gensrc comparison fail between builds.


Bug: https://bugs.openjdk.java.net/browse/JDK-8243985
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8243985-make-generatecharacter-reproducible/webrev.01 



/Magnus


Re: RFR: JDK-8241463 Move build tools to respective modules

2020-03-23 Thread naoto . sato

Hi Magnus,

I looked at i18n related changes:

make/CopyInterimTZDB.gmk
make/ToolsJdk.gmk
make/gendata/Gendata-java.base.gmk
make/gendata/GendataBreakIterator.gmk
make/gendata/GendataTZDB.gmk
make/gensrc/GensrcCharacterData.gmk
make/gensrc/GensrcEmojiData.gmk

They look ok to me.

The *.java changes should have copyright year update.

As to charsetmapping and cldrconverter, I believe they can reside in 
java.base, as jdk.charsets and jdk.localedata modules depend on it.


Naoto

On 3/23/20 12:03 PM, Magnus Ihse Bursie wrote:
The build tools (small java tools that are run during the build to 
generate source code, or data, needed in the JDK) have historically been 
placed in the "make" directory. This maybe made sense long time ago, but 
does not do so anymore.


Instead, the build tools source code should move the the module that 
needs them. For instance, compilefontconfig should move to java.desktop, 
etc.


There are multiple reasons for this:

* Currently we build *all* build tools at once, which mean that we 
cannot compile java.base until e.g. the compilefontconfig tool is 
compiled, even though it is not needed.


* If a build tool, e.g. compilefontconfig is modified, all build tools 
are recompiled, which triggers a rebuild of more or less the entire JDK. 
This makes development of the build tools unnecessary tedious.


* When the build tools are modified, the group owning the corresponding 
module is the proper review instance, not the build team. But since they 
reside under "make", the review mails often include build-dev, but this 
is mostly noise for us. With this move, the ownership is made clear.


In this patch, I have not modified how and when the build tools are 
compiled, but this shuffle is the prerequisite for continuing with that 
in a follow-up patch.


I have also moved the build tools to the org.openjdk.buildtools.* 
package name space (inspired by Skara), instead of the strangely named 
build.tools.* name space.


A few build tools are not moved in this patch. Two of them, 
charsetmapping and cldrconverter, are shared between two modules. (I 
think they should move to modules nevertheless, but they need some more 
thought to make sure I do this right.) The rest are tools that are 
needed for the build in general, like linking or javadoc support. I'll 
move this to a better location too, but in a separate patch.


Bug: https://bugs.openjdk.java.net/browse/JDK-8241463
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8241463-move-build-tools-to-modules/webrev.01 



/Magnus



Re: RFR: JDK-8065704 Set LC_ALL=C for all relevant commands in the build system

2019-10-02 Thread naoto . sato

Hi Magnus,

Looks good to me. Although I cannot provide any reproducible problematic 
instance, there have been instances where native tools, such as `date` 
command had produced localized date, and the build failed to parse it in 
US format. Anyways, it is a good preventive measure to me.


Naoto

On 10/2/19 2:06 AM, Magnus Ihse Bursie wrote:

 From the bug report:
We should prefix LC_ALL=C for most, maybe all, tools we use when building.

This probably means we should run "export LC_ALL=C" early in the 
configure script as well.

---

The fix itself is trivial. While I know we've had several issues 
regarding localization, I could not find any specific instances now that 
I was looking for them. I searched JBS for a while but could not dig up 
anything that was reproducible. So, unfortunately, I have been unable to 
verify that this solves any actual problems. That being said, I believe 
this is a prudent fix that should have been in place long time ago. But 
if anyone can give me a concrete example that breaks so that I can 
verify that this helps, please let me know.


Bug: https://bugs.openjdk.java.net/browse/JDK-8065704
WebRev: http://cr.openjdk.java.net/~ihse/JDK-8065704-LC_ALL/webrev.01

/Magnus


Re: [14] RFR: 8212970: TZ database in "vanguard" format support

2019-07-24 Thread naoto . sato

Hi Joe,

Thank you for the review.

On 7/24/19 2:57 PM, Joe Wang wrote:

Hi Naoto,

The method findNegativeSavings method in TzdbZoneRulesProvider.java 
states that it "Find the minimum negative savings". While the result is 
correct since the rules all have the same value for SAVE, I wonder if 
that's ideal conceptually. Given a start LDT, shouldn't it be looking 
for the SAVE in the exact (narrower) date range (e.g. 1981 - 1989 vs 
1981 - max)?.


I believe it is working as such. The end year is retrieved within the 
method (line 879) and only the minimum negative saving values within the 
window is filtered.




NegativeDSTTest verifies the tzdata, that is the adjusted data after 
import, is that correct? I wonder a comment and a bit of details in the 
test summary would be helpful since there is no negative data in the 
test itself.


Good point. It is confusing. I supplied summary text in the test (also 
the similar line in TestZoneRules.java)


Here is the updated webrev:

http://cr.openjdk.java.net/~naoto/8212970/webrev.11/

Naoto


Best,
Joe

On 7/23/19 3:15 PM, naoto.s...@oracle.com wrote:

Hi,

Please review the fix to the following enhancement:

https://bugs.openjdk.java.net/browse/JDK-8212970

The proposed changeset is located at:

https://cr.openjdk.java.net/~naoto/8212970/webrev.09/

This change aims to support the "vanguard" IANA time zone data format, 
which uses the negative savings and transition time beyond a day 
period. The change basically translates those negative savings and 
transition times, such as 25:00, into the ones that the current JDK 
recognizes, then produces the data file "tzdb.dat" at the build time. 
At the run time, the data file is read and interpreted as before. This 
way the produced tzdb.dat is compatible with the prior JDK releases so 
that the TZ Updater can also distribute it as a time zone update.


I have also refactored redundant copy of ZoneRules file in the build 
directory, by dynamically importing the file under src. Thus some 
build related files are modified. I am hoping folks on the build-dev 
can review those changes.


Naoto




[14] RFR: 8212970: TZ database in "vanguard" format support

2019-07-23 Thread naoto . sato

Hi,

Please review the fix to the following enhancement:

https://bugs.openjdk.java.net/browse/JDK-8212970

The proposed changeset is located at:

https://cr.openjdk.java.net/~naoto/8212970/webrev.09/

This change aims to support the "vanguard" IANA time zone data format, 
which uses the negative savings and transition time beyond a day period. 
The change basically translates those negative savings and transition 
times, such as 25:00, into the ones that the current JDK recognizes, 
then produces the data file "tzdb.dat" at the build time. At the run 
time, the data file is read and interpreted as before. This way the 
produced tzdb.dat is compatible with the prior JDK releases so that the 
TZ Updater can also distribute it as a time zone update.


I have also refactored redundant copy of ZoneRules file in the build 
directory, by dynamically importing the file under src. Thus some build 
related files are modified. I am hoping folks on the build-dev can 
review those changes.


Naoto


Re: [13] RFR: 8221431: Support for Unicode 12.1

2019-05-22 Thread naoto . sato

Hi Erik,

Thank you for your comments. Updated the webrev accordingly:

https://cr.openjdk.java.net/~naoto/8221431/webrev.04/

Naoto

On 5/22/19 4:13 PM, Erik Joelsson wrote:

Hello Naoto,

In GensrcEmojiData.gmk: The MakeDir doesn't look correct with the double 
$$. I would recommend calling the newer MakeTargetDir macro instead. It 
doesn't take an argument.


Otherwise build changes look good.

/Erik

On 2019-05-22 15:56, naoto.s...@oracle.com wrote:
Adding build-dev, as the change adds a small build tool to parse 
emoji-data.


Naoto

On 5/22/19 3:26 PM, naoto.s...@oracle.com wrote:

Hi,

Please review the changes to the following issue:

https://bugs.openjdk.java.net/browse/JDK-8221431

The proposed CSR and changeset are located at:

https://bugs.openjdk.java.net/browse/JDK-8222771
https://cr.openjdk.java.net/~naoto/8221431/webrev.03/

This enhancement incorporates support for the recently released 
Unicode version 12.1. Besides the usual addition of new 
characters/scripts, this enhancement includes normalization of the 
Japanese Reiwa era character (U+32FF), and up-to-date extended 
grapheme cluster support in regex package.


Naoto


Re: [13] RFR: 8221431: Support for Unicode 12.1

2019-05-22 Thread naoto . sato

Adding build-dev, as the change adds a small build tool to parse emoji-data.

Naoto

On 5/22/19 3:26 PM, naoto.s...@oracle.com wrote:

Hi,

Please review the changes to the following issue:

https://bugs.openjdk.java.net/browse/JDK-8221431

The proposed CSR and changeset are located at:

https://bugs.openjdk.java.net/browse/JDK-8222771
https://cr.openjdk.java.net/~naoto/8221431/webrev.03/

This enhancement incorporates support for the recently released Unicode 
version 12.1. Besides the usual addition of new characters/scripts, this 
enhancement includes normalization of the Japanese Reiwa era character 
(U+32FF), and up-to-date extended grapheme cluster support in regex 
package.


Naoto


Re: Add a suggestion for non-English locale of Linux in the test doc

2019-04-15 Thread naoto . sato
As for the wording, I'd suggest "Non-US Locale" instead of 
"Non-English." Some tests may depend on US customary behavior, such as 
date format, decimal separator, etc.


Naoto

On 4/15/19 6:59 AM, Erik Joelsson wrote:

Hello,

Documenting this is certainly the least we can do. If our tests depend 
on the locale being set to en_US, then I think the best action would be 
to provide such a configuration directly in RunTests.gmk. Exporting LANG 
should work for all Unix OSes, but most likely not on Windows. The extra 
VM_OPTIONS would fix most of them it seems. Would it be worth 
investigating the remaining 7 and get them fixed?


In the meantime, documenting seems prudent. I would suggest something 
like this:


### Non-English Locale

If your locale is non-English, some tests are likely to fail. To work 
around this you can set the locale to English. On Unix platforms simply 
setting `LANG=en_US` in the environment before running tests should 
work. On Windows, setting `JTREG="VM_OPTIONS=-Duser.language=en 
-Duser.country=US"` helps for most, but not all test cases.


/Erik

On 2019-04-14 20:28, Jing Tian wrote:

Hi,

We have discussed the issue of the test cases fail because of locale 
before[1].


Thanks for the suggestions given by Naoto and David. I think we can 
put this advice in the test doc, which may be better for people to 
test. This advice can avoid the problem that caused by locale and we 
can pay more attention to the functional points that the test itself 
focuses on.


Set JTREG="VM_OPTIONS=-Duser.language=en -Duser.country=US" , it does 
pass most test cases, but there are still very few test cases(7 in 
total) can't pass the test when they are in a non-English locale.


I think if 'make test' in a non-English locale, we can set the locale 
to English first. Use 'export LANG="en_US"'. But this method is just 
for Linux. I test "tier1 tier2 tier3" after setting LANG="en_US". The 
problems caused by the local settings have not appeared anymore.



JBS: https://bugs.openjdk.java.net/browse/JDK-8222444
Webrev: http://cr.openjdk.java.net/~lzhai/8222444/webrev.00/

The testing.html is updated automatically using "make 
update-build-docs" with pandoc version 2.7.2.


[1] 
https://mail.openjdk.java.net/pipermail/compiler-dev/2019-March/013144.html 
 



Cheers,
Jing Tian



Re: RFR: JDK-8218186 Clean up CLDR generation in build

2019-02-05 Thread Naoto Sato

Thanks, Magnus. The change looks good to me too.

Naoto

On 2/5/19 1:57 AM, Magnus Ihse Bursie wrote:

On 2019-02-01 14:43, naoto.s...@oracle.com wrote:

Hi Magnus,

I am assuming that the generated resource bundles are exactly the same 
as before this fix, correct?


Yes. I have verified this using the COMPARE_BUILD functionality.

/Magnus


Naoto

On 2/1/19 2:50 AM, Magnus Ihse Bursie wrote:
The CLDR data is, since Jigsaw, used in two different modules -- 
java.base and jdk.localedata. Unfortunately, the split between these 
two modules were not fully finished as part of the Jigsaw project.


This patch aims to resolving most of this. The CLDRConverter build 
tool is now called from Gensrc-java.base and Gensrc-jdk.localedata, 
for their respective module. The calls have been updated to match 
modern build-infra standards.


Also, the raw CLDR data was located mixed in with the Java source, in 
jdk.localedata. This patch also moves the data to make/data/cldr, to 
align with input data to all other build tools.


Bug: https://bugs.openjdk.java.net/browse/JDK-8218186
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8218186-cleanup-CLDR/webrev.01


/Magnus




Re: RFR: JDK-8218186 Clean up CLDR generation in build

2019-02-01 Thread naoto . sato

Hi Magnus,

I am assuming that the generated resource bundles are exactly the same 
as before this fix, correct?


Naoto

On 2/1/19 2:50 AM, Magnus Ihse Bursie wrote:
The CLDR data is, since Jigsaw, used in two different modules -- 
java.base and jdk.localedata. Unfortunately, the split between these two 
modules were not fully finished as part of the Jigsaw project.


This patch aims to resolving most of this. The CLDRConverter build tool 
is now called from Gensrc-java.base and Gensrc-jdk.localedata, for their 
respective module. The calls have been updated to match modern 
build-infra standards.


Also, the raw CLDR data was located mixed in with the Java source, in 
jdk.localedata. This patch also moves the data to make/data/cldr, to 
align with input data to all other build tools.


Bug: https://bugs.openjdk.java.net/browse/JDK-8218186
WebRev: http://cr.openjdk.java.net/~ihse/JDK-8218186-cleanup-CLDR/webrev.01

/Magnus


[12] RFR: 8209880: tzdb.dat is not reproducibly built

2018-09-18 Thread Naoto Sato

Hello,

Please review the fix to the following issue:

https://bugs.openjdk.java.net/browse/JDK-8209880

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/8209880/webrev.00/

The fix is to remove the use of HashSet/Map which is not guaranteed to 
preserve the same order on iteration.


Naoto


Re: [12]: RFR: 8209167: Use CLDR's time zone mappings for Windows

2018-09-12 Thread naoto . sato

Hi Magnus, Erik,

Thank you for your comments. I updated the webrev according to your 
suggestions:


http://cr.openjdk.java.net/~naoto/8209167/webrev.02/

As to the duplicated "common" in the dependency, yes that's a separate 
issue. Since it's obvious, I included the fix with this changeset (it 
was actually described as a comment in the jira issue).


Naoto

On 9/12/18 9:08 AM, Erik Joelsson wrote:

On 2018-09-12 03:19, Magnus Ihse Bursie wrote:



On 2018-09-10 23:34, Naoto Sato wrote:

Hello,

Please review the fix to the following issue:

https://bugs.openjdk.java.net/browse/JDK-8209167

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/8209167/webrev.01/


Some comments:
In make/copy/Copy-java.base.gmk:
+ifneq ($(findstring $(OPENJDK_TARGET_OS), aix),)

The findstring construct is hard to read and only needed when we test 
more than one OS. Please rewrite as a single ifeq test for aix.


In GensrcCLDR.gmk:
I don't understand the CLDR_WINTZMAPPINGS file? There's no rule to 
generate it, there's just a dependency..?


It's generated by the same rule as the other file. This is the easiest 
workaround for two files generated from the same rule. It sort of works 
(for clean builds and if the input is chagned), but won't handle all 
cases where one file is removed externally and the other is not. I 
suggested this construct to Naoto. Perhaps a comment explaining this 
would be good.
The removal of the duplicate "common", that seems like a separate bug 
fix?



It does indeed. Before this fix, the wildcards must have returned empty.

/Erik

/Magnus



This fix is to remove the hand crafted map file (lib/tzmappings) on 
Windows, which maps Windows time zones to Java's tzid. It is now 
dynamically generated at the build time, using CLDR's data file 
(windowsZones.xml)


Naoto






[12]: RFR: Use CLDR's time zone mappings for Windows

2018-09-10 Thread Naoto Sato

Hello,

Please review the fix to the following issue:

https://bugs.openjdk.java.net/browse/JDK-8209167

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/8209167/webrev.01/

This fix is to remove the hand crafted map file (lib/tzmappings) on 
Windows, which maps Windows time zones to Java's tzid. It is now 
dynamically generated at the build time, using CLDR's data file 
(windowsZones.xml)


Naoto


Re: RFR: JDK-8204973: Add build support for filtering translations

2018-06-14 Thread Naoto Sato

Looks good to me.

Naoto

On 6/14/18 11:52 AM, Erik Joelsson wrote:

Hello,

Here is a new version of the patch:

http://cr.openjdk.java.net/~erikj/8204973/webrev.02/

Changes from last time:

* Made the regexp for finding locales more correct. It still does not 
try to match 3 letter language strings because doing so triggers a large 
amount of false positives in our souce tree.


* Added another accepted locale (en_US_POSIX) that is now matched by the 
improved regexp.


* Added more locales to the exclude list as they were now discovered by 
the improved regexp.


/Erik


On 2018-06-13 12:47, Erik Joelsson wrote:

Hello,

Oracle will reduce the number of languages that it maintains 
translations of JDK resources for. The current translations will 
remain in the source for now, but we need a way to filter out a set of 
translations at build time so that we only include the ones we 
support. This patch adds such a configuration option. It also changes 
how Oracle builds by using the option to exclude all translations 
except English, Japanese, Simplified Chinese and Traditional Chinese. 
Anyone else building OpenJDK will by default include all translations 
present in the source, just as before.


I added a test that verifies this for builds with the "IMPLEMENTOR" 
field in the release file set to "Oracle Corporation". The test will 
not be run for other OpenJDK builds.


I had to modify an existing test for java.logging which used various 
translations to verify localized log messages to only use translations 
that Oracle chooses to include.


Since this is the second test that specifically verifies build 
behavior, I moved the previous such test together with this new test 
into a common top level test directory "build", under the jdk test 
root. I put these tests in the jdk tier3 test group.


I have run all tier1, 2 and 3 tests in Mach 5 as well as specifically 
looked for tests that use the java.util.Locale class and ran them 
locally.


Webrev: http://cr.openjdk.java.net/~erikj/8204973/webrev.01/index.html

Bug: https://bugs.openjdk.java.net/browse/JDK-8204973

/Erik





[11] RFR: 8201507: Generate alias entries in j.t.f.ZoneName from tzdb at build time

2018-04-12 Thread Naoto Sato

Hi,

Please review the fix to the subject issue. While fixing 8189784 [1], I 
noticed that not only CLDR zones but also tzdb link entries are also 
hard coded. So I further modified j.t.f.ZoneName to generate tzdb 
entries at the build time.


The issue: https://bugs.openjdk.java.net/browse/JDK-8201507
Fix: http://cr.openjdk.java.net/~naoto/8201507/webrev.00/

Like the last time, including build-dev for makefile modification review.

Naoto

[1] http://mail.openjdk.java.net/pipermail/i18n-dev/2018-April/002492.html


Re: [11] RFR: 8189784: Parsing with Java 9 AKST timezone returns the SystemV variant of the timezone

2018-04-09 Thread naoto . sato

Thanks, Erik. Modified GensrcCLDR.gmk as suggested:

http://cr.openjdk.java.net/~naoto/8189784/webrev.03/

Naoto

On 4/9/18 4:15 PM, Erik Joelsson wrote:

Hello Naoto,

Looks good, just a style issue.

When breaking a recipe line, please add 4 additional spaces (after the 
tab) for continuation indent [1]. In this case I would also advocate 
breaking the line sooner to make side by side comparisons easier in the 
future.


/Erik

[1] http://openjdk.java.net/groups/build/doc/code-conventions.html

On 2018-04-09 13:20, Naoto Sato wrote:

Hello,

Please review the changes to the following issue:

https://bugs.openjdk.java.net/browse/JDK-8189784

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/8189784/webrev.02/

There were two causes of the issue. One was that j.t.f.ZoneName 
contained hard coded mappings based on the old CLDR data and never 
been updated. The other reason was that CLDR's deprecated zones 
("SystemV" ones, in this case) were not properly replaced.


I am including build-dev for the review, as the change includes 
generating ZoneName mapping at the build time from the template.


Naoto




[11] RFR: 8189784: Parsing with Java 9 AKST timezone returns the SystemV variant of the timezone

2018-04-09 Thread Naoto Sato

Hello,

Please review the changes to the following issue:

https://bugs.openjdk.java.net/browse/JDK-8189784

The proposed changeset is located at:

http://cr.openjdk.java.net/~naoto/8189784/webrev.02/

There were two causes of the issue. One was that j.t.f.ZoneName 
contained hard coded mappings based on the old CLDR data and never been 
updated. The other reason was that CLDR's deprecated zones ("SystemV" 
ones, in this case) were not properly replaced.


I am including build-dev for the review, as the change includes 
generating ZoneName mapping at the build time from the template.


Naoto


Re: [10] RFR JDK-8177472: Remove hard-coded IANA Subtag Registry map in LocaleEquivalentMap.java

2017-07-20 Thread Naoto Sato

Looks good to me.

Naoto

On 7/19/17 10:33 PM, Nishit Jain wrote:

Hi,

Please review the fix for JDK-8177472

Bug: https://bugs.openjdk.java.net/browse/JDK-8177472
Webrev: http://cr.openjdk.java.net/~nishjain/8177472/webrev.05/

Issue:The existing process of updating the LSR data requires manual 
execution of EquivMapsGenerator.java for generation of the updated maps.
Fix:The execution of EquivMapsGenerator.javafor generation of maps is 
included in the JDK build process. After this change updation of LSR 
data will mostly require replacement of old language-subtag-registry.txt 
file with the new one and changing the copyright year in 
EquivMapsGenerator.java (if required) for the generated maps file.


Regards,
Nishit Jain


Re: Review Request JDK-8169925: Organize licenses by module in source, JMOD file, and run-time image

2016-12-07 Thread Naoto Sato

Hi Mandy,

Just noticed that the CLDR license is located under jdk.localedata 
module. That needs to be moved into java.base, as its English resource 
files are derived from CLDR too.


Naoto

On 12/7/16 1:28 PM, Mandy Chung wrote:

This proposes to organize license files by module in source, JMOD,
and run-time image.

A summary of the proposal:
1. Organize third party notices by module in the source as follows:
src/$MODULE/{share,$OS}/legal/*

The `legal` directory contains one file for each third party
library in the module, for example,
src/java.base/share/legal/asm.md
  unicode.md
  zlib.md

The proposed template for this file is described in [1] and JEP 201
will be updated to reflect this proposed source layout.

2. Introduce a new LEGAL_NOTICES section in JMOD format. A new jmod
   option `—-legal-notices` is added to package legal notices in
   a JMOD file.

3. At jlink time, jlink will copy all legal notices from JMOD files
   to the `legal` directory in the run-time image.  A plugin is
   added to de-duplicate the legal notices if the filename and the
   content matches that may reduce the image footprint.

4. THIRD_PARTY_README in the top-level directory of each repo is removed.
   Manual edit to this file, multiple copies is no longer needed.

Webrev at:
   http://cr.openjdk.java.net/~mchung/jdk9/webrevs/8169925/webrev.00/

Mandy
[1] https://bugs.openjdk.java.net/browse/JDK-8169925



Re: RFR: 8165804: Revisit the way of loading BreakIterator rules/dictionaries

2016-10-20 Thread Naoto Sato

Looks good to me.

Naoto

On 10/20/16 1:33 AM, Masayoshi Okutsu wrote:

Hi,

Please review the changes for JDK-8165804 which is a follow-up of
JDK-8076757. Some notes on the changes.

- Removed INCLUDES := $(TEXT_PKG_LD) from
make/gendata/GendataBreakIterator.gmk in order to avoid compiling
non-BreakIterator*.java for the build tool with boot JDK.

- Added sun.util.resources.BreakIteratorResourceBundle which handles
loading of rule data and dictionaries. BreadIteratorResources* are its
subclasses in the java.base and jdk.localedata modules.

- In BreakIteratorResources*, regular ResourceBundle loading of
BreakIteratorInfo* is avoided because a BreakIteratorInfo can't have its
parent chain. For example, BreakIteratorInfo_th doesn't have the value
for key "CharacterData" and BreakIteratorResources_th.handleGetObject()
must return null for "CharacterData" rather than loading rule data given
by the parent BreakIteratorInfo.

- Moved RuleBasedBreakIterator, DictionaryBasedBreakIterator, and
BreakDictionary to sun.text. BreakIteratorProviderTest.java was changed
to deal with this refactoring.

Issue:
https://bugs.openjdk.java.net/browse/JDK-8165804

Webrev:
http://cr.openjdk.java.net/~okutsu/9/8165804/webrev.01

Thanks,
Masayoshi



Re: [9] RFR: 8160873: (cs) JDK9 Build failure on Hindi locale

2016-07-20 Thread Naoto Sato

Thanks for the review Tim!

On 7/20/16 5:40 PM, Tim Bell wrote:

Naoto:


Please review the fix to the following issue:

https://bugs.openjdk.java.net/browse/JDK-8160873

The proposed fix is located at:

http://cr.openjdk.java.net/~naoto/8160873/webrev.01/

The gist of the issue is that those tools used to generate sources at
build time were affected by the default locale, and if they use some
number formatting functions, the results end up including locale
dependent digits. The fix is to force tools to run in en-US locale by
providing user.language/country properties to bootjdk arguments.


Looks good to me.

It is already common practice for the build to set:

  LC_ALL=C
  export LC_ALL


That works fine on Solaris/Linux platoforms, but for Windows/Linux, 
Java's default locale is determined from the 
ControlPanel/SystemPreferences user settings, thus those system 
properties need to be set.


Naoto


[9] RFR: 8160873: (cs) JDK9 Build failure on Hindi locale

2016-07-18 Thread Naoto Sato

Hello,

Please review the fix to the following issue:

https://bugs.openjdk.java.net/browse/JDK-8160873

The proposed fix is located at:

http://cr.openjdk.java.net/~naoto/8160873/webrev.01/

The gist of the issue is that those tools used to generate sources at 
build time were affected by the default locale, and if they use some 
number formatting functions, the results end up including locale 
dependent digits. The fix is to force tools to run in en-US locale by 
providing user.language/country properties to bootjdk arguments.


Naoto


Re: RFR: 8136556 - Add the ability to perform static builds of MacOSX x64 binaries

2015-10-16 Thread Naoto Sato

Hi Bob, Alan,

On 10/16/15 8:11 AM, Bob Vandette wrote:

I've skimmed through the patches and the DEF_* macros look okay. The only one 
that doesn't look right is jawt.h/jawt.c. As jawt.h is shipped by the JDK then 
I think the include of jni_util.h needs to move from jawt.h to jawt.c.

Ok, I’ll take care of that.


If I read the changes correctly then not loading the 
JavaRuntimeSupport.framework on Mac means the locale might not be right, is 
that correct? Brent might remember this issue as we've often pondered the 
implications of this disappearing in an Mac update.

The implementation falls back to the getPosixLocale function when the Framework 
doesn’t exist.


In that case, the locale settings in the MacOSX's Settings will not be 
reflected in Java's default locale.


Naoto


[9] RFR: 8129881, 8130845, 8132125

2015-08-03 Thread Naoto Sato

Hello,

Please review the changes for the following issues that are the 
regressions for fixing 8008577:


8129881: 8008577 breaks Nashorn test 
(https://bugs.openjdk.java.net/browse/JDK-8129881)
8130845: Change to CLDR Locale data in JDK 9 b71 causes SimpleDateFormat 
parsing errors (https://bugs.openjdk.java.net/browse/JDK-8130845)
8132125: German (Switzerland) formatting broken if CLDR Locale Data is 
used (https://bugs.openjdk.java.net/browse/JDK-8132125)


The proposed fixes are located at:

http://cr.openjdk.java.net/~naoto/8129881.8130845.8132125/webrev.00/

Since this changeset inculudes build changes, I'd like the build group 
to review those makefile changes.


Naoto


Re: RFR: JDK-8130109: Incremental build of java.base-gensrc broken

2015-06-30 Thread Naoto Sato

+1

Naoto

On 6/30/15 12:49 AM, Erik Joelsson wrote:

Hello,

Please review this small patch which fixes the incremental build of
java.base-gensrc. There is a slight mistake in
jdk/make/gensrc/GensrcCLDR.gmk where the target file of a rule has the
wrong path, so make will always think it needs to be generated.

Bug: https://bugs.openjdk.java.net/browse/JDK-8130109

Patch:

diff -r 68ce12551103 make/gensrc/GensrcCLDR.gmk
--- a/make/gensrc/GensrcCLDR.gmk
+++ b/make/gensrc/GensrcCLDR.gmk
@@ -29,7 +29,7 @@
  GENSRC_BASEDIR := $(SUPPORT_OUTPUTDIR)/gensrc/java.base
  GENSRC_DIR := $(SUPPORT_OUTPUTDIR)/gensrc/jdk.localedata

-CLDR_BASEMETAINFO_FILE :=
$(GENSRC_DIR)/sun/util/cldr/CLDRBaseLocaleDataMetaInfo.java
+CLDR_BASEMETAINFO_FILE :=
$(GENSRC_BASEDIR)/sun/util/cldr/CLDRBaseLocaleDataMetaInfo.java
  CLDR_METAINFO_FILE :=
$(GENSRC_DIR)/sun/util/resources/cldr/provider/CLDRLocaleDataMetaInfo_jdk_localedata.java


  CLDR_BASE_LOCALES := en-US

/Erik


Re: i18n dev [9] RFR: 8008577: Use CLDR Locale Data by Default (JEP-252)

2015-06-24 Thread Naoto Sato

Thanks. Here is the diff from webrev.01 to address your comment:

http://hg.openjdk.java.net/jdk9/sandbox/jdk/rev/b8faab65bb62

Naoto

On 6/24/15 2:16 AM, Masayoshi Okutsu wrote:

applyParentLocales() sets parentLocalesMap before populating the map
with data. It's possible that other threads look up the map without the
(full) data. So, a Map (local variable) should be populated and then
parentLocalesMap should be set to the Map. Also, parentLocalesMap needs
to be volatile or synchronized should be used when setting it. I don't
think it's necessary to use ConcurrentHashMap if the map is not modified
after its initialization.

Otherwise, the changes look good to me.

Masayoshi

On 6/24/2015 6:56 AM, Naoto Sato wrote:

Here is the updated webrev:

http://cr.openjdk.java.net/~naoto/8008577/webrev.01/

As to the 3rd comment below, I did not modify it because that would
simply duplicate the same piece of code in each getCandidateLocales()
implementation (from the current location).

Naoto

On 6/19/15 1:53 AM, Masayoshi Okutsu wrote:

Sorry for taking time. Here are my comments.

src/jdk.localedata/share/classes/sun/text/resources/*JavaTimeSupplementary*.java:


  - The year range of the first line of the copyright header should be
2015, for the new ones and 2013, 2015, for the updated ones.
  - I wonder if JavaTimeSupplementary*.java contain some CR-LF lines
because some line spacing is wide in the webrev. (hard to determine that
from the webrev.)

src/java.base/share/classes/sun/util/cldr/CLDRLocaleProviderAdapter.java:

  - applyParentLocales() needs to be thread-safe with parentLocalesMap?

src/java.base/share/classes/sun/util/resources/LocaleData.java:
  - I wonder if the ResourceBundleBasedAdapter.getCandidateLocales()
implementations should return an optimized candidate list rather than
removing unsupported Locales from the list there.

test/sun/text/resources/LocaleDataTest.java:

+ * -cldr option specifies to test CLDR locale data. The default data
file name for this
+ * option is CLDRLocaleData.

  - The file name should be LocaleData.cldr?

test/java/text/Format/DecimalFormat/RoundingAndPropertyTest.java:
  - added the bug id, but no changes to the test?

Thanks,
Masayoshi

On 6/9/2015 5:58 AM, Naoto Sato wrote:

Hello,

Please review the proposed changes for 8008577[1], the implementation
of the JEP-252[2]. The proposed changes are located at:

http://cr.openjdk.java.net/~naoto/8008577/webrev.00/

Here are the very high level summary of changes:

- Now the default locale provider order is CLDR,JRE,SPI.
- The CLDR data is upgraded from 21.0.1 to 27.0.0
- According the CLDR upgrade, the converter tool and the runtime are
now capable of alias and parentLocales tags.
- Regression tests that are specific to the existing JRE locale data
are now run specifically with JRE,SPI providers.

I would like the build group to review the build changes mainly with
the CLDR changes, and i18n group for the rest.

Naoto
--
[1]: https://bugs.openjdk.java.net/browse/JDK-8008577
[2]: https://bugs.openjdk.java.net/browse/JDK-8043554






Re: i18n dev [9] RFR: 8008577: Use CLDR Locale Data by Default (JEP-252)

2015-06-23 Thread Naoto Sato

Here is the updated webrev:

http://cr.openjdk.java.net/~naoto/8008577/webrev.01/

As to the 3rd comment below, I did not modify it because that would 
simply duplicate the same piece of code in each getCandidateLocales() 
implementation (from the current location).


Naoto

On 6/19/15 1:53 AM, Masayoshi Okutsu wrote:

Sorry for taking time. Here are my comments.

src/jdk.localedata/share/classes/sun/text/resources/*JavaTimeSupplementary*.java:

  - The year range of the first line of the copyright header should be
2015, for the new ones and 2013, 2015, for the updated ones.
  - I wonder if JavaTimeSupplementary*.java contain some CR-LF lines
because some line spacing is wide in the webrev. (hard to determine that
from the webrev.)

src/java.base/share/classes/sun/util/cldr/CLDRLocaleProviderAdapter.java:
  - applyParentLocales() needs to be thread-safe with parentLocalesMap?

src/java.base/share/classes/sun/util/resources/LocaleData.java:
  - I wonder if the ResourceBundleBasedAdapter.getCandidateLocales()
implementations should return an optimized candidate list rather than
removing unsupported Locales from the list there.

test/sun/text/resources/LocaleDataTest.java:

+ * -cldr option specifies to test CLDR locale data. The default data
file name for this
+ * option is CLDRLocaleData.

  - The file name should be LocaleData.cldr?

test/java/text/Format/DecimalFormat/RoundingAndPropertyTest.java:
  - added the bug id, but no changes to the test?

Thanks,
Masayoshi

On 6/9/2015 5:58 AM, Naoto Sato wrote:

Hello,

Please review the proposed changes for 8008577[1], the implementation
of the JEP-252[2]. The proposed changes are located at:

http://cr.openjdk.java.net/~naoto/8008577/webrev.00/

Here are the very high level summary of changes:

- Now the default locale provider order is CLDR,JRE,SPI.
- The CLDR data is upgraded from 21.0.1 to 27.0.0
- According the CLDR upgrade, the converter tool and the runtime are
now capable of alias and parentLocales tags.
- Regression tests that are specific to the existing JRE locale data
are now run specifically with JRE,SPI providers.

I would like the build group to review the build changes mainly with
the CLDR changes, and i18n group for the rest.

Naoto
--
[1]: https://bugs.openjdk.java.net/browse/JDK-8008577
[2]: https://bugs.openjdk.java.net/browse/JDK-8043554




  1   2   >