<!DOCTYPE html>
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<p>Chen,</p>
<p><br>
</p>
<p>thank you for your approval for a DRAFT PR.</p>
<p><br>
</p>
<p>Following your proposal, I have published a DRAFT PR at
<a class="moz-txt-link-freetext" href="https://github.com/openjdk/jdk/pull/25432">https://github.com/openjdk/jdk/pull/25432</a> so everybody can see
what the actual intended code change looks like, and better focus
on that actual code change and its risks and benefits. The
intention of that DRAFT PR explicitly is NOT to request code
reviews, but solely to serve as a publicly visible piece of code
to support the ongoing discussion here on this mailing list.<br>
</p>
<p><br>
</p>
<p>TARGET VERSION: 26<br>
</p>
<p><br>
</p>
<p>As we are near to JDK 25 GA, the target is inclusion into JDK 26
or later, so no need to rush. Nevertheless, I would be happy if
everybody comments ASAP (at least briefly) to keep this discussion
alive.<br>
</p>
<p><br>
</p>
<p>PRELIMINARY RISK ANALYSIS: LOW<br>
</p>
<p><br>
</p>
<p>Given the changes at hand now, I have performed a brief corpus
analysis using Github Copilot (using the public Code Repositories
of OpenJDK itself, but also the repositories of Apache Foundation
and Eclipse Foundation) to see if the proposed change implies REAL
risk due to its change of behavior. The result was that Github
Copilot was UNABLE to find at least one single Java class that
would fail after the proposed change of this draft PR. While many
classes extend Writer directly or indirectly, only few have (nor
their super- nor subclasses) an implementation of the now skipped
methods write(String) or write(String, int, int). Those that do
have (nor their super- nor subclasses) are NOT receiving
append(CharSequence) or append(CharSequence, int, int) calls, or
they (or their super- or subclasses) implement write(String) or
write(String, int, int) as an alias to append(CharSequence) or
append(CharSequence, int, int) - hence they already explicitly do
what the DRAFT PR proposes to do implicitly in future. So at least
for this brief analysis, the ACTUAL risk to break existing code
seems to be rather small actually (in contrast to the ASSUMED risk
looking at "append" being there since JDK 5). NB: Having said
that, one such class WAS actually found by Github Copilot, but I
explicitly removed it from the search result, as I already knew it
is still working correctly, as it is an internal "sun" class of
OpenJDK which I have already visited manually in preparation of
this PR.</p>
<p><br>
</p>
<p>NEXT STEPS</p>
<p><br>
</p>
<p>* Please anybody take note of the code change found in the DRAFT
PR at <a class="moz-txt-link-freetext" href="https://github.com/openjdk/jdk/pull/25432">https://github.com/openjdk/jdk/pull/25432</a> in the next few
weeks to see if anything prevents us from turning this PR from the
DRAFT state into the RFR state.<br>
</p>
<p><br>
</p>
<p>* Unless this discussion points us towards dropping the DRAFT PR,
in the next weeks eventually I will open a CSR for approval of the
behavior change, basing on the result of the attended brief corpus
analysis.</p>
<p><br>
</p>
<p>Thanks everybody for sharing your comments!</p>
<p><br>
</p>
<p>-Markus</p>
<p><br>
</p>
<p><br>
</p>
<p></p>
<div class="moz-cite-prefix">Am 18.05.2025 um 22:50 schrieb Chen
Liang:<br>
</div>
<blockquote type="cite"
cite="mid:SJ2PR10MB766968186EE02189D1D9039DA29DA@SJ2PR10MB7669.namprd10.prod.outlook.com">
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
<style type="text/css" style="display:none;">P {margin-top:0;margin-bottom:0;}</style>
<div class="elementToProof"
style="font-family: "Calibri Light", "Helvetica Light", sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Hi Markus,</div>
<div class="elementToProof"
style="font-family: "Calibri Light", "Helvetica Light", sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
I think you can submit a draft PR - while some people here like
Alan Bateman use draft PRs to indicate "not ready for any
review" and discussions are not forwarded to the mailing lists,
they are still very useful for looking at the change set.</div>
<div class="elementToProof"
style="font-family: "Calibri Light", "Helvetica Light", sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div class="elementToProof"
style="font-family: "Calibri Light", "Helvetica Light", sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Note that we are a few short weeks from RDP1 of 25 - while I
also wish improvements can be integrated into the JDK, there is
no guarantee that they will make 25, and we shouldn't rush
changes in order to catch up with a release train.</div>
<div class="elementToProof"
style="font-family: "Calibri Light", "Helvetica Light", sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
<br>
</div>
<div class="elementToProof"
style="font-family: "Calibri Light", "Helvetica Light", sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Regards, Chen</div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif"
style="font-size:11pt" color="#000000"><b>From:</b>
core-libs-dev <a class="moz-txt-link-rfc2396E" href="mailto:core-libs-dev-retn@openjdk.org"><core-libs-dev-retn@openjdk.org></a> on behalf
of Markus KARG <a class="moz-txt-link-rfc2396E" href="mailto:markus@headcrashing.eu"><markus@headcrashing.eu></a><br>
<b>Sent:</b> Sunday, May 18, 2025 10:09 AM<br>
<b>To:</b> <a class="moz-txt-link-abbreviated" href="mailto:core-libs-dev@openjdk.org">core-libs-dev@openjdk.org</a>
<a class="moz-txt-link-rfc2396E" href="mailto:core-libs-dev@openjdk.org"><core-libs-dev@openjdk.org></a><br>
<b>Subject:</b> Re: RFC: 8356679: Using CharSequence::getChars
internally</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span
style="font-size:11pt;">
<div class="PlainText">Roger,<br>
<br>
thank you for your comments.<br>
<br>
Following your advice I have splitted the larger work of
JDK-8356679 <br>
into sub tasks.<br>
<br>
I would like to start with a first PR implementing the
*foundational* <br>
work, i. e. optimizing Writer::append for efficiency
(JDK-8357183). For <br>
convenience, attached below is a copy of the description.<br>
<br>
Comments Welcome!<br>
<br>
If everybody is fine with this, I would be happy to get
some +1 to <br>
publish the PR for JDK-8357183!<br>
<br>
Am 14.05.2025 um 15:57 schrieb Roger Riggs:<br>
> Hi Markus,<br>
><br>
> Starting out with the common case is a good idea for
the first PR.<br>
><br>
> I much prefer a PR with a single goal and that comes
to a conclusion <br>
> and does not add new features or changes after the PR
is submitted.<br>
> I tend to lose interest in PRs with lots of churn, it
means I have to <br>
> re-review the bulk of it when there is a change and
may wait days to <br>
> let it settle down before coming back to it.<br>
><br>
> I tend to think the PR was not really ready to be
reviewed if simple <br>
> issues and corrections have to be made frequently.<br>
> Do your own checking for typos and copyrights and
simple refactoring <br>
> before opening the PR.<br>
> Quality before quantity or speed.<br>
><br>
> I'm fine with separate Jira issues that clearly
delineate a specific <br>
> scope and goal.<br>
><br>
> The title of this issue (8356679) doesn't identify
the real goal.<br>
> It seems to be to improve performance or memory
usage, not just to use <br>
> a new API.<br>
><br>
> These are my personal opinions about contributions
and process.<br>
><br>
> Regards, Roger<br>
><br>
><br>
> On 5/14/25 6:48 AM, Markus KARG wrote:<br>
>> Many of the modified classes derive from a common
super class and <br>
>> share one needed common change (which is one of
the points which are <br>
>> easy to see once you see all of those classes in
a single PR, but <br>
>> hard to explain in plaint-text pre-PR mailing
list threads), so at <br>
>> least those need to be discussed *together*. But
to spare JBS and <br>
>> PRs, I can open the PR with just the first set of
changes, and once <br>
>> we agree that this set is fine, I can push the
next commit *in the <br>
>> same PR*. Otherwise we would need endless JBS,
mailing list threads, <br>
>> and PRs, just to fixe a dozen internal code
lines.<br>
>><br>
>> Having said that, does the current state of this
thread count as <br>
>> "reached common agreement to file a PR" or do I
still have to wait <br>
>> until more people chime in?<br>
>><br>
>> -Markus<br>
>><br>
>><br>
>> Am 13.05.2025 um 15:10 schrieb Roger Riggs:<br>
>>> Hi Markus,<br>
>>><br>
>>> A main point was to avoid trying to do
everything at once.<br>
>>> The PR comments become hard to follow and
intermingled and it takes <br>
>>> longer to get agreement because of the thrash
in the PR.<br>
>>><br>
>>> Roger<br>
>>><br>
>>> On 5/13/25 5:05 AM, Markus KARG wrote:<br>
>>>> Thank you, Roger.<br>
>>>><br>
>>>> Actually the method helps in the
"toString()" variants, too, as in <br>
>>>> some places we could *get rid* of
"toString()" (which is more work <br>
>>>> than "just" a buffer due to the added
compression complexity).<br>
>>>><br>
>>>> In fact, I already took the time to
rewrite *all* of them while <br>
>>>> waiting for the approval of this list
posting. In *all* cases <br>
>>>> *less* buffering / copying is needed, and
*less* "toString()" <br>
>>>> conversion (which is a copy under the
hood) is needed. So if I <br>
>>>> would be allowed to show the code as a
PR, it would be much easier <br>
>>>> to explain and discuss.<br>
>>>><br>
>>>> A PR is the best place to discuss "how to
code would change". In <br>
>>>> the worst case, let's drop it if we see
that it is actually a bad <br>
>>>> thing.<br>
>>>><br>
>>>> -Markus<br>
>>>><br>
>>>><br>
>>>> Am 12.05.2025 um 20:18 schrieb Roger
Riggs:<br>
>>>>> Hi Markus,<br>
>>>>><br>
>>>>> On the surface, its looks
constructive.<br>
>>>>> I suspect that many of these cases
will turn into discussions <br>
>>>>> about the right/best/better way to
buffer the characters.<br>
>>>>> The getChars method only helps when
extracting to a char array, <br>
>>>>> many of the current implementations
create strings as the <br>
>>>>> intermediary. The advantage of the 1
character at a time technique <br>
>>>>> is not needing a (separated
allocated) buffer.<br>
>>>>> Consider taking a few at a time
before launching into the whole set.<br>
>>>>><br>
>>>>> $.02, Roger<br>
>>>>><br>
>>>>> On 5/11/25 2:45 AM, Markus KARG
wrote:<br>
>>>>>> Dear Core Libs Team,<br>
>>>>>><br>
>>>>>> I am hereby requesting comments
on JDK-8356679.<br>
>>>>>><br>
>>>>>> I would like to invest some time
and set up a PR implementing <br>
>>>>>> Chen Liangs's proposal laid out
in <br>
>>>>>> <a
href="https://bugs.openjdk.org/browse/JDK-8356679"
moz-do-not-send="true" class="moz-txt-link-freetext">https://bugs.openjdk.org/browse/JDK-8356679</a>.
For your
<br>
>>>>>> convenience, the text of that JBS
is copied below. According to <br>
>>>>>> the Developer's Guide I do need
to get broad agreement BEFORE <br>
>>>>>> filing a PR. Therefore, I kindly
ask everybody to briefly show <br>
>>>>>> consent, so I may file a PR.<br>
>>>>>><br>
>>>>>> Thanks<br>
>>>>>> -Markus<br>
>>>>>><br>
>>>>>><br>
>>>>>> Copy from <a
href="https://bugs.openjdk.org/browse/JDK-8356679:"
moz-do-not-send="true" class="moz-txt-link-freetext">https://bugs.openjdk.org/browse/JDK-8356679:</a><br>
>>>>>><br>
>>>>>> Recently OpenJDK adopted the new
method <br>
>>>>>> CharSequence::getChars(int, int,
char[], int) for inclusion in <br>
>>>>>> Java 25. As a bulk reader method,
it allows potentially improved <br>
>>>>>> efficiency over the previously
available char-by-char reader <br>
>>>>>> method CharSequence::charAt(int).<br>
>>>>>><br>
>>>>>> Chen Liang suggested on March
23rd on the core-lib-dev mailing <br>
>>>>>> list to use the new method within
the internal source code of <br>
>>>>>> OpenJDK for the implementation of
Appendables (see <br>
>>>>>> <a
href="https://mail.openjdk.org/pipermail/core-libs-dev/2025-March/141521.html)"
moz-do-not-send="true" class="moz-txt-link-freetext">
https://mail.openjdk.org/pipermail/core-libs-dev/2025-March/141521.html)</a>.
<br>
>>>>>> The idea behind this is that the
implementations might be more <br>
>>>>>> efficient then.<br>
>>>>>><br>
>>>>>> A quick analysis of the OpenJDK
source code identified (at least) <br>
>>>>>> the following classes which could
potentially run more efficient <br>
>>>>>> when using CharSequence::getChars
internally, thanks to bulk <br>
>>>>>> reading and / or prevention of
internal copies / toString() <br>
>>>>>> conversions:<br>
>>>>>> * java.io.Writer<br>
>>>>>> * java.io.StringWriter<br>
>>>>>> * java.io.PrintWriter<br>
>>>>>> * java.io.BufferedWriter<br>
>>>>>> * java.io.CharArrayWriter<br>
>>>>>> * java.io.FileWriter<br>
>>>>>> * java.io.OutputStreamWriter<br>
>>>>>> * sun.nio.cs.StreamEncoder<br>
>>>>>> * java.io.PrintStream<br>
>>>>>> * java.nio.CharBuffer<br>
>>>>>><br>
>>>>>> In the sense of "eat your own dog
food", it makes sense to <br>
>>>>>> implement Chen's idea in (at
least) those classes. Possibly more <br>
>>>>>> classes could get identified when
taking a deeper look. Besides <br>
>>>>>> the potential efficiency
improvements, it would be a good show <br>
>>>>>> case for the usage of the new
API.<br>
>>>>>><br>
>>>>>> The risk of this change should be
low, as test coverage exists, <br>
>>>>>> and as the intended changes are
solely internal to the <br>
>>>>>> implementation. No API will get
changed. In some cases the <br>
>>>>>> JavaDocs will get slightly
adapted where it currently exposes the <br>
>>>>>> actual implementation (to not lie
in future).<br>
>>>>>><br>
>>>>><br>
>>><br>
><br>
</div>
</span></font></div>
</blockquote>
</body>
</html>