readline: improve performance#64585
Open
mcollina wants to merge 2 commits into
Open
Conversation
Speed up Interface construction: - Hoist the history accessor property descriptors to module scope and define them with a single ObjectDefineProperties call, instead of allocating six closures and four descriptor objects per instance. - Stop assigning the history options onto the input stream. This avoids hidden class transitions on the user provided stream and no longer mutates it observably. - Only check process.env.TERM for a dumb terminal when the interface is in terminal mode. Reading process.env goes through the environment interceptor and is comparatively expensive, and _ttyWrite is never called when terminal is false. Signed-off-by: Matteo Collina <hello@matteocollina.com>
Collaborator
|
Review requested:
|
Replace the per-line regular expression exec loop with a single split pass, and use a plain string split on "\n" when the chunk contains none of the rare line endings (\r, \u2028, \u2029). This avoids allocating a match object and two index entries per line and lets the common case use the fast string split path. readline/readline-iterable.js type='new' improves by up to 80% and the event based line consumption by up to 65%. Signed-off-by: Matteo Collina <hello@matteocollina.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #64585 +/- ##
==========================================
- Coverage 90.13% 90.12% -0.02%
==========================================
Files 741 741
Lines 241976 241994 +18
Branches 45543 45551 +8
==========================================
- Hits 218116 218101 -15
- Misses 15379 15410 +31
- Partials 8481 8483 +2
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Speed up
readline.createInterface(), which is on the hot path of every CLI tool that reads lines from a stream.A CPU profile of interface construction showed three avoidable costs:
setupHistoryManager()accounted for ~43% of construction self time: every instance ran fourObjectDefinePropertycalls, allocating six fresh closures and four descriptor objects each time. The accessors (history,historyIndex,historySize,isFlushing) are now defined from a module-level shared descriptor map with a singleObjectDefinePropertiescall. They remain own, enumerable, configurable properties, so observable semantics are unchanged.size,historyandremoveHistoryDuplicatesonto the user-provided input stream to forward them to the history manager. It now passes a plain options object instead, so the user's stream is no longer observably mutated and doesn't go through extra hidden-class transitions. The legacy positional-arguments path is unchanged.Interfacewrapper unconditionally readprocess.env.TERM(which goes through the env interceptor) to bind the dumb-terminal_ttyWrite. The check is now gated onthis.terminal, since_ttyWriteis never invoked when the interface is not in terminal mode.Results with the new benchmark (30 runs, interleaved via
benchmark/compare.js):Update: a second commit speeds up line throughput as well.
[kNormalWrite]ran a regular-expressionexecloop that allocated a match object and two index entries per line, then sliced each line out in JS. It now does a single split pass, and uses a plain string split on"\n"when the chunk contains none of the rare line endings (\r,,), which takes V8's fast string-split path.Results for
readline/readline-iterable.js(20 runs, interleaved):Event-based consumption (
rl.on('line')) goes from 8.1M to 13.3M lines/s (+65%) in a microbenchmark of 32-byte lines.