RFR: 8244199: [TestBug]: Update sun/management/jmxremote/bootstrap tests

[Thejasvi Voniadka]

Hi,

May someone please sponsor this patch?

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

Description:
The test 
"test/jdk/sun/management/jmxremote/bootstrap/RmiSslBootstrapTest.sh" fails 
intermittently on a lower release. While investigating the cause for the 
failure, I felt the diagnostic messaging built into the test was not adequate. 
Some of the issues observed:
1. If the test throws a RuntimeException, the full stack trace is not 
displayed.
2. The test has several logging statements. However, the logging 
environment is not properly initialized to allow levels such as FINE and FINER.
3. The logging sequence could be improved: some messages end up at 
stdout, and others at stderr, which makes it harder to follow the output.

The patch is to improve the test in these lines.

Webrev: http://cr.openjdk.java.net/~tvoniadka/reviews/8244199/webrev.full.00

I have also used this opportunity to clean-up the test code as a whole, in 
lines of removal of redundant logic, formatting, coding guidelines, etc If 
the above patch looks too confusing and is hard to follow, here is a simpler 
version that shows only the core logic changes (I intend to submit the full 
patch to be pushed though): 
http://cr.openjdk.java.net/~tvoniadka/reviews/8244199/webrev.simple.00

The patch has been tested on mach5, and all jmxremote tests passed.

Re: RFR: JDK-8244618 - WinUpgradeUUIDTest.java fails after JDK-8236518

[Jesper Wilhelmsson]

Thank you!
Pushed.
/Jesper

> On 8 May 2020, at 02:41, David Holmes  wrote:
> 
> Ship it!
> 
> (though should have gone to core-libs-dev cc'd)
> 
> Thanks,
> David
> 
> On 8/05/2020 10:31 am, Jesper Wilhelmsson wrote:
>> Hi.
>> Please review this patch to problemlist WinUpgradeUUIDTest.java that is 
>> currently failing in tier 2.  Another change is in review to remove the 
>> failure but it doesn't seem likely that it will be pushed today, so in the 
>> interest of keeping tier 2 green during the non-US timezones Friday I 
>> propose to problemlist the test for now.
>> Thanks,
>> /Jesper
>> diff --git a/test/jdk/ProblemList.txt b/test/jdk/ProblemList.txt
>> --- a/test/jdk/ProblemList.txt
>> +++ b/test/jdk/ProblemList.txt
>> @@ -903,6 +903,7 @@
>>  # core_tools
>>  tools/jlink/JLinkReproducibleTest.java  8217166 
>> windows-all
>> +tools/jpackage/windows/WinUpgradeUUIDTest.java#id1  8244620 
>> windows-all
>>  

Re: RFR: JDK-8244618 - WinUpgradeUUIDTest.java fails after JDK-8236518

[David Holmes]

Ship it!

(though should have gone to core-libs-dev cc'd)

Thanks,
David

On 8/05/2020 10:31 am, Jesper Wilhelmsson wrote:

Hi.

Please review this patch to problemlist WinUpgradeUUIDTest.java that is 
currently failing in tier 2.  Another change is in review to remove the failure 
but it doesn't seem likely that it will be pushed today, so in the interest of 
keeping tier 2 green during the non-US timezones Friday I propose to 
problemlist the test for now.

Thanks,
/Jesper


diff --git a/test/jdk/ProblemList.txt b/test/jdk/ProblemList.txt
--- a/test/jdk/ProblemList.txt
+++ b/test/jdk/ProblemList.txt
@@ -903,6 +903,7 @@
  # core_tools

  tools/jlink/JLinkReproducibleTest.java  8217166 
windows-all
+tools/jpackage/windows/WinUpgradeUUIDTest.java#id1  8244620 
windows-all

  

[15] RFR: 8239383: Support for Unicode 13.0

[naoto . sato]

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 support Unicode Character 
Database 13.0, as well as incorporating ICU4J version 67.1, which 
corresponds to Unicode 13.0, for upgrading java.text.BiDi/Normalizer 
support. Grapheme support in java.util.regex package is also upgraded to 
13.0 level.


Naoto

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

[Mikael Vidstedt]

> On May 7, 2020, at 7:52 AM, Kumar Srinivasan  wrote:
> 
> Hi Mikael,
> 
> I may have created solinux when the macosx port was merged and in an effort 
> to reduce the CPP conditionals.
> solinux = solaris + linux  ie. Vanilla unix code vs Darwin code IIRC has 
> Objective-C, MacOSX specific thread initialization etc.

Thank you for the background - it all makes sense now! :)

> I looked over the launcher related code looks ok to me.
> 
> I did notice the \n endings for the messages that Brent pointed out.

Thank you for the review! The line ending should be fixed in webrev.01.

Cheers,
Mikael

> 
>> On May 6, 2020, at 9:43 PM, Mikael Vidstedt > > wrote:
>> 
>> 
>> I have always wondered what “solinux” is supposed to mean - though not 
>> enough to actually ask anybody :)
>> 
>> I’ll file a follow-up enhancement to cover renaming the files.
>> 
>> Thank you for the review!
>> 
>> Cheers,
>> Mikael
>> 
>>> On May 4, 2020, at 7:59 AM, Roger Riggs >> > wrote:
>>> 
>>> Hi Michael,
>>> 
>>> Looks good.
>>> 
>>> Maybe just a future cleanup to rename files, since the "...so..." is 
>>> refering to solaris.
>>> 
>>> src/java.base/unix/native/libjli/java_md_solinux.h
>>> src/java.base/unix/native/libjli/java_md_solinux.h
>>> 
>>> Regards, Roger
>>> 
>>> 
>>> On 5/4/20 4:49 AM, Alan Bateman wrote:
 On 04/05/2020 06:12, Mikael Vidstedt wrote:
> Please review this change which implements part of JEP 381:
> 
> JBS: 
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.openjdk.java.net%2Fbrowse%2FJDK-8244224data=02%7C01%7Ckusrinivasan%40vmware.com%7Ce48bd44f0c484e6fd85608d7f243f1ff%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637244245954061434sdata=W5VvD1owIGoUcbcRkcwixXGPkFLjHUFof2v6cxMqchk%3Dreserved=0
>  
> 
> webrev: 
> https://nam04.safelinks.protection.outlook.com/?url=http:%2F%2Fcr.openjdk.java.net%2F~mikael%2Fwebrevs%2F8244224%2Fwebrev.00%2Fcorelibs%2Fopen%2Fwebrev%2Fdata=02%7C01%7Ckusrinivasan%40vmware.com%7Ce48bd44f0c484e6fd85608d7f243f1ff%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637244245954061434sdata=tQZIjuF5cIPs%2FNqTRtY2WtmwAgwa0iv205wQ9vk0vOQ%3Dreserved=0
>  
> 
> JEP: 
> https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.openjdk.java.net%2Fbrowse%2FJDK-8241787data=02%7C01%7Ckusrinivasan%40vmware.com%7Ce48bd44f0c484e6fd85608d7f243f1ff%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637244245954061434sdata=rDvBE%2BJ2F1IIFC9fbA92rs0KZNgYPg0JZM2ynqp7mcs%3Dreserved=0
>  
> 
> 
> 
> Note: When reviewing this, please be aware that this exercise was 
> *extremely* mind-numbing, so I appreciate your help reviewing all the 
> individual changes carefully. You may want to get that coffee cup filled 
> up (or whatever keeps you awake)!
> 
 I took a pass over the changes. I agree its a bit tedious. I'm sure there 
 will be a few follow up issues as there are opportunities for cleanup in 
 several areas. Just a few comments/questions from a first pass.
 
 ExtendedSocketOption.SO_FLOW_SLA is the Solaris specific socket option 
 that was terminally deprecated in 14. The patch removes the implementation 
 but leave the API (SO_FLOW_SA and jdk.net.SocketFlow). Do you want a 
 someone to take a follow-on issue to remove the API?
 
 ResolverConfigurationImpl.localDomain0 can be removed.
 
 The comment on mcast_join_leave in PlainDatagramSocketImpl.c has a 
 residual reference to Solaris.
 
 JISAutoDetect - might be simpler to just initialize EUCJPName to "EUC_JP".
 
 Socket.setTrafficClass(int) swallows exceptions to workaround strange 
 behaviour on Solaris. Tracked as JDK-8221487 so okay to leave it to that 
 issue if you want. There is also cruft in the old plain SocketImpl that to 
 work around eagerness to report "connection reset errors - I think we 
 should just leave 

RFR: 8244624: Improve handling of JarFile META-INF resources

[Claes Redestad]

Hi,

currently during ZipFile creation, we create an int[] array of pointers
to META-INF entries. These are then retrieved from three different
places in JarFile.

However, JarFile is only interested in some combination a few things:
the existence of and name of the META-INF/MANIFEST file, the existence
of and the names of various signature related files, i.e., files in
META-INF that have a suffix such as .EC, .SF, .RSA and .DSA

Refactoring the contract between JarFile and ZipFile means we can filter
out such entries that we're not interested when opening the file, and
also remove the need to create the String for each entries unless we
actually want them:

Bug:https://bugs.openjdk.java.net/browse/JDK-8244624
Webrev: http://cr.openjdk.java.net/~redestad/8244624/open.00/

This reduces retained footprint of Jar-/ZipFile by slimming down or
removing the Source.metanames array entirely, and a significant speed-up
in some cases.

In the provided microbenchmark, getManifestFromJarWithManifest and
getManifestFromJarWithSignatureFiles doesn't call
JUZFA.getMetaInfEntryNames but still see small (~5%) speed-up decrease
in allocations.

getManifestFromJarWithNoManifest exercise JUZFA.getMetaInfEntryNames in
the baseline, and now calls JUZFA.getManifestName. Result is a 1.25x
speed-up and 30% reduction in allocations. While unrealistic (most JARs
have a META-INF/MANIFEST.MF), this speed-up will translate to a few
places - such as when loading classes from potentially-signed JAR files.

Testing: tier1-2

Thanks!

/Claes

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

[Mikael Vidstedt]

> On May 6, 2020, at 11:41 PM, Alan Bateman  wrote:
> 
> On 07/05/2020 05:56, Mikael Vidstedt wrote:
>> :
>> 
>> * File follow-up enhancement for the removal of SO_FLOW_SA and 
>> jdk.net.SocketFlow
> I've created JDK-8244582 to track this, we should try to this in the same 
> release as JEP 381.

Thank you!

> 
>> :
>> 
>> 
>> * Get confirmation from Alan that the Socket.setTrafficClass(int) and 
>> SocketImpl stuff is good as-is
>> 
> JDK-8221487 tracks the Socket.setTrafficClass issue. I didn't create an issue 
> to mop up issues in the old SocketImpl as it is not used (at least not by 
> default) since JDK 13. The intention is to completely remove the old 
> SocketImpl implementation, don't know which release yet but it's hardly seems 
> worth spending time doing cleanup on this code. I've no doubt there will be 
> cleanup in many areas of the JDK code after JEP 381 is pushed.

Sounds good, thank you!

Cheers,
Mikael

Re: RFR: JDK-8244620: test failure in WinUpgradeUUIDTest

[Philip Race]

What's the failure look like ? I don't see it in the bug report.

-phil.

On 5/7/20, 1:23 PM, Andy Herrick wrote:

please review fix for issue [1] at [2].

/Andy

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

[2] - http://cr.openjdk.java.net/~herrick/8244620/webrev.01/

Re: RFR: JDK-8244620: test failure in WinUpgradeUUIDTest

[Alexey Semenyuk]

Andy,

Is there a plan to properly fix the failing test instead of just 
disabling it?


- Alexey

On 5/7/2020 4:23 PM, Andy Herrick wrote:

please review fix for issue [1] at [2].

/Andy

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

[2] - http://cr.openjdk.java.net/~herrick/8244620/webrev.01/

RFR: JDK-8244620: test failure in WinUpgradeUUIDTest

[Andy Herrick]

please review fix for issue [1] at [2].

/Andy

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

[2] - http://cr.openjdk.java.net/~herrick/8244620/webrev.01/

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

[Mark Davis ☕️]

> (int)(tokens.countTokens() / 0.75f) + 1
> (tokens.countTokens() * 4 + 2) / 3

if you want A * X / Y, rounded up, you can use (A * X - 1 ) / Y + 1
eg where X/Y = 4/3,

0 => 0
1 => 2
2 => 3
3 => 4
4 => 6
...



Mark


On Tue, May 5, 2020 at 9:48 AM Peter Levart  wrote:

> Hi Naoto,
>
> On 4/30/20 12:18 AM, naoto.s...@oracle.com wrote:
> > 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 known.
>
>
> Well, it has to be calculated 1st (countTokens), but I guess this pays
> off when HashSet (the backing HashMap) does not have to be rehashed then.
>
> The expression you use:
>
>  Math.max((int)(tokens.countTokens() / 0.75f) + 1, 16)
>
> ...has a minimum value of 16. Why is that? 16 is just HashMap's default
> initialCapacity if not specified explicitly. But if you only want to
> store say 1 entry in the map, you can specify 2 as initialCapacity and
> HashMap will happily work for such case without resizing.
>
>
> So you could just use:
>
>  (int)(tokens.countTokens() / 0.75f) + 1
>
> And even this expression is sometimes overshooting the minimal required
> value by 1 (when # of tokens is "exact" multiple of 0.75f, say 6). I
> think the following could be used to optimally pre-size the HashMap with
> default load factor 0.75:
>
>  (tokens.countTokens() * 4 + 2) / 3
>
>
> Regards, Peter
>
> >
> > Naoto
>

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

[Mark Davis ☕️]

I wouldn't worry too much about over 2^29 either.

However, the number of possible valid language tags is much larger than
people think, since any combination of multiple variants can occur). So
even not counting the locale extensions, it is over 2^129 (my rough
calculation).

Mark


On Tue, May 5, 2020 at 5:09 PM  wrote:

> 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 go with the current
> version.
>
> Naoto
>
> >
> > s'marks
> >
> >
> >
> >
> > On 5/5/20 1:26 PM, Peter Levart wrote:
> >> There's more...
> >>
> >>
> >> Guava (and JDK in copy constructor) formula:
> >>
> >>  (int) ((float) expectedSize / 0.75f + 1.0f)
> >>
> >>
> >> is not the same as Naoto's formula:
> >>
> >>  (int) (expectedSize / 0.75f) + 1
> >>
> >>
> >> Notice that Naoto does addition of 1 in integer arithmetic after
> >> conversion to int, while Guava/JDK does in floating point before
> >> conversion to int. If I put Naoto's formula into my test program, no
> >> undercalculations are detected.
> >>
> >>
> >> So while Naoto's formula is sub-optimal for expectedSizes that are
> >> multiples of 3, the Guava/JDK also has a bug.
> >>
> >>
> >> Regards, Peter
> >>
> >>
> >> On 5/5/20 10:01 PM, Peter Levart wrote:
> >>>
> >>>
> >>> On 5/5/20 9:41 PM, Martin Buchholz wrote:
>  Hi Peter,
> 
>  Are you saying guava has a tiny bug?
> >>>
> >>>
> >>> If it was just 1 too much when expected size is a multiple of 3 then
> >>> that would not be a bug, just sub-optimal calculation. And the same
> >>> calculation is performed also in JDK when a copy constructor is
> >>> called for example.
> >>>
> >>>
> >>> But I investigated further and what I found could be considered a
> >>> bug. Sometimes, the following expression:
> >>>
> >>>
> >>> (int) ((float) expectedSize / 0.75f + 1.0f)
> >>>
> >>>
> >>> ...calculates a value that is not enough (due to floating point
> >>> arithmetic and conversion to int) to store the expectedSize elements
> >>> into the HashMap without re-hashing.
> >>>
> >>>
> >>> What HashMap does with initialCapacity parameter is to round it up to
> >>> nearest power of 2:
> >>>
> >>> static int tableSizeFor(int cap) {
> >>> int n = -1 >>> Integer.numberOfLeadingZeros(cap - 1);
> >>> return (n < 0) ? 1 : (n >= MAXIMUM_CAPACITY) ?
> >>> MAXIMUM_CAPACITY : n + 1;
> >>> }
> >>>
> >>> then it uses this as the initial backing table size. From that table
> >>> size it calculates the threshold value:
> >>>
> >>> static int threshold(int cap) {
> >>> float ft = (float) cap * 0.75f;
> >>> return (cap < MAXIMUM_CAPACITY && ft < (float)
> >>> MAXIMUM_CAPACITY ?
> >>> (int) ft : Integer.MAX_VALUE);
> >>> }
> >>>
> >>> ... and uses it as the max. number of elements that a HashMap can
> >>> hold before it is re-hashed. So I did the following test (comparing
> >>> the effectiveness of above formula with alternative
> >>> (expectedSize*4+2)/3 formula):
> >>>
> >>>
> >>> public class HMTest {
> >>> static final int MAXIMUM_CAPACITY = 1 << 30;
> >>>
> >>> static int tableSizeFor(int cap) {
> >>> int n = -1 >>> Integer.numberOfLeadingZeros(cap - 1);
> >>> return (n < 0) ? 1 : (n >= MAXIMUM_CAPACITY) ?
> >>> MAXIMUM_CAPACITY : n + 1;
> >>> }
> >>>
> >>> static int threshold(int cap) {
> >>> float ft = (float) cap * 0.75f;
> >>> return (cap < MAXIMUM_CAPACITY && ft < (float)
> >>> MAXIMUM_CAPACITY ?
> >>> (int) ft : Integer.MAX_VALUE);
> >>> }
> >>>
> >>> public static void main(String[] args) {
> >>> for (int expectedSize = 0; expectedSize < (Integer.MAX_VALUE
> >>> - 2) / 4; expectedSize++) {
> >>> int cap1 = (int) ((float) expectedSize / 0.75f + 1.0f);
> >>> int cap2 = (expectedSize * 4 + 2) / 3;
> >>> int ts1 = tableSizeFor(cap1);
> >>> int ts2 = tableSizeFor(cap2);
> >>> int th1 = threshold(ts1);
> >>> int th2 = threshold(ts2);
> >>>
> >>> if (th1 < expectedSize || th2 < expectedSize) {
> >>> System.out.printf("%d: (%d, %d, %d)%s (%d, %d,
> %d)%s\n",
> >>> expectedSize,
> >>> cap1, ts1, th1, (th1 < expectedSize) ? "!" :
> >>> " ",
> >>> cap2, ts2, th2, (th2 < expectedSize) ? "!" : "
> "
> >>> );
> >>> }
> >>> }
> >>> }
> >>> }
> >>>
> >>>
> >>> And what this prints is the following:
> >>>
> >>>
> >>> 25165825: (33554432, 33554432, 25165824)! (33554434, 67108864,
> 50331648)
> >>> 50331649: (67108864, 67108864, 50331648)! (67108866, 134217728,
> >>> 100663296)
> >>> 50331650: (67108864, 67108864, 50331648)! (67108867, 

Re: os::javaTimeSystemUTC to call nanosecond precision OS API, so Clock.systemUTC() can give nanosecond precision UTC

[Mark Kralj-Taylor]

Hi David,

So the issue is that while glibc 2.12 (in OEL6.4) supports
clock_gettime(CLOCK_REALTIME) it requires a runtime dependency on
librt.so, which is an optional runtime dependency, hence the dynamic
lookup you mention. The complexity of dynamic lookup makes this
enhancement unattractive. But when openjdk updates to require glibc
>=2.17 it would be trivial, because clock_gettime() was moved into the
core libc.so.

With glibc >= 2.17 there would still need to be a call to
clock_gettime() to see if the optional CLOCK_MONOTONIC is supported,
whereas CLOCK_REALTIME is required by the Posix spec and header docs.

---
This leads me to ask a different question:

What is the openjdk process for reviewing and updating its minimum dependencies?
- When are dependencies reviewed?
- What is the mailgroup to look at? (please let me know if there is a
better place to ask this question)

The info I found was last updated for JDK 13:
https://wiki.openjdk.java.net/display/Build/Supported+Build+Platforms

- Glibc 2.12 was released in 2010 and glibc 2.17 in 2012
- The current openjdk LTS release is Java SE 11 in September 2018,
supported through to October 2024 (extended support to September
2026): https://en.wikipedia.org/wiki/Java_version_history).
- Oracle Enterprise Linux 7.0 released in 2014 included glibc 2.17

These dates suggest an update of openjdk's glibc dependency might be due.
But I guess the driving factor is how the timelines of openjdk LTS
release support fits with those of Oracle Enterprise Linux versions.

---
FYI some observations on my experience of building and testing openjdk.

I installed OEL6.4, but found it very tricky to build a `real` openjdk
distribution on a current Ubuntu that would run with all features on
OEL6.4.
- The base openjdk build was easy to use, but it built a JDK with elf
dependencies based on my OS, so of course would not run on OLE6.
- The openjdk make/devkit infrastructure didn't work for me to make an
OEL6 build, after lots of attempts (including running in chroot or
docker isolation). I'm sure it works reliably, provided you have the
correct OS setup. But its not easy for an outsider to replicate that.
- The AdoptOpenJDK Docker build was super easy to use, and I liked
that it used Docker to use a reproducible and fully scripted OS build
environment. https://github.com/AdoptOpenJDK/openjdk-build
- But AdoptOpenJDK Docker build doesn't support building and using an
OEL6 devkit. AdoptOpenJDK build targets RHEL 7.4+:
https://github.com/AdoptOpenJDK/openjdk-build/wiki/%5BWIP%5D-Minimum-OS-levels

openjdk's devkit infra must pre-date the rise of Docker, and fully
reproducible build environments becoming the norm.
Maybe one day there will be time to use Docker or similar to simplify
the openjdk build, and making it easy for anyone to reproduce.

As with using newer glibc features (glibc 2.17 vs 2.12), its likely to
be drastically simpler when openjdk is able to update dependencies to
newer OS versions. Especially if openjdk can be built on its minimum
required per-platform OS version in a (Docker) container.

Mark

On Tue, 14 Apr 2020 at 23:34, David Holmes  wrote:
>
> Hi Mark,
>
> On 15/04/2020 3:09 am, Mark Kralj-Taylor wrote:
> > David, Daniel,
> > What is the oldest (lowest) version of Linux and glibc for openjdk 15?
>
> I'm not clear on that.
>
> > The availability of clock_gettime(CLOCK_REALTIME) on the oldest
> > Linux/glibc supported by openjdk 15 is likely to be the deciding
> > factor on if Hotspot Linux code can call
> > clock_gettime(CLOCK_REALTIME).
> >
> > doc/building.md suggests minimum Linux is Oracle Enterprise Linux 6.4
> > (i.e. RHEL 6.4).
> > Which I think uses glibc 2.12-1.107 (based on
> > https://yum.oracle.com/repo/OracleLinux/OL6/4/base/x86_64/index_src.html).
> > Searching for glibc sources it looks like this supports
> > clock_gettime(CLOCK_REALTIME).
> >
> > The wording of Linux docs suggests that clock_gettime(CLOCK_REALTIME)
> > should be supported if the clock_gettime() API exists. But other clock
> > sources are not mandatory. So CLOCK_REALTIME should be available even
> > if CLOCK_MONOTONIC is not.
> > - See: https://linux.die.net/man/2/clock_gettime.
> > - Also "POSIX.1-2008 marks gettimeofday() as obsolete, recommending
> > the use of clock_gettime(2) instead." from:
> > https://linux.die.net/man/2/gettimeofday
> >
> > Note that Hotspot os_posix.cpp checks for non-error return from
> > clock_gettime(CLOCK_MONOTONIC) to guard setting the _clock_gettime
> > function pointer. Which was why in the patch I called clock_gettime
> > directly for Linux specific os_linux.cpp (a subset of Posix OS-s).
> >
> > Also os_linux.cpp has:
> > #ifndef SUPPORTS_CLOCK_MONOTONIC
> > #error "Build platform doesn't support clock_gettime and related 
> > functionality"
> > #endif
> > Which made me wonder if openjdk might require CLOCK_MONOTONIC - which
> > would mean clock_gettime(CLOCK_REALTIME) is supported.
>
> So we require that the build platform supports 

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

[Joe Wang]

On 5/7/2020 9:03 AM, naoto.s...@oracle.com wrote:

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 Chronology.ofLocale()/DecimalStyle.of() methods indirectly, so I 
believe no doc change is warranted.


Ok, thanks for the explanation.

-Joe



Naoto

On 5/6/20 11:32 PM, Joe Wang wrote:

Hi Naoto,

The Javadoc states:
 If the locale contains the "ca" (calendar), "nu" (numbering 
system), "rg" (region override), and/or "tz" (timezone) Unicode 
extensions, the chronology, numbering system and/or the zone are 
overridden.


If you remove the two statements that check whether the specified 
locale contains "ca" or "nu", would you need to update the Javadoc as 
well?


Best,
Joe

On 5/6/2020 1:44 PM, naoto.s...@oracle.com wrote:

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://bugs.openjdk.java.net/browse/JDK-8243162), and the 
rationale for this fix is discussed there.


Naoto

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

[Roger Riggs]

Looks good, thanks

On 5/7/20 12:06 PM, naoto.s...@oracle.com wrote:

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.java: Please wrap the excessively long lines; 
string concatination will put them together for the test.


Thanks, Roger


On 5/6/20 4:44 PM, naoto.s...@oracle.com wrote:

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://bugs.openjdk.java.net/browse/JDK-8243162), and the 
rationale for this fix is discussed there.


Naoto

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

[Joe Wang]

On 5/7/2020 1:14 AM, Stephen Colebourne wrote:

On Thu, 7 May 2020 at 07:38, Joe Wang  wrote:

The Javadoc states:
  If the locale contains the "ca" (calendar), "nu" (numbering
system), "rg" (region override), and/or "tz" (timezone) Unicode
extensions, the chronology, numbering system and/or the zone are overridden.

If you remove the two statements that check whether the specified locale
contains "ca" or "nu", would you need to update the Javadoc as well?

Those things are checked by the methods Chronology.of(locale) and
DecimalStyle.of(locale), so although indirect, I think the Javadoc is
fine.


I see.  Thanks.

-Joe


Stephen

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

[naoto . sato]

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.java: Please wrap the excessively long lines; 
string concatination will put them together for the test.


Thanks, Roger


On 5/6/20 4:44 PM, naoto.s...@oracle.com wrote:

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://bugs.openjdk.java.net/browse/JDK-8243162), and the rationale 
for this fix is discussed there.


Naoto

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

[naoto . sato]

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 
Chronology.ofLocale()/DecimalStyle.of() methods indirectly, so I believe 
no doc change is warranted.


Naoto

On 5/6/20 11:32 PM, Joe Wang wrote:

Hi Naoto,

The Javadoc states:
     If the locale contains the "ca" (calendar), "nu" (numbering 
system), "rg" (region override), and/or "tz" (timezone) Unicode 
extensions, the chronology, numbering system and/or the zone are 
overridden.


If you remove the two statements that check whether the specified locale 
contains "ca" or "nu", would you need to update the Javadoc as well?


Best,
Joe

On 5/6/2020 1:44 PM, naoto.s...@oracle.com wrote:

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://bugs.openjdk.java.net/browse/JDK-8243162), and the rationale 
for this fix is discussed there.


Naoto

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

[Johannes Kuhn]

I just saw a question on StackOverflow [1] which reports to have the 
same issue as in JDK-8226810 [2] - with the same stack trace.


While I could not reproduce the issue, I tried to understand which code 
path could lead to that outcome.

While doing so I did come across getEncodingInternal in java_props_md.c [3]

The path when GetLocaleInfo in line 72 fails just sets the codepage to 1252.
The following switch doesn't have a branch for codepage 1252, so the 
default branch is used.


The first two elements of the char array are set to "Cp", while the rest 
of that char array is still uninitialized.

This (partially) uninitialized array is then returned.

This seems wrong.
I would therefore either change codepage to 0 (resulting in a default 
encoding of UTF-8) or also copy the "Cp1252" to the char buffer.



(More when I follow this code path):
In InitializeEncoding [4] this invalid encoding is then set to 
jnuEncoding, and fastEncoding to NO_FAST_ENCODING.
A call to JNU_NewStringPlatform will then result in a call to 
newSizedStringJava [5], where jnuEncodingSupported is called.
This calls java.nio.charset.Charset.isSupported(jnuEncoding) and caches 
the result - I assume it returns false. Or throws.


This means new String(byte[]) is called.

Here comes the puzzler: java.lang.StringCoding.decode(byte[],int,int) 
calls Charset.getDefaultCharset() as first action.
- and does not fall back to iso-8859-1 as the comment in 
newSizedStringJava suggests.


Charset.getDefaultCharset() then tries to read the properties - which 
are currently in the process to be initialized - and fails with a 
NullPointerException.




In the end, I don't know what causes the bug, or how I can replicate it.
I think I did find a likely suspect.

I therefore would like to propose the following patch:

diff --git a/src/java.base/windows/native/libjava/java_props_md.c 
b/src/java.base/windows/native/libjava/java_props_md.c

--- a/src/java.base/windows/native/libjava/java_props_md.c
+++ b/src/java.base/windows/native/libjava/java_props_md.c
@@ -73,6 +73,7 @@
   LOCALE_IDEFAULTANSICODEPAGE,
   ret+2, 14) == 0) {
 codepage = 1252;
+    strcpy(ret, "Cp1252");
 } else {
 codepage = atoi(ret+2);
 }

Because this path is not taken on my system, I can't test if this change 
affects anything. Tests run as before.

But I still think that it is The Right Thing To Do.

Maybe logging the GetLastError too - but it's very early in the VM's 
startup.
I did consider calling GetLocaleInfo twice - with NULL as buffer and 0 
as length - which returns the required size of the buffer.

But that would have some implication on the startup time.
I could also check GetLastError and try again with a bigger buffer - but 
then the question arises: Can the VM handle such a charset?


Johannes

[1]: 
https://stackoverflow.com/questions/61650372/jdk-14-0-1-error-occurred-during-initialization-of-vm-java-lang-nullpointerexcep

[2]: https://bugs.openjdk.java.net/browse/JDK-8226810
[3]: 
https://hg.openjdk.java.net/jdk/jdk/file/d40d865753fb/src/java.base/windows/native/libjava/java_props_md.c#l64
[4]: 
https://hg.openjdk.java.net/jdk/jdk/file/d40d865753fb/src/java.base/share/native/libjava/jni_util.c#l773
[5]: 
https://hg.openjdk.java.net/jdk/jdk/file/d40d865753fb/src/java.base/share/native/libjava/jni_util.c#l661

[8u] RFR(S): 8244548: JDK 8u: sun.misc.Version.jdkUpdateVersion() returns wrong result

[Severin Gehwolf]

Hi,

Please review this OpenJDK 8u fix for an issue where the update version
as configured via --with-update-version=XXX might overflow in internal
JDK structures and thus, will get reported wrong. In particular, only 1
byte is being reserved for the update versions internally. That is, it
works fine up to a configured update version of 255 (2^8 - 1). We've
passed that in OpenJDK 8u with the 8u262 cycle currently in EA. Hence,
we are now seeing this issue.

Bug: https://bugs.openjdk.java.net/browse/JDK-8244548
webrev: http://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8244548/02/
testing: Linux tier 1

The proposed fix is to extend the update_version field in
jdk_version_info from 8 to 16 bit and to use 16 bit of the jvm_version
integer in jvm_version_info. Thus, the new upper bound for update
version number is 2^16-1 => 65535 which should be sufficient. I don't
think OpenJDK 8u will live that long ;-)

jvm_version_info.jvm_version currently holds this quadruplet:

Most significant 8 bits => major version, followed by 8 bits => minor
version, followed by 8 bits => micro version, followed by 8 bits =>
build version. Note that JVM minor version represents the update
version as passed in via configure and the micro version is currently
not used (always 0). See vm_version.cpp lines 100-102 where only major,
minor and build number are ever been set. Knowing this, we can still
preserve the same behavior after patch by defining JVM_VERSION_MICRO to
0 for any version.

For jdk_version_info the fix is simpler, since the update_version is a
separate field for which I've extended it to 16 bit. Andrew Brygin
suggested to reduce the reserved1 field from currently 16 bit to 8 bit
since we are extending update_version by 8 bits, thus making the whole
structure grow. I'm not sure reducing reserved1 to 8 bits so as to not
grow the structure would be necessary, but I'd be happy to do so if
there is such consensus.

Thoughts?

Thanks,
Severin

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

[Kumar Srinivasan]

Hi Mikael,

I may have created solinux when the macosx port was merged and in an effort to 
reduce the CPP conditionals.
solinux = solaris + linux  ie. Vanilla unix code vs Darwin code IIRC has 
Objective-C, MacOSX specific thread initialization etc.

I looked over the launcher related code looks ok to me.

I did notice the \n endings for the messages that Brent pointed out.

Best

Kumar Srinivasan






On May 6, 2020, at 9:43 PM, Mikael Vidstedt 
mailto:mikael.vidst...@oracle.com>> wrote:


I have always wondered what “solinux” is supposed to mean - though not enough 
to actually ask anybody :)

I’ll file a follow-up enhancement to cover renaming the files.

Thank you for the review!

Cheers,
Mikael

On May 4, 2020, at 7:59 AM, Roger Riggs 
mailto:roger.ri...@oracle.com>> wrote:

Hi Michael,

Looks good.

Maybe just a future cleanup to rename files, since the "...so..." is refering 
to solaris.

src/java.base/unix/native/libjli/java_md_solinux.h
src/java.base/unix/native/libjli/java_md_solinux.h

Regards, Roger


On 5/4/20 4:49 AM, Alan Bateman wrote:
On 04/05/2020 06:12, Mikael Vidstedt wrote:
Please review this change which implements part of JEP 381:

JBS: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.openjdk.java.net%2Fbrowse%2FJDK-8244224data=02%7C01%7Ckusrinivasan%40vmware.com%7Ce48bd44f0c484e6fd85608d7f243f1ff%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637244245954061434sdata=W5VvD1owIGoUcbcRkcwixXGPkFLjHUFof2v6cxMqchk%3Dreserved=0
webrev: 
https://nam04.safelinks.protection.outlook.com/?url=http:%2F%2Fcr.openjdk.java.net%2F~mikael%2Fwebrevs%2F8244224%2Fwebrev.00%2Fcorelibs%2Fopen%2Fwebrev%2Fdata=02%7C01%7Ckusrinivasan%40vmware.com%7Ce48bd44f0c484e6fd85608d7f243f1ff%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637244245954061434sdata=tQZIjuF5cIPs%2FNqTRtY2WtmwAgwa0iv205wQ9vk0vOQ%3Dreserved=0
JEP: 
https://nam04.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugs.openjdk.java.net%2Fbrowse%2FJDK-8241787data=02%7C01%7Ckusrinivasan%40vmware.com%7Ce48bd44f0c484e6fd85608d7f243f1ff%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C0%7C0%7C637244245954061434sdata=rDvBE%2BJ2F1IIFC9fbA92rs0KZNgYPg0JZM2ynqp7mcs%3Dreserved=0


Note: When reviewing this, please be aware that this exercise was *extremely* 
mind-numbing, so I appreciate your help reviewing all the individual changes 
carefully. You may want to get that coffee cup filled up (or whatever keeps you 
awake)!

I took a pass over the changes. I agree its a bit tedious. I'm sure there will 
be a few follow up issues as there are opportunities for cleanup in several 
areas. Just a few comments/questions from a first pass.

ExtendedSocketOption.SO_FLOW_SLA is the Solaris specific socket option that was 
terminally deprecated in 14. The patch removes the implementation but leave the 
API (SO_FLOW_SA and jdk.net.SocketFlow). Do you want a someone to take a 
follow-on issue to remove the API?

ResolverConfigurationImpl.localDomain0 can be removed.

The comment on mcast_join_leave in PlainDatagramSocketImpl.c has a residual 
reference to Solaris.

JISAutoDetect - might be simpler to just initialize EUCJPName to "EUC_JP".

Socket.setTrafficClass(int) swallows exceptions to workaround strange behaviour 
on Solaris. Tracked as JDK-8221487 so okay to leave it to that issue if you 
want. There is also cruft in the old plain SocketImpl that to work around 
eagerness to report "connection reset errors - I think we should just leave 
that because the old socket impl is not used by default and will be removed at 
some point.

-Alan.

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

[Roger Riggs]

Hi Naoto,

Looks fine with a small source edit below.

TestUnicodeExtension.java: Please wrap the excessively long lines; 
string concatination will put them together for the test.


Thanks, Roger


On 5/6/20 4:44 PM, naoto.s...@oracle.com wrote:

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://bugs.openjdk.java.net/browse/JDK-8243162), and the rationale 
for this fix is discussed there.


Naoto

Re: RFR: 8237834: com/sun/jndi/ldap/LdapDnsProviderTest.java failing with LDAP response read timeout

[Daniel Fuchs]

Hi Aleksei,

If that solves the issue, then LGTM!

best regards,

-- daniel

On 07/05/2020 14:57, Aleks Efimov wrote:

Hi Daniel,

As a follow-up to our off-list discussion, I've updated the 
generateUnseenPort method:

a) Added comment to be more specific about the number of times it is called
b) The port range has been extended to [, PortConfig.getUpper())

Webrev with new version:
http://cr.openjdk.java.net/~aefimov/8237834/01

With Best Regards,
Aleksei

On 07/05/2020 11:10, Daniel Fuchs wrote:

Hi Aleksei,

I agree with the general idea. However, the method:

 215 private static int generateUnseenPort() {
 216 int port;
 217 do {
 218 port =  + RND.nextInt(1000);
 219 } while (SEEN_PORTS.contains(port));
 220 SEEN_PORTS.add(port);
 221 return port;
 222 }

might never return - if none of the port in the range
[, 2111] are free. Arguably, the test will have
failed in timeout before that. But maybe an assertion
error should be thrown if that happens.

best regards,

-- daniel


On 07/05/2020 00:52, Aleks Efimov wrote:

Hi,

LdapDnsProviderTest test expects to have no services running on  
port on test machine. That could cause it to fail intermittently due 
to unexpected exception message thrown by LDAP client, i.e. timeout 
or disconnect instead of expected connection refusal.


The proposed fix tries to make test more stable ('intermittent' key 
was still added) by running test cases with non-default port multiple 
times by trying to use different random port numbers ('randomness' 
key was added) for each run.


Webrev: http://cr.openjdk.java.net/~aefimov/8237834/00/index.html

JBS: https://bugs.openjdk.java.net/browse/JDK-8237834

Test was not observed to fail for over 300 iterations.

With Best Regards,
Aleksei

Re: RFR: 8237834: com/sun/jndi/ldap/LdapDnsProviderTest.java failing with LDAP response read timeout

[Aleks Efimov]

Hi Daniel,

As a follow-up to our off-list discussion, I've updated the 
generateUnseenPort method:

a) Added comment to be more specific about the number of times it is called
b) The port range has been extended to [, PortConfig.getUpper())

Webrev with new version:
http://cr.openjdk.java.net/~aefimov/8237834/01

With Best Regards,
Aleksei

On 07/05/2020 11:10, Daniel Fuchs wrote:

Hi Aleksei,

I agree with the general idea. However, the method:

 215 private static int generateUnseenPort() {
 216 int port;
 217 do {
 218 port =  + RND.nextInt(1000);
 219 } while (SEEN_PORTS.contains(port));
 220 SEEN_PORTS.add(port);
 221 return port;
 222 }

might never return - if none of the port in the range
[, 2111] are free. Arguably, the test will have
failed in timeout before that. But maybe an assertion
error should be thrown if that happens.

best regards,

-- daniel


On 07/05/2020 00:52, Aleks Efimov wrote:

Hi,

LdapDnsProviderTest test expects to have no services running on  
port on test machine. That could cause it to fail intermittently due 
to unexpected exception message thrown by LDAP client, i.e. timeout 
or disconnect instead of expected connection refusal.


The proposed fix tries to make test more stable ('intermittent' key 
was still added) by running test cases with non-default port multiple 
times by trying to use different random port numbers ('randomness' 
key was added) for each run.


Webrev: http://cr.openjdk.java.net/~aefimov/8237834/00/index.html

JBS: https://bugs.openjdk.java.net/browse/JDK-8237834

Test was not observed to fail for over 300 iterations.

With Best Regards,
Aleksei

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

[Alan Bateman]

On 07/05/2020 12:37, Johannes Kuhn wrote:

:

In the end, I don't know what causes the bug, or how I can replicate it.
I think I did find a likely suspect.
Good sleuthing. I don't what the conditions are for GetLocaleInfo to 
fail but it does look like that would return possibly non-terminated 
garbage starting with "CP" so we should at least fix that.


The issue in JDK-8226810 might be something else. One of the submitters 
to that issue did engage and provided enough information to learn that 
the locale is zh_CN and also reported that it was failing for GB18030. 
GB18030 is not in java.base so that at least explained that report.


-Alan

Re: RFR: 8237834: com/sun/jndi/ldap/LdapDnsProviderTest.java failing with LDAP response read timeout

[Daniel Fuchs]

Hi Aleksei,

I agree with the general idea. However, the method:

 215 private static int generateUnseenPort() {
 216 int port;
 217 do {
 218 port =  + RND.nextInt(1000);
 219 } while (SEEN_PORTS.contains(port));
 220 SEEN_PORTS.add(port);
 221 return port;
 222 }

might never return - if none of the port in the range
[, 2111] are free. Arguably, the test will have
failed in timeout before that. But maybe an assertion
error should be thrown if that happens.

best regards,

-- daniel


On 07/05/2020 00:52, Aleks Efimov wrote:

Hi,

LdapDnsProviderTest test expects to have no services running on  
port on test machine. That could cause it to fail intermittently due to 
unexpected exception message thrown by LDAP client, i.e. timeout or 
disconnect instead of expected connection refusal.


The proposed fix tries to make test more stable ('intermittent' key was 
still added) by running test cases with non-default port multiple times 
by trying to use different random port numbers ('randomness' key was 
added) for each run.


Webrev: http://cr.openjdk.java.net/~aefimov/8237834/00/index.html

JBS: https://bugs.openjdk.java.net/browse/JDK-8237834

Test was not observed to fail for over 300 iterations.

With Best Regards,
Aleksei

Re: RFR [LDAP]: 8062947: Fix exception message to correctly represent LDAP connection failure

[Vyom Tiwari]

LGTM as well.
Vyom

On Thu, May 7, 2020 at 3:01 PM Chris Yin  wrote:

> +1
>
> Thanks,
> Chris
>
> > On 6 May 2020, at 6:35 PM, Daniel Fuchs  wrote:
> >
> > Hi Aleksei,
> >
> > Looks good to me.
> >
> > best regards,
> >
> > -- daniel
> >
> > On 05/05/2020 18:23, Aleks Efimov wrote:
> >> "LDAP response read timed out, timeout used:-1ms.":
> >> https://bugs.openjdk.java.net/browse/JDK-8062947
> >> The following fix tries to tackle this issue:
> >> http://cr.openjdk.java.net/~aefimov/8062947/00
> >
>
>

-- 
Thanks,
Vyom

Re: RFR [LDAP]: 8062947: Fix exception message to correctly represent LDAP connection failure

[Chris Yin]

+1

Thanks,
Chris

> On 6 May 2020, at 6:35 PM, Daniel Fuchs  wrote:
> 
> Hi Aleksei,
> 
> Looks good to me.
> 
> best regards,
> 
> -- daniel
> 
> On 05/05/2020 18:23, Aleks Efimov wrote:
>> "LDAP response read timed out, timeout used:-1ms.":
>> https://bugs.openjdk.java.net/browse/JDK-8062947
>> The following fix tries to tackle this issue:
>> http://cr.openjdk.java.net/~aefimov/8062947/00
> 

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

[Stephen Colebourne]

On Thu, 7 May 2020 at 07:38, Joe Wang  wrote:
> The Javadoc states:
>  If the locale contains the "ca" (calendar), "nu" (numbering
> system), "rg" (region override), and/or "tz" (timezone) Unicode
> extensions, the chronology, numbering system and/or the zone are overridden.
>
> If you remove the two statements that check whether the specified locale
> contains "ca" or "nu", would you need to update the Javadoc as well?

Those things are checked by the methods Chronology.of(locale) and
DecimalStyle.of(locale), so although indirect, I think the Javadoc is
fine.
Stephen

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

[Alan Bateman]

On 07/05/2020 05:56, Mikael Vidstedt wrote:

:

* File follow-up enhancement for the removal of SO_FLOW_SA and 
jdk.net.SocketFlow
I've created JDK-8244582 to track this, we should try to this in the 
same release as JEP 381.



:


* Get confirmation from Alan that the Socket.setTrafficClass(int) and 
SocketImpl stuff is good as-is

JDK-8221487 tracks the Socket.setTrafficClass issue. I didn't create an 
issue to mop up issues in the old SocketImpl as it is not used (at least 
not by default) since JDK 13. The intention is to completely remove the 
old SocketImpl implementation, don't know which release yet but it's 
hardly seems worth spending time doing cleanup on this code. I've no 
doubt there will be cleanup in many areas of the JDK code after JEP 381 
is pushed.


-Alan.

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

[Joe Wang]

Hi Naoto,

The Javadoc states:
    If the locale contains the "ca" (calendar), "nu" (numbering 
system), "rg" (region override), and/or "tz" (timezone) Unicode 
extensions, the chronology, numbering system and/or the zone are overridden.


If you remove the two statements that check whether the specified locale 
contains "ca" or "nu", would you need to update the Javadoc as well?


Best,
Joe

On 5/6/2020 1:44 PM, naoto.s...@oracle.com wrote:

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://bugs.openjdk.java.net/browse/JDK-8243162), and the rationale 
for this fix is discussed there.


Naoto