Re: RFR: 8241055: Regex Grapheme Matcher Performance Depends too much on Total Input Sequence Size

Hi Philipp, Thanks for the update. I like you enriched the RegExTest.java. Two nits: - RegExTest.java at line 4805: I'd use parentheses for this return statement. - PatternBench.java should keep the original copyright year, i.e., "2020," -> "2019, 2020," Otherwise, it looks good. Naoto O

Re: RFR: 8241055: Regex Grapheme Matcher Performance Depends too much on Total Input Sequence Size

Looks good. As you noticed, the error does not seem to occur with JDK14 as bootjdk. I will push it after I confirmed OK on my side. Thank you for your contribution. Naoto On 4/16/20 12:26 PM, Philipp Kunz wrote: Hi Naoto, Changed as you suggested. However, I could not verify it due to, mos

Re: RFR: 8242541: Small charset issues (ISO8859-16, x-eucJP-Open, x-IBM834 and x-IBM949C)

Hi Takiguchi-san, Change looks good. I'd expect a test case in open/test/jdk/java/nio/charset/Charset/RegisteredCharsets.java for the added "ISO8859_16" alias. Naoto On 4/20/20 6:37 PM, Ichiroh Takiguchi wrote: Hello. Could you review the fix ? Bug:    https://bugs.openjdk.java.net/browse

Re: RFR: 8243254: Examine ZipFile slash optimization for non-ASCII compatible charsets

+1 Naoto On 4/22/20 9:13 AM, Lance Andersen wrote: Hi Claes, The latest version looks good. Thank you for the patch. Best Lance On Apr 22, 2020, at 6:26 AM, Claes Redestad wrote: Hi, new webrev based on discussions here and offline: http://cr.openjdk.java.net/~redestad/8243254/open.01/

[15] RFR: 8243664: JavaDoc of CompactNumberFormat points to wrong enum

Hello, Please review this small doc fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8243664 Here is the diff: --- --- old/src/java.base/share/classes/java/text/CompactNumberFormat.java 2020-04-27 09:09:14.0 -0700 +++ new/src/java.base/share/classes/java/text/Compa

Re: RFR [15] 8243541: (tz) Upgrade time-zone data to tzdata2020a

Looks OK to me. Thanks for the update. Naoto On 4/27/20 11:19 AM, Kiran Ravikumar wrote: Hi Martin and Andrew, Thanks for the review and providing a direction towards updating the translations. Here is an updated webrev with the changes: http://cr.openjdk.java.net/~kravikumar/8243541/web

Re: RFR: 8242541: Small charset issues (ISO8859-16, x-eucJP-Open, x-IBM834 and x-IBM949C)

Looks good. Naoto On 4/28/20 11:41 AM, Ichiroh Takiguchi wrote: Hello Naoto. Thank you for your attention. And I'm sorry about bad response. I added ISO8859_16 entry into test/jdk/java/nio/charset/Charset/RegisteredCharsets.java Could you review the fix again ? Bug:    https://bugs.openjd

[15] RFR: 8244152: Remove unnecessary hash map resize in LocaleProviderAdapters

Hello, Please review this small fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8244152 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8244152/webrev.00/ The hash map used there didn't have initial capacity, even though the exact numbers are kno

Re: RFR: 8244224: Implementation of JEP 381: Remove the Solaris and SPARC Ports - (core libraries)

Hi Mikael, I took a look at i18n related files. It looks good overall. One nit in java/nio/charset/Charset/DefaultCharsetTest.java: If the test is only applicable to linux (@requires os.family == "linux" in jtreg tag after the change), the condition isLinux() is no longer needed at line 73.

Re: [15] RFR: 8244152: Remove unnecessary hash map resize in LocaleProviderAdapters

Hi Peter, You are correct. Thanks. I'll remove that initial value of 16. Naoto On 5/5/20 9:37 AM, Peter Levart wrote: Hi Naoto, On 4/30/20 12:18 AM, [email protected] wrote: Hello, Please review this small fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8244152 Th

[15] RFR: 8244459: Optimize the hash map size in LocaleProviderAdapters

And here is the fix. Please review. http://cr.openjdk.java.net/~naoto/8244459/webrev.00/ Naoto On 5/5/20 10:25 AM, [email protected] wrote: Hi Peter, You are correct. Thanks. I'll remove that initial value of 16. Naoto On 5/5/20 9:37 AM, Peter Levart wrote: Hi Naoto, On 4/30/20 12:18

Re: [15] RFR: 8244459: Optimize the hash map size in LocaleProviderAdapters

Thanks, all. I didn't see this coming! If I understand the discussion correctly, Peter's suggestion is the most optimal (Mark, your formula produces 1 for the expected size is 0, although it won't be happening in this particular case). And Joe, thank you for finding my silly mistake :-) So her

Re: [15] RFR: 8244459: Optimize the hash map size in LocaleProviderAdapters

Hi Stuart, On 5/5/20 4:29 PM, Stuart Marks wrote: As for Naoto's changeset, I don't think it should be held up while we bikeshed this. :-) I'm ok with whatever he decides. I don't think the number of language tags exceed 2^29, unless languages in the whole universe are counted :-) So I would

[15]: RFR: 8244245: localizedBy() should override localized values with default values

Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8244245 The CSR and proposed changeset are located at: https://bugs.openjdk.java.net/browse/JDK-8244246 https://cr.openjdk.java.net/~naoto/8244245/webrev.00/ This stems from the closed issue (https:

Re: [15]: RFR: 8244245: localizedBy() should override localized values with default values

Hi Roger, Thank you for the review. Wrapped the long lines as suggested, along with some typo fixes in the comments: https://cr.openjdk.java.net/~naoto/8244245/webrev.01/ Naoto On 5/7/20 7:43 AM, Roger Riggs wrote: Hi Naoto, Looks fine with a small source edit below. TestUnicodeExtension.

Re: [15]: RFR: 8244245: localizedBy() should override localized values with default values

Hi Joe, Thank you for the review. The removed check was explicitly avoiding the default chrono/number in the locale overriding the current locale values, which is exactly what this issue is trying to remove. As Stephen wrote in another email, Unicode Extensions are correctly dealt in Chronolo

[15] RFR: 8239383: Support for Unicode 13.0

Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8239383 Its CSR and proposed changeset are located at: https://bugs.openjdk.java.net/browse/JDK-8239504 https://cr.openjdk.java.net/~naoto/8239383/webrev.00/ This updates java.lang.Character class to

Re: JDK-8226810: An other case and a small change suggestion

Ditto. Good catch! I am not sure the fix would address the issue in 8226810 (cannot confirm it either, as my Windows box is at my office where I cannot enter at the moment :-), but this definitely looks like a bug. I would change the additional line to "strcpy(ret+2, "1252");" as Cp is copied

Re: JDK-8226810: An other case and a small change suggestion

Hi Johannes, On 5/8/20 1:37 PM, Johannes Kuhn wrote: Thanks. I think strcpy(ret+2, "1252") vs. strcpy(ret, "Cp1252")  is a just matter of style. I would prefer the later, as it makes the intent clear. But not my call. I thought the former was clearer, as at that point, Cp/MS/GB part is not

Re: RFR: 8244767 - Potential non-terminated string in getEncodingInternal() on Windows

+1 Naoto On 5/11/20 5:25 PM, Brian Burkhalter wrote: Hi Brent, It looks OK to me. Brian On May 11, 2020, at 4:36 PM, Brent Christian wrote: Please review this small fix in Windows native code: Bug: https://bugs.openjdk.java.net/browse/JDK-8244767

Re: RFR: 8244855 : Remove unused "getParent" function from Windows jni_util_md.c

Looks good, with the change to copyright year to "2020." Naoto On 5/12/20 12:12 PM, Lance @ Oracle wrote: +1 Best, Lance Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 [email protected] Sent from my

Re: [15] RFR: 8239383: Support for Unicode 13.0

Ping. Naoto On 5/7/20 2:59 PM, [email protected] wrote: Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8239383 Its CSR and proposed changeset are located at: https://bugs.openjdk.java.net/browse/JDK-8239504 https://cr.openjdk.java.net/~naot

Re: [15] RFR: 8239383: Support for Unicode 13.0

Thanks for the review, Joe! On 5/13/20 2:58 PM, Joe Wang wrote: +1 They are from the reference files, but it's interesting to note the ranges in Blocks.txt (that Character.java follows) for Tangut Supplement (18D00..18D8F) and CJK Unified Ideographs Extension G (3..3134F) are slightly di

Re: RFR [15] 8244342: Compilation warnings about unexpected serialization related method signatures.

+1 Naoto On 5/15/20 11:35 AM, Lance Andersen wrote: Looks ok joe On May 15, 2020, at 1:54 PM, Joe Wang wrote: Hi, Please review a fix for the compilation warnings. Thanks Roger for the detailed instructions! If you could verify the fix with the work-in-progress processor, that would be g

[15] RFR: 8245241: Incorrect locale provider preference is not logged

Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8245241 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8245241/webrev.00/ Incorrect user-provided provider preference is supposed to be logged. However it is not so because

[15] RFR: 8239480: Support for CLDR version 37

Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8239480 The proposed changeset, and the release note draft are located at: https://cr.openjdk.java.net/~naoto/8239480/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8243581 This updates the CLDR

Re: [15] RFR: 8245241: Incorrect locale provider preference is not logged

Hi Joe, thanks for the review. On 5/20/20 4:10 PM, Joe Wang wrote: Hi Naoto, I don't seem to see the DateFormat class or the getDateInstance methods specify how errors may be handled (or logged). Is that stated somewhere else? Description of the system property "java.locale.providers" is in

Re: [15] RFR: 8239480: Support for CLDR version 37

Hi Joe, thanks again! The license file "unicode-license.txt" is actually included in the version 37 artifact: https://github.com/unicode-org/cldr/blob/release-37/unicode-license.txt The changeset uses the file as is, so I believe it is OK. Naoto On 5/20/20 6:51 PM, Joe Wang wrote: Looks go

Re: [15] RFR: 8245241: Incorrect locale provider preference is not logged

Hi Daniel, Thank you for your review! On 5/21/20 3:42 AM, Daniel Fuchs wrote: Hi Naoto, Logging uses: ZonedDateTime zdt = ZonedDateTime.ofInstant(     record.getInstant(), ZoneId.systemDefault()); and then String.format("%1$tb %1$td, %1$tY %1$tl:%1$tM:%1$tS %1$Tp %2$s%n%4$s: %

Re: 8239013: java.util.logging.Logger catalog cache keeps strong references to ResourceBundles

Looks good to me. Naoto On 5/22/20 2:33 AM, Daniel Fuchs wrote: Hi, Please find below a fix for: 8239013: java.util.logging.Logger catalog cache keeps strong references to ResourceBundles https://bugs.openjdk.java.net/browse/JDK-8239013 webrev: http://cr.openjdk.java.net/~dfuchs/we

Re: [PATCH] Typo in java.util.regex.Pattern

Just noticed that the release year of the 3rd ed. should also be changed to 2009 in the next line. Naoto On 5/30/20 4:02 AM, Lance Andersen wrote: I can sponsor this for you Best Lance -- Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network

Re: [PATCH] Typo in java.util.regex.Pattern

Thanks, Pavel. I think the year stays 2006 then. Sorry, Lance, for the false alarm. Naoto On 5/30/20 2:34 PM, Pavel Rappo wrote: I'm holding a copy in my hands right now. I can see "ISBN: 978-0-596-52812-6" written on the back of the book. Searching that number in the database yields the foll

Re: [PATCH] Typo in java.util.regex.Pattern

Looks good! Naoto On 5/30/20 3:40 PM, Lance Andersen wrote: All good Naoto Here is the revised diff — $ hg diff src/java.base/share/classes/java/util/regex/Pattern.java *diff -r 968b57610c0f src/java.base/share/classes/java/util/regex/Pattern.java* *--- a/src/java.base/share/classes/java

[15] RFR: 8246261: TCKLocalTime.java failed due to "AssertionError: expected [18:14:22] but found [18:14:23]"

Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8246261 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8246261/webrev.00/ The test case compares two LocalTime objects, created with LocalTime.now(Clock/ZoneId). So inheren

Re: [15] RFR: 8246261: TCKLocalTime.java failed due to "AssertionError: expected [18:14:22] but found [18:14:23]"

Hi Joe, In fact, this bug was possibly revealed by the fix to 8242504, where the system clock precision is now nanoseconds. Before that, it used to be millisecond precision, so the first try for the exact match succeeded for most of the cases. Even with the nano precision fix, most of the cas

Re: [15] RFR: 8246261: TCKLocalTime.java failed due to "AssertionError: expected [18:14:22] but found [18:14:23]"

Updated webrev: http://cr.openjdk.java.net/~naoto/8246261/webrev.01/ Naoto On 6/1/20 6:30 PM, [email protected] wrote: Hi Joe, In fact, this bug was possibly revealed by the fix to 8242504, where the system clock precision is now nanoseconds. Before that, it used to be millisecond precis

[15] RFR: 8246662: Test java/time/test/java/time/format/TestUnicodeExtension.java failed on japanese locale.

Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8246662 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8246662/webrev.00/ This is a regression caused by the fix to JDK-8244245, where the locale related fields in the form

Re: [15] RFR: 8246662: Test java/time/test/java/time/format/TestUnicodeExtension.java failed on japanese locale.

Hi Roger, Thanks for the review. On 6/9/20 8:52 AM, Roger Riggs wrote: Hi Naoto, Since the default locale is being changed even briefly, the test should be run in /othervm. Add:  @run testng/othervm ... All java/time/test/java/time/format tests are run in othervm mode by default, defined

Re: [15] RFR: 8246662: Test java/time/test/java/time/format/TestUnicodeExtension.java failed on japanese locale.

Thanks! On 6/9/20 11:48 AM, Roger Riggs wrote: Hi Naoto, Looks fine. fyi,  Objects.equals(a,b) can replace DateTimeFormatter: 1510-1511. Right. I will replace it before the push. Naoto Thanks, Roger On 6/9/20 1:01 PM, [email protected] wrote: Hi Roger, Thanks for the review. On 6

Re: [15] RFR: 8246662: Test java/time/test/java/time/format/TestUnicodeExtension.java failed on japanese locale.

Hi Joe, On 6/9/20 11:54 AM, Joe Wang wrote: Looks fine to me for the implementation of the method. Thank you for the review! It's interesting to observe that the with methods, as demonstrated in the test, always create a new DateTimeFormatter instance. The Javadoc also said so, e.g. "

RFR: 8246721: java/util/Locale/LocaleProvidersRun.java failed on Windows platforms.

Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8246721 The proposed changeset is located at: http://cr.openjdk.java.net/~naoto/8246721/webrev.00/ The test has been modified to deal with double quotes in string arguments consistently across platf

RFR: 8245448: Remove minimum 4 digit requirement from Year.parse()

Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8245448 The proposed changeset and its CSR are located at: https://cr.openjdk.java.net/~naoto/8245448/webrev.00/ https://bugs.openjdk.java.net/browse/JDK-8246623 Naoto

Re: RFR: 8247706: Unintentional use of new Date(year...) with absolute year

Thanks, Martin. +1 When you do the push, please update the copyright year to 2020. Naoto On 6/17/20 12:30 PM, Roger Riggs wrote: Hi Martin, Looks fine, I'm embarrassed by the one in the java.time tests, we should have known better. Thanks, Roger On 6/17/20 3:22 PM, Martin Buchholz wrote

RFR 8248184: AMPM_OF_DAY doc fix in ChronoField

Hi, Please review this small doc fix for the following issue: https://bugs.openjdk.java.net/browse/JDK-8248184 The proposed patch is here: --- old/src/java.base/share/classes/java/time/temporal/ChronoField.java 2020-06-23 14:43:43.0 -0700 +++ new/src/java.base/share/classes/java/time/

Re: RFR 8248184: AMPM_OF_DAY doc fix in ChronoField

Thanks, Brian and Lance. I forgot to update the copyright year. Pushed the changeset with the year update. Naoto On 6/23/20 3:09 PM, Lance Andersen wrote: +1 Best Lance -- Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Netw

Re: RFR 8248412: test/jdk/java/sql/testng/test/sql/DriverManagerPermissionsTests.java can fail

Hi Lance, The change looks good to me. Naoto On 6/26/20 8:25 AM, Lance Andersen wrote: Hi all, Please review this patch for https://bugs.openjdk.java.net/browse/JDK-8248412 where DriverManagerPermissionsTests.java can occasionally fail in a

RFR: 8248695: HostLocaleProviderAdapterImpl provides invalid date-only

Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8248695 The proposed changeset is located at: http://cr.openjdk.java.net/~naoto/8248695/webrev.00/ There were two causes that resulted in throwing exceptions. One was that the Host adapter for Windo

Re: RFR: 8249086: JDK 15 L10n resource file update - msg drop 10

Hi Leo, Looks good to me. Naoto On 7/8/20 10:58 AM, [email protected] wrote: Hi, Pls review the l10n resource files update for JDK 15 msg drop 10. Bug: https://bugs.openjdk.java.net/browse/JDK-8249086 Webrev: http://cr.openjdk.java.net/~ljiang/8249086/read/ Thanks, Leo

Re: RFR: 8248695: HostLocaleProviderAdapterImpl provides invalid date-only

Ping. On 7/7/20 3:55 PM, [email protected] wrote: Hello, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8248695 The proposed changeset is located at: http://cr.openjdk.java.net/~naoto/8248695/webrev.00/ There were two causes that resulted in throw

Re: RFR: 8248695: HostLocaleProviderAdapterImpl provides invalid date-only

Hi Joe, Thank you for your review. On 7/13/20 3:55 PM, Joe Wang wrote: Hi Naoto, Would it make sense to provide an additional test using the public APIs similar to the one provided in the bug report? I'm sure yours is correct and covers more cases than the original, but it would be nice to h

Re: RFR: 8248695: HostLocaleProviderAdapterImpl provides invalid date-only

Hi Joe, On 7/13/20 7:28 PM, Joe Wang wrote: On 7/13/2020 7:01 PM, [email protected] wrote: Hi Joe, Thank you for your review. On 7/13/20 3:55 PM, Joe Wang wrote: Hi Naoto, Would it make sense to provide an additional test using the public APIs similar to the one provided in the bug r

RFR: 8248655: Support supplementary characters in String case insensitive operations

Hello, Please review the fix to the following issues: https://bugs.openjdk.java.net/browse/JDK-8248655 https://bugs.openjdk.java.net/browse/JDK-8248434 The proposed changeset and its CSR are located at: https://cr.openjdk.java.net/~naoto/8248655.8248434/webrev.00/ https://bugs.openjdk.java.net

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

Thank you, Jim, for the quick review! On 7/15/20 9:26 AM, Jim Laskey wrote: I think I'm good with this. +1 Asides: 325 int cp1 = (int)getChar(value, k1); 326 int cp2 = (int)getChar(other, k2); I would be tempted to short cut by exiting when not equal, but I think w

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

Hi Roger, Thank you for your review and suggestions. On 7/15/20 10:56 AM, Roger Riggs wrote: Hi Naoto, Given the extra tests in the body of the loop, I think its worth finding or creating a JMH test for this and checking the performance. Good point. With performance in mind, I would try

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

Hi Joe, Thank you for your review. On 7/15/20 10:57 AM, Joe Wang wrote: Hi Naoto, In StringUTF16.java, if one is isHighSurrogate and the other not, you may quickly return without going through the rest of the process, probably not significant as cp1 and cp2 and/or u1 and u2 won't be equal a

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

Hi, Based on the suggestions, I modified the fix as follows: https://cr.openjdk.java.net/~naoto/8248655.8248434/webrev.01/ Changes from the initial revision are: - Shared the implementation between compareToCI() and regionMatchesCI() - Enabled immediate short cut if two code points match. - Cr

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

Hi Mark, Thank you for your comments. On 7/17/20 8:03 PM, Mark Davis ☕ wrote: One option is to have a fast path that uses char functions, up to the point where you hit a high surrogate, then drop into the slower codepoint path. That saves time for the high-runner cases. On the other hand, if

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

Hi Joe, Thank you for your comments. On 7/20/20 11:20 AM, Joe Wang wrote: Hi Naoto, StringUTF16: line 384 - 388 seem unnecessary since you'd only get there if 389:isHighSurrogate is not true. Good point. But more importantly, StringUTF16 has existing method "codePointAt" you may want to c

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

Small correction in the updated part: http://cr.openjdk.java.net/~naoto/8248655.8248434/webrev.04/ Naoto On 7/20/20 2:39 PM, [email protected] wrote: Hi Joe, Thank you for your comments. On 7/20/20 11:20 AM, Joe Wang wrote: Hi Naoto, StringUTF16: line 384 - 388 seem unnecessary since y

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

Hi Joe, On 7/20/20 7:14 PM, Joe Wang wrote: Hi Naoto, "Unless it showed 100% slower", wow, your tolerance is quite high :-). On the other hand, I do agree it's unlikely to show in specJVM (that's a speculation though). I am not saying 100% slowing is permissible :-) That's an example of de

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

Thank you, Roger. On 7/21/20 7:50 AM, Roger Riggs wrote: Hi Naoto, StringUTF16.java: 343, 348: The masking and comparisons seem awkward. I'd suggest just negating the value and testing for < 0 to flag the less usual case. If you continue with the flag bit, define your own static final constan

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

Thank you, Joe. I got it now. Will try out and benchmark. Naoto On 7/21/20 10:05 AM, Joe Wang wrote: On 7/20/2020 8:58 PM, [email protected] wrote: The short-cut worked well. There's maybe a further optimization we could do to rid us of the performance concern (or the need to run additi

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

Hi Brent, On 7/21/20 2:56 PM, Brent Christian wrote: Hi, Naoto I have a few comments: src/java.base/share/classes/java/lang/StringUTF16.java 379 private static int getSupplementaryCodePoint(byte[] ba, int cp, int index, int start, int end) I think it would be worth a small addition to

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

Hi, I revised the fix again, based on further suggestions: https://cr.openjdk.java.net/~naoto/8248655.8248434/webrev.05/ Changes from v.04 are (all in StringUTF16.java): - The short cut now does case insensitive comparison that makes the fix closer to the previous implementation (for BMP char

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

Hi Joe, Thank you for the consecutive reviews! On 7/22/20 1:20 PM, Joe Wang wrote: Hi Naoto, The change looks good to me. "supLower" is indeed super slow :-) The only minor comment I have is that the compareCodePointCI method performs toUpperCase unconditionally. That's not a problem for the

Re: RFR: 8248655: Support supplementary characters in String case insensitive operations

Thanks Roger, Ah, I just saw your email just after I sent mine! On 7/22/20 1:38 PM, Roger Riggs wrote: Hi Naoto, Looks fine. (with Joe's suggestion) On 7/22/20 4:20 PM, Joe Wang wrote: Hi Naoto, The change looks good to me. "supLower" is indeed super slow :-) The only minor comment I have

RFR: 8247546: Pattern matching does not skip correctly over supplementary characters

Hi, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8247546 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8247546/webrev.00/ On compiling the regex pattern, the start node is chosen based on the pattern, i.e, whether it involv

Re: RFR: 8247546: Pattern matching does not skip correctly over supplementary characters

On 7/27/20 11:51 AM, [email protected] wrote: Apart from the original issue, there was a bug in Range() method (Pattern.java:5795), so it is fixed along. Created a test case for this: --- a/test/jdk/java/util/regex/SupplementaryTestCases.txt +++ b/test/jdk/java/util/regex/SupplementaryTest

Re: RFR: 8247546: Pattern matching does not skip correctly over supplementary characters

Hi Joe, Thank you for the review! On 7/27/20 6:04 PM, Joe Wang wrote: Hi Naoto, Looks good to me, using the correct supplementary-aware implementation is an improvement for the impl to handle the situation. A note (probably a workaround) for the user's case, to replace invalid surrogate ch

RFR 8233048: WeekFields.ISO is not a singleton

Hi, Please review the fix to the following issue: https://bugs.openjdk.java.net/browse/JDK-8233048 The proposed chageset is located at: https://cr.openjdk.java.net/~naoto/8233048/webrev.00/ java.time.temporal.WeekFields.ISO is instantiated using the constructor, which is a different object r

Re: Language locales have different calendars than country locales in 9+

Hi Bernd, As you pointed out, the change you see here is the result of this change in JDK9: https://bugs.openjdk.java.net/browse/JDK-8008577 where the default locale provider was switched to CLDR. Although we don't describe those behavior changes in the spec (as it is regarded as l10n chang

Re: Language locales have different calendars than country locales in 9+

On 7/31/20 3:08 PM, Bernd Eckenfels wrote: Hello, Is there a good description what calendar is actually used for a language-only locale? Is there a typical association of calendars with locales or is it a global default? If the unicode data does not have such a value, would it be better to fa

Re: [16] 8251017: java/io/File/GetXSpace.java fails on UNIX

Looks good, Brian. Naoto On 8/4/20 9:01 AM, Brian Burkhalter wrote: Add @requires tag for [1]. This patch is layered on top of that proposed in [2]. --- a/test/jdk/java/io/File/GetXSpace.java +++ b/test/jdk/java/io/File/GetXSpace.java @@ -24,6 +24,7 @@ /** * @test * @bug 4057701 628671

Re: RFR [16/java.xml] 8246816: XMLGregorianCalendar.hashCode() produces far too many identical hashes

Hi Joe, For the consistency with equals(), just wondering if the sub-second element should be obtained with getFractionalSecond() instead of getMillisecond(), as the equals() subsequently calls it in internalCompare() method. Also should it call getEonAndYear() appropriately for the year? N

Re: RFR [16]: 8250665 : Wrong translation for the month of May in ar_JO, ar_LB and ar_SY

Looks good to me. Naoto On 8/6/20 9:47 AM, [email protected] wrote: Hi, Pls review the fix for month names in COMPAT provider data for ar_JO/LB/SY. Customer reported some month names are incorrect in JDK8 code according to latest CLDR definition. Bug: https://bugs.openjdk.java.net/browse

Re: RFR [16/java.xml] 8246816: XMLGregorianCalendar.hashCode() produces far too many identical hashes

Hi Joe, thank you for looking it into further. Yes, I agree the chances of collision is very rare, as sub-millisecond difference in two objects. So fine with the version 01. Naoto On 8/6/20 12:18 PM, Joe Wang wrote: Thanks Naoto, Roger for the review! For Naoto's concern about the equals me

Re: 8251272: Typo in java.util.Formatter: "Numberic" should be "Numeric"

+1 Good catch! Naoto On 8/6/20 4:12 PM, Brent Christian wrote: Looks good! -Brent On 8/6/20 3:54 PM, Brian Burkhalter wrote: For [1], please review this trivial fix [2]. Thanks, Brian [1] https://bugs.openjdk.java.net/browse/JDK-8251272 [2] diff --- a/src/java.base/share/classes/java/uti

Re: RFR 8251202: Add missing javadoc comments

Looks good, Lance. nit: a couple of comments (line 128,133) don't end with periods. Naoto On 8/6/20 3:32 PM, Lance Andersen wrote: Hi all, Please review the webrev for: https://bugs.openjdk.java.net/browse/JDK-8251202 , which adds missing jav

Re: [Ping] Re: 8249703: test/jdk/java/io/File/GetXSpace.java fails on macos

Looks fine, Brian. I might add the condition if the block number is odd. Naoto On 8/7/20 8:33 AM, Brian Burkhalter wrote: On Jul 31, 2020, at 10:42 AM, Brian Burkhalter wrote: On macOS, the number of 1024 byte blocks appears to be incorrectly calculated by 'df' using integer division by

Re: RFR [16/java.xml] 8246816: XMLGregorianCalendar.hashCode() produces far too many identical hashes

Hi Joe, If the new XMLGregorianCalendarImpl.hashCode() just calls super.hashCode(), could it be removed entirely? Also XMLGregorianCalendarImpl.equals() seems to do the same thing as its parent. Could it also be removed? Naoto On 8/7/20 10:16 AM, Joe Wang wrote: Naoto, Roger, Sorry have t

Re: [Ping] Re: 8249703: test/jdk/java/io/File/GetXSpace.java fails on macos

Looks good. Thank you for the change. Naoto On 8/7/20 10:57 AM, Brian Burkhalter wrote: Hi Naoto, Good point. Here’s an updated version: http://cr.openjdk.java.net/~bpb/8249703/webrev.01/. The number of blocks is not available via a dedicated API call and so has to be calculated. Thanks, B

Re: RFR [16/java.xml] 8246816: XMLGregorianCalendar.hashCode() produces far too many identical hashes

Looks good. Thank you for the change. Naoto On 8/7/20 12:13 PM, Joe Wang wrote: Hi Naoto, Why not :-). I kept it to go with equals(). Removing both now. Added a reference comparison as did in the Impl class. http://cr.openjdk.java.net/~joehw/jdk16/8246816/webrev_04/ Joe On 8/7/20 10:50 AM

Re: [Ping] Re: 8181919: Refactor test/java/io/File/GetXSpace.sh to java test

Looks good, Brian. I'd add the bug id in GetXSpace.java's @bug tag. Naoto On 8/13/20 8:40 AM, Brian Burkhalter wrote: Thanks! On Aug 11, 2020, at 11:32 AM, Brian Burkhalter wrote: A revised version [A] is available pursuant to the changes made in the course of the review [B] of the fix fo

RFR: 8251499: no-placeholder compact number patterns throw IllegalArgumentException

Hello, Please review the fix for the following issue: https://bugs.openjdk.java.net/browse/JDK-8251499 The proposed changeset is located at: https://cr.openjdk.java.net/~naoto/8251499/webrev.00/ The current implementation of CompactNumberFormat assumes that there is always the number placeho

Re: RFR: 8251499: no-placeholder compact number patterns throw IllegalArgumentException

Hi Joe, It turned out that the previous fix did not address plural format cases. That means that just making the divisor negative to indicate non-placeholder cannot distinguish multiple plural cases with the same divisor. Instead, I created a list of placeholders (minimum digits) for each ind

Re: RFR: 8251499: no-placeholder compact number patterns throw IllegalArgumentException

Hi Roger, Thank you for your comment. I added a brief comment in the issue on how the implementation behaves in the problem case. Naoto On 8/18/20 8:19 AM, Roger Riggs wrote: Hi Naoto, I think the issue would benefit from a comment describing the solution. Its not clear how the code address

Re: RFR: 8251499: no-placeholder compact number patterns throw IllegalArgumentException

Hi Joe, Thank you for your comment. I consolidated those duplicated pieces into one piece. Did not make it a private method, though, as it would need to return two results from the method (cnfMultiplier and whether to return immediately for no-placeholder cases). https://cr.openjdk.java.net/

Re: RFR: 8251499: no-placeholder compact number patterns throw IllegalArgumentException

Hi Roger, Thank you for your comments. I've addressed your suggestions and created a new webrev below: https://cr.openjdk.java.net/~naoto/8251499/webrev.03/ Naoto On 8/19/20 8:33 AM, Roger Riggs wrote: Hi Naoto, CompactNumberFormat.java: 269: The field "placeHolders" should be named consi

Re: RFR 8251203: Fix "no comment" warnings in java.base/java.lang and java/io

Hi Roger, Changes look good to me. One question, do we need to update the copyright year for this change? Naoto On 8/21/20 8:09 AM, Roger Riggs wrote: Please review the addition of comments to classes and fields to resolve javadoc "no comment" warnings in the java.lang and java.io packages.

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

+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

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

Still looks good. Naoto On 8/24/20 2: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, [email protected] wrote: +1

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

+1 Naoto On 8/25/20 11:47 AM, Joe Wang wrote: Cc-ing [email protected] (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 th

Re: [NEW BUG] NumberFormat.parse fails in some scenarios

Hi Christoph, The behavior you are observing is what is expected with the CLDR provider, as the currency format is exactly the one that CLDR has for the de locale: https://unicode-org.github.io/cldr-staging/charts/37/by_type/numbers.number_formatting_patterns.html#53687a25c19b6481 I'd recomm

RFR: 8252552: DecimalFormat javadoc contains HTML tags in example code

Hello, Please review this simple javadoc fix to: https://bugs.openjdk.java.net/browse/JDK-8252552 The proposed diff is inserted here: --- --- old/src/java.base/share/classes/java/text/DecimalFormat.java 2020-08-31 09:07:22.0 -0700 +++ new/src/java.base/share/classes/java/text/DecimalF

Re: RFR: 8252552: DecimalFormat javadoc contains HTML tags in example code

, 2020, at 12:22 PM, Naoto Sato wrote: Hello, Please review this simple javadoc fix to: https://bugs.openjdk.java.net/browse/JDK-8252552 The proposed diff is inserted here: --- --- old/src/java.base/share/classes/java/text/DecimalFormat.java 2020-08-31 09:07:22.0 -0700 +++ new/src

Re: RFR [16/java.xml] 8252354 : Properties :: storeToXML method does not throw ClassCastException when supplied non strings.

Hi Joe, Looks good to me. The test can declare @Test(expected = CCE.class) to eliminate storeToXML(), but I am ok with either way. Naoto On 9/1/20 2:12 PM, Joe Wang wrote: Please review a fix to report CCE when keys or values are not Strings as specified. JBS: https://bugs.openjdk.java.net

Re: RFR: 8157729: examples in LinkedHashMap and LinkedHashSet class doc use raw types

On Tue, 8 Sep 2020 22:47:40 GMT, Joe Darcy wrote: >> Add some generics and wrap in `{@code}` to protect angle brackets. > > Marked as reviewed by darcy (Reviewer). Looks good with the copyright year update. - PR: https://git.openjdk.java.net/jdk/pull/87

Re: RFR: 8157729: examples in LinkedHashMap and LinkedHashSet class doc use raw types

On Tue, 8 Sep 2020 22:32:47 GMT, Stuart Marks wrote: > Add some generics and wrap in `{@code}` to protect angle brackets. Marked as reviewed by naoto (Reviewer). - PR: https://git.openjdk.java.net/jdk/pull/87

Re: RFR: 8157729: examples in LinkedHashMap and LinkedHashSet class doc use raw types [v2]

On Tue, 8 Sep 2020 23:03:25 GMT, Stuart Marks wrote: >> Add some generics and wrap in `{@code}` to protect angle brackets. > > Stuart Marks has updated the pull request incrementally with one additional > commit since the last revision: > > Update copyright years. All good. - M

Re: RFR: 8251495: Clarify DOM documentation

On Wed, 9 Sep 2020 22:56:14 GMT, Joe Wang wrote: > Revert changes made by JDK-8249643, removing the implNote. Looks good. - Marked as reviewed by naoto (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/100

  1   2   3   4   5   6   7   8   9   10   >