Request for sponsor: JDK-8221430: StringBuffer(CharSequence) constructor truncates when -XX:-CompactStrings specified

[Ivan Gerasimov]

Thanks Andrew!

Introducing getCharSequenceCoder() is actually an enhancement, which may 
improve pre-allocation in certain cases.

It's not actually required to restore correctness of 
StringBuilder/Buffer constructors.

I recommend to separate it from this bug fix, so it can be discussed 
separately, and determined if this is the best approach to this enhancement.

If you agree, I can adjust your latest patch accordingly, run it through 
the Mach5 test systems and push it on your behalf.

With kind regards,

Ivan


On 3/25/19 5:00 PM, Andrew Leonard wrote:
> Hi Ivan,
> Here is my updated webrev: 
> http://cr.openjdk.java.net/~aleonard/8221430/webrev.01/ 
> <http://cr.openjdk.java.net/%7Ealeonard/8221430/webrev.01/>
>
> Thanks
> Andrew
>
> Andrew Leonard
> Java Runtimes Development
> IBM Hursley
> IBM United Kingdom Ltd
> Phone internal: 245913, external: 01962 815913
> internet email: andrew_m_leonard at uk.ibm.com
>
>
>
>
> From: Ivan Gerasimov <ivan.gerasimov at oracle.com>
> To: Andrew Leonard <andrew_m_leonard at uk.ibm.com>
> Cc: core-libs-dev at openjdk.java.net
> Date: 25/03/2019 22:55
> Subject: Re: Request for sponsor: JDK-8221430: 
> StringBuffer(CharSequence) constructor truncates when 
> -XX:-CompactStrings specified
> ------------------------------------------------------------------------
>
>
>
> I was thinking of organizing code similar to what is done in 
> AbstractStringBuilder(int):
>
> if (COMPACT_STRINGS && coderHint == LATIN1) {
>     value = new byte[capacity];
>     coder = LATIN1;
> } else {
>     value = StringUTF16.newBytesFor(capacity);
>     coder = UTF16;
> }
>
> With kind regards,
> Ivan
>
> On 3/25/19 3:45 PM, Andrew Leonard wrote:
> Hi Ivan,
> I think I see what you're saying, you mean we also need to correct 
> this line in AbstractStringBuilder
> constructor:
> value = (coder == LATIN1)
>                ? new byte[capacity] : StringUTF16.newBytesFor(capacity);
> to be maybe:
> value = (COMPACT_STRINGS && coder == LATIN1)
>                ? new byte[capacity] : StringUTF16.newBytesFor(capacity);
>
> The passed in coder stills need to be correct, since with 
> COMPACT_STRINGS a string could be UTF16 if
> it cannot be compacted, so it's more than just a hint isn't it?
>
> Thanks
> Andrew
>
> Andrew Leonard
> Java Runtimes Development
> IBM Hursley
> IBM United Kingdom Ltd
> Phone internal: 245913, external: 01962 815913
> internet email: _andrew_m_leonard at uk.ibm.com_ 
> <mailto:andrew_m_leonard at uk.ibm.com>
>
>
>
>
> From: Ivan Gerasimov _<ivan.gerasimov at oracle.com>_ 
> <mailto:ivan.gerasimov at oracle.com>
> To: Andrew Leonard _<andrew_m_leonard at uk.ibm.com>_ 
> <mailto:andrew_m_leonard at uk.ibm.com>, _core-libs-dev at openjdk.java.net_ 
> <mailto:core-libs-dev at openjdk.java.net>
> Date: 25/03/2019 22:20
> Subject: Re: Request for sponsor: JDK-8221430: 
> StringBuffer(CharSequence) constructor truncates when 
> -XX:-CompactStrings specified
> ------------------------------------------------------------------------
>
>
>
> Hi Andrew!
>
> Thanks for finding this bug!
>
> Your fix solves the problem.
>
> However, I think the main issue is that the constructor
> AbstractStringBuilder(byte,int,int) is now broken:  as you discovered,
> it allows to create a string buffer with the coder LATIN1 when
> COMPACT_STRINGS is false.
>
> Wouldn't it make sense to rename the argument of the constructor to,
> say, coderHint, and then either use it as the coder if
> COMPACT_STRINGS==true, or discard it otherwise.
>
> What do you think?
>
> With kind regards,
> Ivan
>
> On 3/25/19 12:45 PM, Andrew Leonard wrote:
> > Hi,
> > Please can I request a sponsor for this fix to a JDK-13 regression?
> >
> > Patch with jtreg testcase here:
> > _http://cr.openjdk.java.net/~aleonard/8221430/webrev.00/_ 
> <http://cr.openjdk.java.net/%7Ealeonard/8221430/webrev.00/>
> >
> > bug: _https://bugs.openjdk.java.net/browse/JDK-8221430_
> >
> > Many thanks
> > Andrew
> >
> > Andrew Leonard
> > Java Runtimes Development
> > IBM Hursley
> > IBM United Kingdom Ltd
> > Phone internal: 245913, external: 01962 815913
> > internet email: _andrew_m_leonard at uk.ibm.com_ 
> <mailto:andrew_m_leonard at uk.ibm.com>
> >
> > Unless stated otherwise above:
> > IBM United Kingdom Limited - Registered in England and Wales with number
> > 741598.
> > Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire 
> PO6 3AU
> >
>
> -- 
> With kind regards,
> Ivan Gerasimov
>
>
>
>
>
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with 
> number 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 
> 3AU
>
> -- 
> With kind regards,
> Ivan Gerasimov
>
>
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with 
> number 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

-- 
With kind regards,
Ivan Gerasimov