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

[Jan Lahoda]

On Mon, 19 May 2025 12:11:46 GMT, Tatsunori Uchino <duke at openjdk.org> wrote:

>> Jan Lahoda has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   (Attempting to) fix the test on Windows.
>
> 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.

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

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