kernel: bypass unicode translation in group for latin1 putc_requests #9013
Conversation
CT Test Results 3 files 155 suites 2h 10m 40s ⏱️ Results for commit 27e7d05. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
lib/kernel/src/group.erl
Outdated
| Latin1 = get_unicode_state(Drv) =:= false, | ||
| Raw = get_output_state(Drv) =:= raw, |
There was a problem hiding this comment.
Is my understanding correct that this adds two extra round-trip RPC calls between group and the driver for every print request?
There was a problem hiding this comment.
Yes, your understanding was correct, but that has been removed, I think we can assume that user_drvers output mode is raw when we are in noshell mode. And we then only have to check encoding is latin1. which is now done in setopts. I have changed so that we dont need the extra roundtrips.
There was a problem hiding this comment.
I think we can assume that user_drvers output mode is raw when we are in noshell mode.
No, you cannot. The shell =:= noshell in group is used to determine if a shell process is attached to the specific group, that is if the group is the user process or not. If the system has a shell, there will be one or more other group processes that handles the shells. Also, the change of encoding request can happen from any of the groups in the system.
You can however assume that it is only the process with shell =:= noshell that we can do the optimization with, so if you want to avoid doing the calls to user_drv, then user_drv needs to notify the user group of the encoding change. user_drv can also switch from being in raw to cooked mode if anyother issues shell:start_interactive/0, so user_drv also need to notify user of that change.
There was a problem hiding this comment.
Aha, then I will revert back to asking the user_drv for now. But make it into a single question instead of two.
frazze-jobb
force-pushed
the
frazze/kernel/group_latin1_bypass/OTP-19296
branch
from
2f69015
to
c4c6ca1
Compare
lib/stdlib/test/escript_SUITE.erl
Outdated
| @@ -39,7 +39,8 @@ | |||
| overflow/1, | |||
| verify_sections/4, | |||
| unicode/1, | |||
| bad_io_server/1 | |||
| bad_io_server/1, | |||
| bypass_unicode_conversion/1 | |||
There was a problem hiding this comment.
| bypass_unicode_conversion/1 | |
| bypass_unicode_conversion/1 |
lib/kernel/src/group.erl
Outdated
| Latin1 = get_unicode_state(Drv) =:= false, | ||
| Raw = get_output_state(Drv) =:= raw, |
There was a problem hiding this comment.
I think we can assume that user_drvers output mode is raw when we are in noshell mode.
No, you cannot. The shell =:= noshell in group is used to determine if a shell process is attached to the specific group, that is if the group is the user process or not. If the system has a shell, there will be one or more other group processes that handles the shells. Also, the change of encoding request can happen from any of the groups in the system.
You can however assume that it is only the process with shell =:= noshell that we can do the optimization with, so if you want to avoid doing the calls to user_drv, then user_drv needs to notify the user group of the encoding change. user_drv can also switch from being in raw to cooked mode if anyother issues shell:start_interactive/0, so user_drv also need to notify user of that change.
frazze-jobb
force-pushed
the
frazze/kernel/group_latin1_bypass/OTP-19296
branch
4 times, most recently
from
96bf978
to
ec11a53
Compare
frazze-jobb
added
the
testing
lib/kernel/src/user_drv.erl
Outdated
| io_request({put_chars_sync, latin1, Chars, Reply}, TTY) -> | ||
| case {prim_tty:unicode(TTY), prim_tty:output_mode(TTY)} of | ||
| {false, raw} -> | ||
| Bin = if is_binary(Chars) -> Chars; true -> list_to_binary(Chars) end, |
There was a problem hiding this comment.
If the list_to_binary crashes, an error should be returned to the user.
frazze-jobb
force-pushed
the
frazze/kernel/group_latin1_bypass/OTP-19296
branch
from
ec11a53
to
9d6974f
Compare
latin1 requests are sent to user_drv, if its in latin1 mode, and raw output mode, then just output the latin1 binary. Otherwise user_drv will convert the output to unicode.
frazze-jobb
force-pushed
the
frazze/kernel/group_latin1_bypass/OTP-19296
branch
from
9d6974f
to
27e7d05
Compare
Group checks that user_drv is in raw output mode and latin1 encoding mode
if true then the request is sent as latin1 and outputted by user_drv as is
if false then the request is converted to a unicode binary before its sent to user_drv.