Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pasting long json in REPL is slow #40626

Open
septatrix opened this issue Oct 27, 2021 · 11 comments
Open

Pasting long json in REPL is slow #40626

septatrix opened this issue Oct 27, 2021 · 11 comments
Labels
repl Issues and PRs related to the REPL subsystem.

Comments

@septatrix
Copy link

Version

v14.18.1

Platform

Linux thinkpad-e570 5.14.13-200.fc34.x86_64 #1 SMP Mon Oct 18 12:39:31 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

  1. Copy a large amount of data to your clipboard¹
  2. Open the nodejs REPL
  3. Paste the data

¹ The effect can be observed starting at ~2000 chars on my pc with an Intel i7-7500U (4) @ 3.500GHz).
I generated the data using cat /dev/zero | head -c 2000 | tr "\0" 0 | wl-copy

How often does it reproduce? Is there a required condition?

It is consistent and becomes more noticeable with a larger clipboard content

What is the expected behavior?

The pasting should happen instantaneous

python-repl.mp4

What do you see instead?

One can see how the content trickles in bit by bit

nodejs-repl.mp4

Additional information

I am using Gnome Terminal but can observe the same behaviour in other terminals like Alacritty. The cpu utilization when pasting goes up to 100%. Other REPLs like python do not show the same behaviour.

@mscdex
Copy link
Contributor

mscdex commented Oct 27, 2021

Out of curiosity, what if you try doing

$ TERM=dumb node

and then pasting the string? It's not an actual solution, but it should disable the automatic preview which might be causing the slowdown.

@septatrix
Copy link
Author

Yes, then the speed is comparable to other REPLs.

PS: As to why one would ever paste so many characters: In my case I was pasting some json which I got from my browser (due to a complex authentication flow) but wanted to traverse and analyze in nodejs. My workaround was to go the extra step of saving the json to another file and then loading that in nodejs.

@BridgeAR
Copy link
Member

The input is analyzed as @mscdex already outlined. Doing that for a big input could indeed be costly. Having a flamegraph for the specific situation (having a simple perf output would also be fine) would allow us to identify the specific part that is slow. If everything fails, we could limit the input evaluation only to be run for input up to a specific size.

@septatrix
Copy link
Author

I am not sure how to generate such a profiling dump but I guess it should be easy to reproduce. Another possible solution would be to have a little debounce on the preview. Pasting data to the terminal results in a near-zero gap between each char as opposed to holding a key down for which the OS often uses a repetition delay.

@BridgeAR
Copy link
Member

@septatrix node --prof -e 'require("repl").start()'

Without checking closer: it would probably be useful to use a debounce time to trigger the preview.

@septatrix
Copy link
Author

septatrix commented Oct 27, 2021

isolate-0x55ab4670d5f0-367032-v8.log (with 10k chars)

@BridgeAR
Copy link
Member

BridgeAR commented Oct 27, 2021

The following two entries are the main entries.

    588   14.0%   62.6%  LazyCompile: *Interface._getDisplayPos readline.js:842:46
    312    7.4%   33.2%  RegExp: [\u001B\u009B][[\]()#;?]*(?:(?:(?:[a-zA-Z\d]*(?:;[-a-zA-Z\d\/#&.:=?%@~_]*)*)?\u0007)|(?:(?:\d{1,4}(?:;\d{0,4})*)?[\dA-PR-TZcf-ntqry=><~]))

It would be possible to use a special getDisplayPos() function that would only count the row characters as that's required most frequently. In there, we could limit the string that is passed to stripVTControlCharacters() to e.g., a multiple of the current characters visible per line.
The regular expression is also triggered too frequently in _getDisplayPos(). It would only have to be called once up front with stripVTControlCharacters(). getStringWidth() should be called with the second argument being false.

@Mesteery Mesteery added the repl Issues and PRs related to the REPL subsystem. label Oct 27, 2021
@BridgeAR
Copy link
Member

@septatrix you might want to check #41005. This could already improve the situation a bit.

@devsnek
Copy link
Member

devsnek commented Nov 29, 2021

i experimented with this in the prototype repl a bit. you can get a somewhat reasonable handling of this by only highlighting/evaluating/etc between each stdin data event, since pastes will be (more or less) one big event.

@septatrix
Copy link
Author

@septatrix you might want to check #41005. This could already improve the situation a bit.

Is there some easy way to try it out (besides compiling from source)?

@antsmartian
Copy link
Contributor

@BridgeAR I vote for the debounce time too. But wondering what would happen if a user just pastes 1 + 1, we might get a slightly delayed preview than what the user expects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
repl Issues and PRs related to the REPL subsystem.
Projects
None yet
Development

No branches or pull requests

6 participants