RFR: 8356165: System.in in jshell replace supplementary characters with ?? [v3]

[Jan Lahoda]

On Mon, 19 May 2025 12:52:51 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

>> src/jdk.jshell/share/classes/jdk/internal/jshell/tool/ConsoleIOContext.java line 980:
>> 
>>> 978:         if (pendingBytes == null || pendingBytes.length <= pendingBytesPointer) {
>>> 979:             char userChar = readUserInputChar();
>>> 980:             StringBuilder dataToConvert = new StringBuilder();
>> 
>> FWIW I think we can avoid using StringBuilder (and make the code more RAM-friendly):
>> 
>> 
>> char[] dataToConvert = { useChar, '\0' };
>> // if (...) {
>> // ...
>> // if (...) {
>> // ...
>> dataToConvert[1] = lowSurrogate;
>> // }
>> // ...
>> // }
>> // low-surrogate code unit never be null char
>> pendingBytes = dataToConvert[1] != '\0' ? String.valueOf(dataToConvert) : String.valueOf(dataToConvert[0]);
>> 
>> 
>> The next version of .NET is said to be able to allocate such a tiny array to the stack, instead of the heap, but I don't know whether JVM can do the same optimization.
>
> We could do something like that, although I wrote it as I wrote it mostly because that's more clearly correct. Although the current tests probably cover all the cases, so with a bit of work, we probably could eliminate the (explicit) array completely.
> 
> Overall, on most places, it is usually not necessary to be too clever - the VM can optimize and eliminate allocations if needed.
> 
> I'll leave it up to Adam and/or Christian whether they would prefer a slightly more complex code with less (explicit/visible) allocation.

FWIW, we could do this:
https://github.com/lahodaj/jdk/commit/6a07648829ee8df263cf3fc1c55055fe45d4eb17

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25079#discussion_r2095681607