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

lib: add ASCII fast path to getStringWidth() #29301

Closed
wants to merge 4 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions benchmark/misc/getstringwidth.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
'use strict';

const common = require('../common.js');

const bench = common.createBenchmark(main, {
type: ['ascii', 'mixed', 'emojiseq', 'fullwidth'],
n: [10e4]
}, {
flags: ['--expose-internals']
});

function main({ n, type }) {
const { getStringWidth } = require('internal/readline/utils');

const str = ({
ascii: 'foobar'.repeat(100),
addaleax marked this conversation as resolved.
Show resolved Hide resolved
mixed: 'foo'.repeat(100) + '😀' + 'bar'.repeat(100),
emojiseq: '👨‍👨‍👧‍👦👨‍👩‍👦‍👦👨‍👩‍👧‍👧👩‍👩‍👧‍👦'.repeat(10),
fullwidth: '你好'.repeat(150)
})[type];

bench.start();
for (let j = 0; j < n; j += 1)
getStringWidth(str);
bench.end(n);
}
33 changes: 26 additions & 7 deletions lib/internal/readline/utils.js
Original file line number Diff line number Diff line change
@@ -34,13 +34,32 @@ if (internalBinding('config').hasIntl) {
const icu = internalBinding('icu');
getStringWidth = function getStringWidth(str, options) {
options = options || {};
if (!Number.isInteger(str))
str = stripVTControlCharacters(String(str));
return icu.getStringWidth(
str,
Boolean(options.ambiguousAsFullWidth),
Boolean(options.expandEmojiSequence)
);
if (Number.isInteger(str)) {
// Provide information about the character with code point 'str'.
return icu.getStringWidth(
str,
Boolean(options.ambiguousAsFullWidth),
false
);
bnoordhuis marked this conversation as resolved.
Show resolved Hide resolved
}
str = stripVTControlCharacters(String(str));
let width = 0;
for (let i = 0; i < str.length; i++) {
// Try to avoid calling into C++ by first handling the ASCII portion of
// the string. If it is fully ASCII, we skip the C++ part.
const code = str.charCodeAt(i);
if (code < 127) {
width += code >= 32;
bnoordhuis marked this conversation as resolved.
Show resolved Hide resolved
continue;
}
width += icu.getStringWidth(
str.slice(i),
Boolean(options.ambiguousAsFullWidth),
Boolean(options.expandEmojiSequence)
);
break;
}
return width;
};
isFullWidthCodePoint =
function isFullWidthCodePoint(code, options) {
22 changes: 22 additions & 0 deletions test/parallel/test-icu-stringwidth.js
Original file line number Diff line number Diff line change
@@ -69,3 +69,25 @@ assert.strictEqual(

// Control chars and combining chars are zero
assert.strictEqual(readline.getStringWidth('\u200E\n\u220A\u20D2'), 1);

// Test that the fast path for ASCII characters yields results consistent
// with the 'slow' path.
for (const ambiguousAsFullWidth of [ false, true ]) {
for (let i = 0; i < 256; i++) {
const char = String.fromCharCode(i);
assert.strictEqual(
readline.getStringWidth(i, { ambiguousAsFullWidth }),
readline.getStringWidth(char, { ambiguousAsFullWidth }));
assert.strictEqual(
readline.getStringWidth(char + '🎉', { ambiguousAsFullWidth }),
readline.getStringWidth(char, { ambiguousAsFullWidth }) + 2);

if (i < 32 || (i >= 127 && i < 160)) { // Control character
assert.strictEqual(
readline.getStringWidth(i, { ambiguousAsFullWidth }), 0);
} else if (i < 127) { // Regular ASCII character
assert.strictEqual(
readline.getStringWidth(i, { ambiguousAsFullWidth }), 1);
}
}
}