Re: DriverManager.isDriverAllowed has an unintentional side-effect?

[Jaikiran Pai]

Hello Lance,

On 11/02/20 2:05 am, Lance Andersen wrote:
> Hi Jaikiran
>
>
>>
>> Should this check in the isDriverAllowed, instead use "false" and not
>> trigger the intialization of the class?
>
> As I mentioned above, this dates back to the very early days of JDBC
> and JDK 1.2.  Any changes in this area could be quite disruptive and
> would require extensive testing. 
>
That's a valid reason and I understand the unwillingness to change this.

On a related note, the ensureDriversInitialized method right now gets
run once per JVM. However, given that the rest of the DriverManager
deals with per classloader Driver(s), would it be right to somehow make
ensureDriversInitialized run once per classloader instance? That way,
this issue won't show up in first place, given that
ensureDriversInitialized would have ensured that any drivers available
in that classloader would have been loaded during the first call from
class A (in that example).

-Jaikiran

Re: JEP 370 - text example leads to exception

[Chris T]

Paul, thank you very much!

  In the meantime I did more reading about VarHandles and understood better
the "philosophy" of coordinates and it made sense (eventually I was able to
fix my own issues).
  Also, thank you for sharing Maurizio's talk - I will refer it in my
examples. Speaking of sharing - these days I will finish my examples on JEP
370 and will share them with you as well (there will be a github + some
youtube introduction to the feature).
  I think that the capability to allocate more than 2GB of memory is going
to be a hit!

Cheers!
  Chris T

On Mon, Feb 10, 2020 at 12:58 PM Paul Sandoz  wrote:

> Thanks for pointing out the inconsistencies.
>
> I modified the JEP with updated code snippets that compile against the
> latest API in JDK 14 [*].
>
> The handle created “withStride” requires an additional coordinate the is
> an offset from the base address.
>
> You may find Maurizio’s recent talk at Fosdem 2020 helpful and informative:
>
>   https://fosdem.org/2020/schedule/event/bytebuffers/
>
> Hth,
> Paul.
>
> [*] I wish there was a way to automate the compile and test of such
> snippets without duplication.
>
>
> On Feb 7, 2020, at 7:23 PM, Chris T  wrote:
>
> I tried to build an example on top of this code snippet (from the JEP
> text):
>
> VarHandle intHandle = MemoryHandles.varHandle(int.class);
> VarHandle intElemHandle = MemoryHandles.withStride(intHandle, 4);
>
> try (MemorySegment segment = MemorySegment.allocateNative(100)) {
>   MemoryAddress base = segment.baseAddress();
>   for (int i = 0 ; i < 25 ; i++) {
>intElemHandle.set(base, (long)i);
>   }
> }
>
> The first issue was that the API for the first line need to get the
> ByteOrder parameter (that's not a big deal). I ended up with this:
>
>VarHandle intHandle = MemoryHandles.varHandle(int.class, order);
>VarHandle intElemHandle = MemoryHandles.withStride(intHandle, 4);
>
>try (MemorySegment segment = MemorySegment.allocateNative(100)) {
>  MemoryAddress base = segment.baseAddress();
>  for (int i = 0; i < 25; i++) {
>// this is the line where the app crashes:
>intElemHandle.set(base, (long) i);
>  }
>}
>
> The issue that I have is that the code crashes with:
> java.lang.invoke.WrongMethodTypeException: cannot convert
> MethodHandle(VarHandle,MemoryAddressProxy,long,int)void to
> (VarHandle,MemoryAddress,long)void
> at
>
> java.base/java.lang.invoke.MethodHandle.asTypeUncached(MethodHandle.java:880)
> at java.base/java.lang.invoke.MethodHandle.asType(MethodHandle.java:865)
> at
>
> java.base/java.lang.invoke.VarHandleGuards.guard_LJ_V(VarHandleGuards.java:184)
> at
>
> com.github.kbnt.java14.fma.ForeignMemoryAccessExamples.exampleXXStrides(ForeignMemoryAccessExamples.java:65)
> at
>
> com.github.kbnt.java14.fma.ForeignMemoryAccessExamples.main(ForeignMemoryAccessExamples.java:25)
>
> Any idea why this happens (and more importantly how the code can be
> changed)?
>
> Thanks!
>  Chris T
>
>
>

Re: RFR 8235812 : (regex) Unicode linebreak with quantifier does not match valid input

[Ivan Gerasimov]

Thank you Roger for review!

I've adjusted the test as you suggested and pushed the fix.

With kind regards,
Ivan

On 2/10/20 1:11 PM, Roger Riggs wrote:

Hi Ivan,

This look fine.

In the test TegExTest: 5074, I would output the failed cases to 
System.err.

That way they get properly interleaved with the test progress output.

No need for another review.

Thanks, Roger



On 2/5/20 8:22 PM, Ivan Gerasimov wrote:

Hello!

j.u.regex.Pattern supports a special char class \R, which is 
specified to be equal to 
\u000D\u000A|[\u000A\u000B\u000C\u000D\u0085\u2028\u2029].


In particular, this means that the input "\r\n" must match to both 
patterns \R and \R\R.


(In the later case, first \R matches \r and second \R matches \n.)

A pattern \R{2} is expected to be equal to \R\R.

However with the current implementation this does not hold (so, 
Pattern.matches("\\R{2}", "\r\n") == false, while 
Pattern.matches("\\R\\R", "\r\n") == true).


The root cause of this bug is that the special char class \R is 
handled via dedicated class LineEnding, which is not able to 
correctly handle backtracking in  presence of quantifiers).


A simple solution is to treat \R with quantifiers as an anonymous 
group, which will make it comply with the specification.


Without quantifiers, \R is still handled via more efficient 
implementation of LineEnding.


Would you please help review the fix?

Some minor cleanup was done along the way in the affected code.

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8235812
WEBREV: http://cr.openjdk.java.net/~igerasim/8235812/00/webrev/

Control build and testing (tiers1-4) are all green.




--
With kind regards,
Ivan Gerasimov

Re: [15] RFR (xs) 8046362 IdentityHashMap.hash comments should be clarified

[Stuart Marks]

On 2/10/20 7:52 AM, Martin Buchholz wrote:

On Fri, Feb 7, 2020 at 2:47 PM David Holmes  wrote:


Hi Martin,

On 8/02/2020 3:10 am, Martin Buchholz wrote:

I explored System.identityHashCode (see below).


System.identityHashCode is a VM call that is potentially very expensive.
In the worst-case it triggers monitor inflation.



I can believe that.  The VM can't simply return the address of an object
because ... the object might move and an address is a poor hash code
because low bits will tend to be zero.

But  IdentityHashMap already calls System.identityHashCode - that price
must be paid.

My proposal is still "Let's trust System.identityHashCode to produce
acceptable hash codes"


OK... I appreciate that there are a bunch of subtle issues here, from whether 
it's necessary to do mixing of the identity hash code to whether an 
open-addressed table really is faster than separate chaining in this case. 
Before we go on too long, though, I'd like to ask what to do about my proposed 
comments changes.


Should I proceed with pushing my comments changes (which I've appended below for 
convenience)? Or is somebody going to dive in right away and make changes that 
will invalidate my proposed comments changes, and thus I should hold off pushing 
them?


Thanks,

s'marks





# HG changeset patch
# User smarks
# Date 1580167577 28800
#  Mon Jan 27 15:26:17 2020 -0800
# Node ID 0e08ce23484ca42597105225ffa3dd0827cb4b60
# Parent  981f6982717a7df4a2a43dd0ae6b2c2389e683f9
8046362: IdentityHashMap.hash comments should be clarified
Reviewed-by: XXX

diff -r 981f6982717a -r 0e08ce23484c 
src/java.base/share/classes/java/util/IdentityHashMap.java
--- a/src/java.base/share/classes/java/util/IdentityHashMap.javaMon Jan 27 
14:03:58 2020 -0800
+++ b/src/java.base/share/classes/java/util/IdentityHashMap.javaMon Jan 27 
15:26:17 2020 -0800

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2020, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -115,17 +115,19 @@
  * exception for its correctness: fail-fast iterators should be used only
  * to detect bugs.
  *
- * Implementation note: This is a simple linear-probe hash table,
- * as described for example in texts by Sedgewick and Knuth.  The array
- * alternates holding keys and values.  (This has better locality for large
- * tables than does using separate arrays.)  For many JRE implementations
- * and operation mixes, this class will yield better performance than
- * {@link HashMap} (which uses chaining rather than linear-probing).
- *
  * This class is a member of the
  * href="{@docRoot}/java.base/java/util/package-summary.html#CollectionsFramework">

  * Java Collections Framework.
  *
+ * @implNote
+ * This is a simple linear-probe hash table,
+ * as described for example in texts by Sedgewick and Knuth.  The array
+ * contains alternating keys and values, with keys at even indexes and values
+ * at odd indexes. (This arrangement has better locality for large
+ * tables than does using separate arrays.)  For many JRE implementations
+ * and operation mixes, this class will yield better performance than
+ * {@link HashMap}, which uses chaining rather than linear-probing.
+ *
  * @see System#identityHashCode(Object)
  * @see Object#hashCode()
  * @see Collection
@@ -293,7 +295,7 @@
  */
 private static int hash(Object x, int length) {
 int h = System.identityHashCode(x);
-// Multiply by -127, and left-shift to use least bit as part of hash
+// Multiply by -254 to use the hash LSB and to ensure index is even
 return ((h << 1) - (h << 8)) & (length - 1);
 }

Re: RFR [15] 8161558: ListIterator should not discard cause on exception

[Stuart Marks]

Hi Kiran,

Thanks for reworking the test. This looks good to me. One small thing:

  53 throw new IndexOutOfBoundsException();

This should include the offending index in the IOOBE. This might make Rémi 
happy. (Then again, it might not.) :-)


I think this covers the concerns that Martin has, but let's wait a bit to hear 
from him first.


s'marks

On 2/5/20 11:49 AM, Kiran Ravikumar wrote:

Thanks Stuart and Martin,


Here is an updated webrev with the changes.

http://cr.openjdk.java.net/~kravikumar/8161558/webrev.01/

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


Thanks,

Kiran


On 15/01/2020 12:46, Martin Buchholz wrote:

Hi Kiran,

Looks good to me, but I always nitpick ...

Stray semicolon?
       var iterator = list.listIterator(list.size());; // position at end

I would have documented whitebox test assumptions: that nCopies iterators are 
implemented via AbstractList, and that AbstractList's list iterator inherits 
behavior from iterator.


I probably would have added a plain iterator test, and might have refactored 
the code that tests the exception.



On Wed, Jan 15, 2020 at 4:07 AM Kiran Ravikumar 
> wrote:


Hi Guys,


Could someone please review my fix to add missing standard constructor
overloads to NoSuchElementException class and update the AbstractList
class to use them.


A CSR was filed and approved. Along with the code change a new test is
added to verify the behavior.


Please find the webrev at  -


http://cr.openjdk.java.net/~kravikumar/8161558/webrev.00/


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


Thanks,

Kiran

Re: RFR [15] 8161558: ListIterator should not discard cause on exception

[Stuart Marks]

Hi Remi,

The bug report originally requested that a bunch of different exceptions include 
a cause. I don't think the cause should be added to all of them. The cases that 
Kiran is adding are ones where I thought that adding a cause does have value.


If someone is using a ListIterator (or a plain Iterator for that matter) they 
might have an incorrect model for what the index value is after a certain 
operation (remove, for example), and they might get an NSEE unexpectedly. They 
might reasonably wonder what the state of the iterator is that resulted in that 
exception. Without a cause, NSEE doesn't have that information. Chaining the 
IOOBE will usually include the index that caused the problem, which I think is 
useful in such circumstances.


The iterators in AbstractList all keep track of indexes and call get() for 
access to the appropriate element. I don't think they should do a bounds check 
and throw an exception on that basis, because the bounds could change between 
the time they're checked and the call to get(). Thus, the iterators would have 
to catch the exception from get() even if the bounds are checked in advance, 
making the bounds check redundant.


s'marks

On 2/5/20 2:05 PM, Remi Forax wrote:

Stuart, Martin, Kiran,
I think this "bug" should not be fixed because it's one of the cases where 
providing more information is actually bad from a user POV.

The current code throws NoSuchElementException when the iterator reach the end 
so from the user POV, this is the right exception because of the right issue, 
so from the user POV there is no need to change the actual code.
If we chain the exception,
- it's less clear from a user POV
- the user may think that there is an error in the AbstractList implementation 
but it's not the case, it's just that AbstractList iterators next method is 
implemented weirdly, it prefers to go out of bound instead of checking the 
bound.

I'm okay with NoSuchElementException having a new constructor that takes a 
chained exception but i really think that in this specific case, it's a bad 
idea(TM) to use it to propagate an exception to the user that it should not 
care about.

BTW, perhaps, those method next() should be re-written to test the bound instead of 
catching the IOOBE because i'm not sure this "optimization" make sense nowadays.

regards,
Rémi

- Mail original -

De: "Kiran Ravikumar" 
À: "core-libs-dev" 
Envoyé: Mercredi 5 Février 2020 20:49:09
Objet: Re: RFR [15] 8161558: ListIterator should not discard cause on exception



Thanks Stuart and Martin,


Here is an updated webrev with the changes.

http://cr.openjdk.java.net/~kravikumar/8161558/webrev.01/

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


Thanks,

Kiran


On 15/01/2020 12:46, Martin Buchholz wrote:

Hi Kiran,

Looks good to me, but I always nitpick ...

Stray semicolon?
        var iterator = list.listIterator(list.size());; // position at end

I would have documented whitebox test assumptions: that nCopies
iterators are implemented via AbstractList, and that
AbstractList's list iterator inherits behavior from iterator.

I probably would have added a plain iterator test, and might have
refactored the code that tests the exception.


On Wed, Jan 15, 2020 at 4:07 AM Kiran Ravikumar
mailto:kiran.sidhartha.raviku...@oracle.com>> wrote:

 Hi Guys,


 Could someone please review my fix to add missing standard
 constructor
 overloads to NoSuchElementException class and update the AbstractList
 class to use them.


 A CSR was filed and approved. Along with the code change a new
 test is
 added to verify the behavior.


 Please find the webrev at  -


 http://cr.openjdk.java.net/~kravikumar/8161558/webrev.00/


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


 Thanks,

 Kiran

Re: [15] RFR: 8234347: "Turkey" meta time zone does not generate composed localized names, 8236548: Localized time zone name inconsistency between English and other locales

[naoto . sato]

Drafted a CSR:

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

Appreciate the review for this as well.

Naoto

On 2/10/20 8:52 AM, naoto.s...@oracle.com wrote:

Good point. I will file a CSR for the behavioral changes.

Naoto

On 2/7/20 6:00 PM, Joe Wang wrote:

Hi Naoto,

I see the existing tests were changed, e.g. the abbreviation / short 
timezone name, the result of calling getDisplayName. Would you need a 
CSR to discuss/document the potential incompatibility?


Best,
Joe

On 2/7/20 1:44 PM, naoto.s...@oracle.com wrote:

Hello,

Please review the fix to the following issues:

https://bugs.openjdk.java.net/browse/JDK-8234347
https://bugs.openjdk.java.net/browse/JDK-8236548

The proposed changeset is located at:

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

This changeset includes the following changes:

- English time zone names missing in CLDR source files are no longer 
copied from COMPAT provider at build time. Rather it is synthesized 
at runtime, which has been the way other locales did.


- Runtime CLDR time zone name provider fallback logic has been 
refined. It now falls back to parent locales per each missing name, 
instead of resource bundle. Also, region fall back is now using 
exemplar city to synthesize the name (e.g., "Turkey" meta zone)


- Minor fix in DateTimeFormatterBuilder on zone text parsing. It now 
parses zone texts that start with "UTC", yet it is ZoneId.


Naoto

Re: 6202130: Need to handle UTF-8 values and break up lines longer than 72 bytes

[Lance Andersen]

Hi all,

Here is a webrev for the patch that Philipp is proposing which will make it 
easier to review:  http://cr.openjdk.java.net/~lancea/6202130/webrev.00 


> On Dec 26, 2019, at 11:50 AM, Philipp Kunz  wrote:
> 
> Hi,
> The specification says, a line break in a manifest can occur beforeor
> after a Unicode character encoded in UTF-8.
>>   ...>  value: SPACE *otherchar newline
> *continuation>  continuation:  SPACE *otherchar
> newline>...>  otherchar: any UTF-8 character except NUL, CR
> and LF
> The current implementation breaks manifest lines at 72 bytes regardless
> ofhow the bytes around the break are part of a sequence of bytes
> encoding acharacter. Code points may use up to four bytes when encoded
> in UTF-8.Manifests with line breaks inside of sequences of bytes
> encoding Unicodecharacters in UTF-8 with more than one bytes not only
> are invalid UTF-8but also look ugly in text editors.For example, a
> manifest could look like this:
> import java.util.jar.Manifest;import java.util.jar.Attributes;import
> static java.util.jar.Attributes.Name;
> public class CharacterBrokenDemo1 {public static void main(String[]
> args) throws Exception{Manifest mf = new
> Manifest();Attributes attrs =
> mf.getMainAttributes();attrs.put(Name.MANIFEST_VERSION,
> "1.0");attrs.put(new Name("Some-Key"),  "Some
> languages have decorated characters, " +   "for
> example: español"); // or
> "espa\u00D1ol"mf.write(System.out);}}
> Above code produces a result as follows with some unexpected question
> markswhere the encoding is invalid:
>>   Manifest-Version: 1.0>Some-Key: Some languages have decorated
> characters, for example: espa?> ?ol
> This is of course an example written with actual question marks to get
> a validtext for this message. The trick here is that "Some-Key" to
> "example :espa"amounts to exactly one byte less encoded in UTF-8 than
> would fit on one linewith the 72 byte limit so that the subsequent
> character encoded with two bytesgets broken inside of the sequence of
> two bytes for this character across acontinuation line break.
> When decoding the resulting bytes from UTF-8 as one whole string, the
> twoquestion marks will not fit together again even if the line break
> with thecontinuation space is removed. However, Manifest::read removes
> the continuationline breaks ("\r\n ") before decoding the manifest
> header value from UTF-8 andhence can reproduce the original value.
> Characters encoded in UTF-8 can not only span up to four bytes for one
> codepoint each, there are also combining characters or classes thereof
> or combiningdiacritical marks or whatever the appropriate term could
> be, that combine morethan one code point into what is usually
> experienced and referred to as acharacter.
> The term character really gets ambiguous at this point. I wouldn't know
> whatthe specification actually refers to with that term "character". So
> rather thandiving in too much specification or any sorts of theory,
> let's look at anotherexample:
> import java.util.jar.Manifest;import java.util.jar.Attributes;import
> static java.util.jar.Attributes.Name;
> public class DemoCharacterBroken2 {public static void main(String[]
> args) throws Exception{Manifest mf = new
> Manifest();Attributes attrs =
> mf.getMainAttributes();attrs.put(Name.MANIFEST_VERSION,
> "1.0");attrs.put(new Name("Some-Key"), " ".repeat(53) +
> "Angstro\u0308m");mf.write(System.out);}}
> which produces console output as follows:
>>   Manifest-Version: 1.0>Some-
> Key:  Angstro> 
> ̈m
> (In case this does not display well, the diaeresis is on the m on the
> last line)
> When the whole Manifest is decoded from UTF-8 as one big single string
> andcontinuation line breaks are not removed until after UTF-8 decoding
> the wholemanifest, the diaeresis (umlaut, two points above, u0308)
> apparently kind ofjumps onto the following letter m because somehow it
> cannot be combined withthe preceding space. The UTF-8 decoder (all of
> my editors I tried, not onlyEclipse and its console view, also less,
> gedit, cat and terminal) somehowtries to fix that but the diaeresis may
> not necessarily jump back on the "o"where it originally belonged by
> removing the continuation line break ("\r\n ")after UTF-8 decoding has
> taken place, at least that did not work for me.
> Hence, ideally combining diacritical marks should better not be
> separated fromwhatever they combine with when breaking manifest lines
> onto a continuationline. Such combinations, however, seem to be
> unlimited in terms of number ofcode points combining into the same
> "experienced" character. I was able tofind combinations that not only
> exceed the limit of 72 bytes per line but alsoexceed the line buffer
> 

Re: RFR 8235812 : (regex) Unicode linebreak with quantifier does not match valid input

[Roger Riggs]

Hi Ivan,

This look fine.

In the test TegExTest: 5074, I would output the failed cases to System.err.
That way they get properly interleaved with the test progress output.

No need for another review.

Thanks, Roger



On 2/5/20 8:22 PM, Ivan Gerasimov wrote:

Hello!

j.u.regex.Pattern supports a special char class \R, which is specified 
to be equal to \u000D\u000A|[\u000A\u000B\u000C\u000D\u0085\u2028\u2029].


In particular, this means that the input "\r\n" must match to both 
patterns \R and \R\R.


(In the later case, first \R matches \r and second \R matches \n.)

A pattern \R{2} is expected to be equal to \R\R.

However with the current implementation this does not hold (so, 
Pattern.matches("\\R{2}", "\r\n") == false, while 
Pattern.matches("\\R\\R", "\r\n") == true).


The root cause of this bug is that the special char class \R is 
handled via dedicated class LineEnding, which is not able to correctly 
handle backtracking in  presence of quantifiers).


A simple solution is to treat \R with quantifiers as an anonymous 
group, which will make it comply with the specification.


Without quantifiers, \R is still handled via more efficient 
implementation of LineEnding.


Would you please help review the fix?

Some minor cleanup was done along the way in the affected code.

BUGURL: https://bugs.openjdk.java.net/browse/JDK-8235812
WEBREV: http://cr.openjdk.java.net/~igerasim/8235812/00/webrev/

Control build and testing (tiers1-4) are all green.

Re: DriverManager.isDriverAllowed has an unintentional side-effect?

[Lance Andersen]

Hi Jaikiran



> On Feb 10, 2020, at 10:09 AM, Jaikiran Pai  wrote:
> 
> Right now within the implementation of APIs in java.sql.DriverManager
> there are classloader checks to ensure whether the caller is allowed
> access to registered drivers. For example the getDriver(String url)
> API[1] calls the isDriverAllowed (private) method[2] to check if the
> registered driver is allowed access from the caller's classloader. The
> implementation of isDriverAllowed has this[3]:
> 
> aClass =  Class.forName(driver.getClass().getName(), true, classLoader);

This dates back to JDK 1.2 (1998/99) and I would be hesitant to change this at 
this time
> 
> Is it intentional to intialize that class by passing "true" to the
> forName call? The reason I ask is the following scenario:
> 
> 1. Imagine a multi classloader environment.
> 
> 2. Consider postgres JDBC driver (from postgres.jar) gets loaded through
> classloader C1 and is registered in DriverManager in context of C1.
> 
> 3. Now consider a class A loaded in classloader C2. Let's say this C2
> also has postgres.jar in its classpath but hasn't yet loaded any of the
> classes from it yet nor have any calls to "registerDriver" been made by
> any of the classes loaded by this C2.
> 
> 4. Class A (in context of C2) now calls getDriver(String url) passing it
> a JDBC url corresponding to postgres. This will result in a
> SQLException("No suitable driver") exception and that's fine, because C2
> isn't allowed access to driver registered in context of C1. However, now
> consider the following code(again within class A in context of C2):
> 
> try {
> 
> DriverManager.getDriver("jdbc:postgres:");
> 
> } catch (SQLException e) {
> 
>   // expected
> 
>   // now lets do the same call again
> 
>   driver = DriverManager.getDriver("jdbc:postgres:"); --> this one
> passes
> 
> }
> 
> So what's being done is, the SQLException("no suitable driver") is
> caught and the exact same call which triggered this issue, is called
> again. This time the call passes and returns a JDBC driver corresponding
> to the postgres driver.
> 
> Looking at the implementation of DriverManager (which I linked before),
> I can understand why this happens but it just seems odd that the API
> would behave in this manner (that too with no indication in the
> documentation).
> 
> What really is happening here is that the current implementation of
> isDriverAllowed, due to its usage of "true" to initialize the driver
> class ends up triggering the Driver's static block (which as per the
> JDBC spec) is expected/mandated to register with the DriverManager. As a
> result, this call ends up registering the driver, now in the context of
> C2 and as a result the subsequent calls to this (and other APIs) start
> passing from classes loaded by C2 (like that class A).
> 
> Should this check in the isDriverAllowed, instead use "false" and not
> trigger the intialization of the class?

As I mentioned above, this dates back to the very early days of JDBC and JDK 
1.2.  Any changes in this area could be quite disruptive and would require 
extensive testing. 

> 
> [1]
> https://hg.openjdk.java.net/jdk/jdk/file/5a3b04593405/src/java.sql/share/classes/java/sql/DriverManager.java#l266
> 
> [2]
> https://hg.openjdk.java.net/jdk/jdk/file/5a3b04593405/src/java.sql/share/classes/java/sql/DriverManager.java#l280
> 
> [3]
> https://hg.openjdk.java.net/jdk/jdk/file/5a3b04593405/src/java.sql/share/classes/java/sql/DriverManager.java#l555
> 
> -Jaikiran
> 
> 

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: JEP 370 - text example leads to exception

[Paul Sandoz]

Thanks for pointing out the inconsistencies.

I modified the JEP with updated code snippets that compile against the latest 
API in JDK 14 [*].

The handle created “withStride” requires an additional coordinate the is an 
offset from the base address.

You may find Maurizio’s recent talk at Fosdem 2020 helpful and informative:

  https://fosdem.org/2020/schedule/event/bytebuffers/ 


Hth,
Paul.

[*] I wish there was a way to automate the compile and test of such snippets 
without duplication.


> On Feb 7, 2020, at 7:23 PM, Chris T  wrote:
> 
> I tried to build an example on top of this code snippet (from the JEP text):
> 
> VarHandle intHandle = MemoryHandles.varHandle(int.class);
> VarHandle intElemHandle = MemoryHandles.withStride(intHandle, 4);
> 
> try (MemorySegment segment = MemorySegment.allocateNative(100)) {
>   MemoryAddress base = segment.baseAddress();
>   for (int i = 0 ; i < 25 ; i++) {
>intElemHandle.set(base, (long)i);
>   }
> }
> 
> The first issue was that the API for the first line need to get the
> ByteOrder parameter (that's not a big deal). I ended up with this:
> 
>VarHandle intHandle = MemoryHandles.varHandle(int.class, order);
>VarHandle intElemHandle = MemoryHandles.withStride(intHandle, 4);
> 
>try (MemorySegment segment = MemorySegment.allocateNative(100)) {
>  MemoryAddress base = segment.baseAddress();
>  for (int i = 0; i < 25; i++) {
>// this is the line where the app crashes:
>intElemHandle.set(base, (long) i);
>  }
>}
> 
> The issue that I have is that the code crashes with:
> java.lang.invoke.WrongMethodTypeException: cannot convert
> MethodHandle(VarHandle,MemoryAddressProxy,long,int)void to
> (VarHandle,MemoryAddress,long)void
> at
> java.base/java.lang.invoke.MethodHandle.asTypeUncached(MethodHandle.java:880)
> at java.base/java.lang.invoke.MethodHandle.asType(MethodHandle.java:865)
> at
> java.base/java.lang.invoke.VarHandleGuards.guard_LJ_V(VarHandleGuards.java:184)
> at
> com.github.kbnt.java14.fma.ForeignMemoryAccessExamples.exampleXXStrides(ForeignMemoryAccessExamples.java:65)
> at
> com.github.kbnt.java14.fma.ForeignMemoryAccessExamples.main(ForeignMemoryAccessExamples.java:25)
> 
> Any idea why this happens (and more importantly how the code can be
> changed)?
> 
> Thanks!
>  Chris T

Re: New issues in notarization of jpackage-created Mac applications (Scott Palmer)

[James Elliott]

Thanks for drawing my attention to that related bug, Scott. I would add a 
comment on it that Apple has updated the notarization process so this symlink 
is now rejected at that stage, not just by Catalina itself, but I lack standing 
to do so. (It still strikes me as strange that OpenJDK development is so 
insular compared to other open source communities I am involved with. ^_^) But 
I am glad I have at least found this mailing list.

-James

Re: [15] RFR: 8234347: "Turkey" meta time zone does not generate composed localized names, 8236548: Localized time zone name inconsistency between English and other locales

[naoto . sato]

Good point. I will file a CSR for the behavioral changes.

Naoto

On 2/7/20 6:00 PM, Joe Wang wrote:

Hi Naoto,

I see the existing tests were changed, e.g. the abbreviation / short 
timezone name, the result of calling getDisplayName. Would you need a 
CSR to discuss/document the potential incompatibility?


Best,
Joe

On 2/7/20 1:44 PM, naoto.s...@oracle.com wrote:

Hello,

Please review the fix to the following issues:

https://bugs.openjdk.java.net/browse/JDK-8234347
https://bugs.openjdk.java.net/browse/JDK-8236548

The proposed changeset is located at:

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

This changeset includes the following changes:

- English time zone names missing in CLDR source files are no longer 
copied from COMPAT provider at build time. Rather it is synthesized at 
runtime, which has been the way other locales did.


- Runtime CLDR time zone name provider fallback logic has been 
refined. It now falls back to parent locales per each missing name, 
instead of resource bundle. Also, region fall back is now using 
exemplar city to synthesize the name (e.g., "Turkey" meta zone)


- Minor fix in DateTimeFormatterBuilder on zone text parsing. It now 
parses zone texts that start with "UTC", yet it is ZoneId.


Naoto

Re: [15] RFR (xs) 8046362 IdentityHashMap.hash comments should be clarified

[Martin Buchholz]

On Fri, Feb 7, 2020 at 2:47 PM David Holmes  wrote:

> Hi Martin,
>
> On 8/02/2020 3:10 am, Martin Buchholz wrote:
> > I explored System.identityHashCode (see below).
>
> System.identityHashCode is a VM call that is potentially very expensive.
> In the worst-case it triggers monitor inflation.
>

I can believe that.  The VM can't simply return the address of an object
because ... the object might move and an address is a poor hash code
because low bits will tend to be zero.

But  IdentityHashMap already calls System.identityHashCode - that price
must be paid.

My proposal is still "Let's trust System.identityHashCode to produce
acceptable hash codes"

DriverManager.isDriverAllowed has an unintentional side-effect?

[Jaikiran Pai]

Right now within the implementation of APIs in java.sql.DriverManager
there are classloader checks to ensure whether the caller is allowed
access to registered drivers. For example the getDriver(String url)
API[1] calls the isDriverAllowed (private) method[2] to check if the
registered driver is allowed access from the caller's classloader. The
implementation of isDriverAllowed has this[3]:

aClass =  Class.forName(driver.getClass().getName(), true, classLoader);

Is it intentional to intialize that class by passing "true" to the
forName call? The reason I ask is the following scenario:

1. Imagine a multi classloader environment.

2. Consider postgres JDBC driver (from postgres.jar) gets loaded through
classloader C1 and is registered in DriverManager in context of C1.

3. Now consider a class A loaded in classloader C2. Let's say this C2
also has postgres.jar in its classpath but hasn't yet loaded any of the
classes from it yet nor have any calls to "registerDriver" been made by
any of the classes loaded by this C2.

4. Class A (in context of C2) now calls getDriver(String url) passing it
a JDBC url corresponding to postgres. This will result in a
SQLException("No suitable driver") exception and that's fine, because C2
isn't allowed access to driver registered in context of C1. However, now
consider the following code(again within class A in context of C2):

try {

    DriverManager.getDriver("jdbc:postgres:");

} catch (SQLException e) {

  // expected

  // now lets do the same call again

  driver = DriverManager.getDriver("jdbc:postgres:"); --> this one
passes

}

So what's being done is, the SQLException("no suitable driver") is
caught and the exact same call which triggered this issue, is called
again. This time the call passes and returns a JDBC driver corresponding
to the postgres driver.

Looking at the implementation of DriverManager (which I linked before),
I can understand why this happens but it just seems odd that the API
would behave in this manner (that too with no indication in the
documentation).

What really is happening here is that the current implementation of
isDriverAllowed, due to its usage of "true" to initialize the driver
class ends up triggering the Driver's static block (which as per the
JDBC spec) is expected/mandated to register with the DriverManager. As a
result, this call ends up registering the driver, now in the context of
C2 and as a result the subsequent calls to this (and other APIs) start
passing from classes loaded by C2 (like that class A).

Should this check in the isDriverAllowed, instead use "false" and not
trigger the intialization of the class?

[1]
https://hg.openjdk.java.net/jdk/jdk/file/5a3b04593405/src/java.sql/share/classes/java/sql/DriverManager.java#l266

[2]
https://hg.openjdk.java.net/jdk/jdk/file/5a3b04593405/src/java.sql/share/classes/java/sql/DriverManager.java#l280

[3]
https://hg.openjdk.java.net/jdk/jdk/file/5a3b04593405/src/java.sql/share/classes/java/sql/DriverManager.java#l555

-Jaikiran

Re: [PATCH] Enhancement proposal for java.util.StringJoiner

[Сергей Цыпанов]

Hello Tagir,

FYI, there's a full table of results for original/patched benchmarks:

Benchmark  (count)  (latin)  (length)  
OriginalPatched   Units
stringJoiner 1 true 1  26.9 ±   
0.7   48.8 ±   2.2   ns/op
stringJoiner 1 true 5  30.5 ±   
1.0   46.1 ±   2.1   ns/op
stringJoiner 1 true10  31.2 ±   
0.6   47.3 ±   1.3   ns/op
stringJoiner 1 true   100  62.5 ±   
3.3   79.9 ±   4.8   ns/op
stringJoiner 5 true 1  78.2 ±   
1.6  110.3 ±   2.9   ns/op
stringJoiner 5 true 5  94.2 ±   
8.7  116.6 ±   0.7   ns/op
stringJoiner 5 true10  95.3 ±   
6.9  100.1 ±   0.4   ns/op
stringJoiner 5 true   100 188.0 ±  
10.2  136.0 ±   0.4   ns/op
stringJoiner10 true 1 160.3 ±   
4.5  172.9 ±   0.8   ns/op
stringJoiner10 true 5 169.0 ±   
4.7  180.2 ±   9.1   ns/op
stringJoiner10 true10 205.7 ±  
16.4  182.7 ±   1.1   ns/op
stringJoiner10 true   100 366.5 ±  
17.0  284.5 ±   3.1   ns/op
stringJoiner   100 true 11117.6 ±  
11.1 2123.7 ±  11.1   ns/op
stringJoiner   100 true 51270.7 ±  
40.2 2163.6 ±  12.4   ns/op
stringJoiner   100 true101364.4 ±  
14.0 2283.8 ±  16.1   ns/op
stringJoiner   100 true   1003592.9 ± 
164.8 3535.2 ±  29.9   ns/op
stringJoiner 1false 1  35.6 ±   
1.2   59.1 ±   3.0   ns/op
stringJoiner 1false 5  39.3 ±   
1.2   52.6 ±   2.5   ns/op
stringJoiner 1false10  42.2 ±   
1.6   53.6 ±   0.3   ns/op
stringJoiner 1false   100  70.5 ±   
1.8   86.4 ±   0.4   ns/op
stringJoiner 5false 1  89.0 ±   
3.5  102.2 ±   1.0   ns/op
stringJoiner 5false 5  87.6 ±   
0.7  106.5 ±   1.2   ns/op
stringJoiner 5false10 109.0 ±   
5.6  116.5 ±   1.2   ns/op
stringJoiner 5false   100 324.0 ±  
16.5  221.9 ±   0.5   ns/op
stringJoiner10false 1 183.9 ±   
5.9  204.7 ±   5.5   ns/op
stringJoiner10false 5 198.7 ±   
9.7  202.4 ±   1.5   ns/op
stringJoiner10false10 196.7 ±   
6.9  226.7 ±   6.4   ns/op
stringJoiner10false   100 535.8 ±   
2.3  553.0 ±   5.6   ns/op
stringJoiner   100false 11674.6 ± 
122.1 1940.8 ±  16.2   ns/op
stringJoiner   100false 51791.9 ±  
58.1 2158.1 ±  12.0   ns/op
stringJoiner   100false102124.1 ± 
193.3 2364.0 ±  25.2   ns/op
stringJoiner   100false   1004323.4 ±  
29.2 4675.5 ±  11.8   ns/op

stringJoiner:·gc.alloc.rate.norm 1 true 1 120.0 ±   
0.0  120.0 ±   0.0B/op
stringJoiner:·gc.alloc.rate.norm 1 true 5 128.0 ±   
0.0  120.0 ±   0.0B/op
stringJoiner:·gc.alloc.rate.norm 1 true10 144.0 ±   
0.0  136.0 ±   0.0B/op
stringJoiner:·gc.alloc.rate.norm 1 true   100 416.0 ±   
0.0  312.0 ±   0.0B/op
stringJoiner:·gc.alloc.rate.norm 5 true 1 144.0 ±   
0.0  136.0 ±   0.0B/op
stringJoiner:·gc.alloc.rate.norm 5 true 5 200.0 ±   
0.0  168.0 ±   0.0B/op
stringJoiner:·gc.alloc.rate.norm 5 true10 272.0 ±   
0.0  216.0 ±   0.0B/op
stringJoiner:·gc.alloc.rate.norm 5 true   1001632.0 ±   
0.0 1128.0 ±   0.0B/op
stringJoiner:·gc.alloc.rate.norm10 true 1 256.0 ±   
0.0  232.0 ±   0.0B/op
stringJoiner:·gc.alloc.rate.norm10 true 5 376.0 ±   
0.0  316.8 ±   4.9B/op
stringJoiner:·gc.alloc.rate.norm10 true10 520.0 ±   
0.0  408.0 

Re: [PATCH] Enhancement proposal for java.util.StringJoiner

[Tagir Valeev]

Hello!

In many tests, I see little or no performance improvements. E.g.:
stringJoiner  10010   1768.8 ±
160.6  1760.8 ± 111.4ns/op

How would you explain that this code change doesn't improve the
performance for given count and length?

Also, you are using `new String(bytes)` assuming that the platform
charset is latin1-compatible. This is not always true, so your code
would produce incorrect results depending on this setting. In general,
decoding via charset is tons of extra work. Instead, I would
experiment with pre-sized StringBuilder inside compactElts, instead of
char[] array. While it does some extra checks, StringBuilder already
can use the compact string representation, so if it's properly
pre-sized, no extra memory will be allocated.

Finally, if you optimize for the case when X is true you should always
benchmark what happens when X is false. It's great that in some cases
we see a speedup for latin1 strings. But what if not all of the
strings are latin1? Is there any performance degradation? If yes, can
we tolerate it?

With best regards,
Tagir Valeev.

On Mon, Feb 10, 2020 at 3:22 PM Сергей Цыпанов
 wrote:
>
> Hello,
>
> I've reworked the code, patch is attached. Could you please review my 
> solution regarding usage of SahredSecrets?
>
> P.S. After I've created the patch it came to my mind that instead of checking 
> all Strings when calling StringJoiner.add()
> we can check them in toString() method and fail-fast in case at least one of 
> them is non-latin. This likely to reduce
> regression related to usage of reflection.
>
> Regards,
> Sergey Tsypanov
>
> 05.02.2020, 23:21, "fo...@univ-mlv.fr" :
> > - Mail original -
> >>  De: "Сергей Цыпанов" 
> >>  À: "Remi Forax" , "core-libs-dev" 
> >> 
> >>  Envoyé: Mercredi 5 Février 2020 22:12:34
> >>  Objet: Re: [PATCH] Enhancement proposal for java.util.StringJoiner
> >
> >>  Hello,
> >>
> >>>  If you want to optimize StringJoiner, the best way to do it is to use 
> >>> the shared
> >>>  secret mechanism so a java.util class can see implementation details of a
> >>>  java.lang class without exposing those details publicly.
> >>>  As an example, take a look to EnumSet and its implementations.
> >>
> >>  I've looked into SharedSecrets, it seems there's no ready-to-use method 
> >> for
> >>  accessing package-private method. Do you mean it's necessary to add 
> >> particular
> >>  functionality to JavaLangReflectionAccess as they did for JavaLangAccess 
> >> in
> >>  order to deal with EnumSet?
> >
> > yes !
> > crossing package boundary in a non public way is not free,
> > but given that StringJoiner is used quite often (directly or indirectly 
> > using Collectors.joining()), it may worth the cost.
> >
> >>  Regards,
> >>  Sergey
> >
> > Regards,
> > Rémi
> >
> >>  04.02.2020, 12:12, "Remi Forax" :
> >>>  - Mail original -
>    De: "Сергей Цыпанов" 
>    À: "jonathan gibbons" , "core-libs-dev"
>    
>    Envoyé: Mardi 4 Février 2020 08:53:31
>    Objet: Re: [PATCH] Enhancement proposal for java.util.StringJoiner
> >>>
>    Hello,
> >>>
> >>>  Hi Sergey,
> >>>
>    I'd probably agree about a new class in java.lang, but what is wrong 
>  about
>    exposing package-private method
>    which doesn't modify the state of the object and has no side effects?
> >>>
> >>>  You can not change the implementation anymore,
> >>>  by example if instead of having a split between latin1 and non latin1, 
> >>> we decide
> >>>  in the future to split between utf8 and non utf8.
> >>>
> >>>  If you want to optimize StringJoiner, the best way to do it is to use 
> >>> the shared
> >>>  secret mechanism so a java.util class can see implementation details of a
> >>>  java.lang class without exposing those details publicly.
> >>>  As an example, take a look to EnumSet and its implementations.
> >>>
> >>>  regards,
> >>>  Rémi
> >>>
>    04.02.2020, 00:58, "Jonathan Gibbons" :
> >   Sergey,
> >
> >   It is equally bad to create a new class in the java.lang package as it
> >   is to add a new public method to java.lang.String.
> >
> >   -- Jon
> >
> >   On 2/3/20 2:38 PM, Сергей Цыпанов wrote:
> >>Hello,
> >>
> >>as of JDK14 java.util.StringJoiner still uses char[] as a storage 
> >> of glued
> >>Strings.
> >>
> >>This applies for the cases when all joined Strings as well as 
> >> delimiter, prefix
> >>and suffix contain only ASCII symbols.
> >>
> >>As a result when StringJoiner.toString() is invoked, byte[] stored 
> >> in String is
> >>inflated in order to fill in char[] and
> >>finally char[] is compressed when constructor of String is called:
> >>
> >>String delimiter = this.delimiter;
> >>char[] chars = new char[this.len + addLen];
> >>int k = getChars(this.prefix, chars, 0);
> >>if (size > 0) {
> 

Re: RFR: 8237878: Improve ModuleLoaderMap datastructures

[Claes Redestad]

On 2020-02-10 12:34, Alan Bateman wrote:

On 10/02/2020 09:04, Claes Redestad wrote:

:

So how about:
http://cr.openjdk.java.net/~redestad/8237878/open.02/


Thanks for restoring the use of Function. Changing 
Module::defineClass to invoke a method on ModuleLoaderMap is okay but 
the method needs to something like "isBuiltinMapper" because it tests if 
the function is a built-in mapper used for the boot layer (or child 
layers created when we need to dynamically augment the set of platform 
modules).


Minor nit but I think the comment on the Mapper constructor would say 
that it creates a Mapper to map the modules in the given Configuration 
to the built-in class loaders.


The rest looks good to me.


Thanks!

I'll run a few tests and push with this addendum, then:

diff -r 43b98c0e075d src/java.base/share/classes/java/lang/Module.java
--- a/src/java.base/share/classes/java/lang/Module.java	Mon Feb 10 
12:40:49 2020 +0100
+++ b/src/java.base/share/classes/java/lang/Module.java	Mon Feb 10 
12:42:43 2020 +0100

@@ -1094,7 +1094,7 @@

 // map each module to a class loader
 ClassLoader pcl = ClassLoaders.platformClassLoader();
-boolean isModuleLoaderMapper = 
ModuleLoaderMap.isModuleLoaderMapper(clf);
+boolean isModuleLoaderMapper = 
ModuleLoaderMap.isBuiltinMapper(clf);


 for (int index = 0; index < numModules; index++) {
 String name = resolvedModules[index].name();
diff -r 43b98c0e075d 
src/java.base/share/classes/jdk/internal/module/ModuleLoaderMap.java
--- 
a/src/java.base/share/classes/jdk/internal/module/ModuleLoaderMap.java 
Mon Feb 10 12:40:49 2020 +0100
+++ 
b/src/java.base/share/classes/jdk/internal/module/ModuleLoaderMap.java 
Mon Feb 10 12:42:43 2020 +0100

@@ -61,7 +61,8 @@
 private final Map map;

 /**
- * Maps module names to the corresponding built-in classloader.
+ * Creates a Mapper to map module names in the given 
Configuration to

+ * built-in classloaders.
  *
  * As a proxy for the actual classloader, we store an easily 
archiveable
  * index value in the internal map. The index is stored as a 
boxed value

@@ -132,7 +133,7 @@
  * to the boot or platform classloader if the ClassLoader mapping 
function

  * originate from here.
  */
-public static boolean isModuleLoaderMapper(FunctionClassLoader> clf) {
+public static boolean isBuiltinMapper(Function 
clf) {

 return clf instanceof Mapper;
 }
 }

/Claes

Re: RFR: 8237878: Improve ModuleLoaderMap datastructures

[Alan Bateman]

On 10/02/2020 09:04, Claes Redestad wrote:

:

So how about:
http://cr.openjdk.java.net/~redestad/8237878/open.02/


Thanks for restoring the use of Function. Changing 
Module::defineClass to invoke a method on ModuleLoaderMap is okay but 
the method needs to something like "isBuiltinMapper" because it tests if 
the function is a built-in mapper used for the boot layer (or child 
layers created when we need to dynamically augment the set of platform 
modules).


Minor nit but I think the comment on the Mapper constructor would say 
that it creates a Mapper to map the modules in the given Configuration 
to the built-in class loaders.


The rest looks good to me.

-Alan

Re: RFR: 8237878: Improve ModuleLoaderMap datastructures

[Claes Redestad]

On 2020-02-09 17:49, Alan Bateman wrote:

On 06/02/2020 13:48, Claes Redestad wrote:

:

New webrev:

http://cr.openjdk.java.net/~redestad/8237878/open.01/
The archiving looks good but I'd prefer if this patch didn't change the 
usages of Function to ModuleLoaderMap.Mapper - 
that's an implementation class that should not be used outside of the 
ModuleLoaderMap (it should be private but there is special check in the 
Module code that prevents this).


Ok, makes sense to make this private - I guess we could refactor the
instanceof in Module to call a function on ModuleLoaderMap to
encapsulate this better. Hoisting the check from the loop is a small
optimization for the bootstrap case, regardless.



The ModuleLoaderMap constructor needs a comment so that future 
maintainers know that it maps the modules in the given configuration to 
the built-in class loaders. Also would be good if the declaration of map 
were changed back to Map and a // comment to make it 
clear that it maps the module name to an index.


So how about:
http://cr.openjdk.java.net/~redestad/8237878/open.02/

/Claes

Re: [PATCH] Enhancement proposal for java.util.StringJoiner

[Сергей Цыпанов]

Hello,

I've reworked the code, patch is attached. Could you please review my solution 
regarding usage of SahredSecrets?

P.S. After I've created the patch it came to my mind that instead of checking 
all Strings when calling StringJoiner.add()
we can check them in toString() method and fail-fast in case at least one of 
them is non-latin. This likely to reduce 
regression related to usage of reflection.

Regards,
Sergey Tsypanov

05.02.2020, 23:21, "fo...@univ-mlv.fr" :
> - Mail original -
>>  De: "Сергей Цыпанов" 
>>  À: "Remi Forax" , "core-libs-dev" 
>> 
>>  Envoyé: Mercredi 5 Février 2020 22:12:34
>>  Objet: Re: [PATCH] Enhancement proposal for java.util.StringJoiner
>
>>  Hello,
>>
>>>  If you want to optimize StringJoiner, the best way to do it is to use the 
>>> shared
>>>  secret mechanism so a java.util class can see implementation details of a
>>>  java.lang class without exposing those details publicly.
>>>  As an example, take a look to EnumSet and its implementations.
>>
>>  I've looked into SharedSecrets, it seems there's no ready-to-use method for
>>  accessing package-private method. Do you mean it's necessary to add 
>> particular
>>  functionality to JavaLangReflectionAccess as they did for JavaLangAccess in
>>  order to deal with EnumSet?
>
> yes !
> crossing package boundary in a non public way is not free,
> but given that StringJoiner is used quite often (directly or indirectly using 
> Collectors.joining()), it may worth the cost.
>
>>  Regards,
>>  Sergey
>
> Regards,
> Rémi
>
>>  04.02.2020, 12:12, "Remi Forax" :
>>>  - Mail original -
   De: "Сергей Цыпанов" 
   À: "jonathan gibbons" , "core-libs-dev"
   
   Envoyé: Mardi 4 Février 2020 08:53:31
   Objet: Re: [PATCH] Enhancement proposal for java.util.StringJoiner
>>>
   Hello,
>>>
>>>  Hi Sergey,
>>>
   I'd probably agree about a new class in java.lang, but what is wrong 
 about
   exposing package-private method
   which doesn't modify the state of the object and has no side effects?
>>>
>>>  You can not change the implementation anymore,
>>>  by example if instead of having a split between latin1 and non latin1, we 
>>> decide
>>>  in the future to split between utf8 and non utf8.
>>>
>>>  If you want to optimize StringJoiner, the best way to do it is to use the 
>>> shared
>>>  secret mechanism so a java.util class can see implementation details of a
>>>  java.lang class without exposing those details publicly.
>>>  As an example, take a look to EnumSet and its implementations.
>>>
>>>  regards,
>>>  Rémi
>>>
   04.02.2020, 00:58, "Jonathan Gibbons" :
>   Sergey,
>
>   It is equally bad to create a new class in the java.lang package as it
>   is to add a new public method to java.lang.String.
>
>   -- Jon
>
>   On 2/3/20 2:38 PM, Сергей Цыпанов wrote:
>>    Hello,
>>
>>    as of JDK14 java.util.StringJoiner still uses char[] as a storage of 
>> glued
>>    Strings.
>>
>>    This applies for the cases when all joined Strings as well as 
>> delimiter, prefix
>>    and suffix contain only ASCII symbols.
>>
>>    As a result when StringJoiner.toString() is invoked, byte[] stored in 
>> String is
>>    inflated in order to fill in char[] and
>>    finally char[] is compressed when constructor of String is called:
>>
>>    String delimiter = this.delimiter;
>>    char[] chars = new char[this.len + addLen];
>>    int k = getChars(this.prefix, chars, 0);
>>    if (size > 0) {
>> k += getChars(elts[0], chars, k); // inflate byte[] -> char[]
>>
>> for(int i = 1; i < size; ++i) {
>> k += getChars(delimiter, chars, k);
>> k += getChars(elts[i], chars, k);
>> }
>>    }
>>
>>    k += getChars(this.suffix, chars, k);
>>    return new String(chars); // compress char[] -> byte[]
>>
>>    This can be improved by detecting cases when String.isLatin1() 
>> returns true for
>>    all involved Strings.
>>
>>    I've prepared a patch along with benchmark proving that this change 
>> is correct
>>    and brings improvement.
>>    The only concern I have is about String.isLatin1(): as far as String 
>> belongs to
>>    java.lang and StringJoiner to java.util
>>    package-private String.isLatin1() cannot be directly accessed, we 
>> need to make
>>    it public for successful compilation.
>>
>>    Another solution is to create an intermediate utility class located 
>> in java.lang
>>    which delegates the call to String.isLatin1():
>>
>>    package java.lang;
>>
>>    public class StringHelper {
>> public static boolean isLatin1(String str) {
>> return str.isLatin1();
>> }
>>    }
>>
>>    This allows to keep java.lang.String intact and have access to it's