Skip to content

Optimize loader string encoding routine #1913

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Optimize loader string encoding routine #1913

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
12 changes: 8 additions & 4 deletions lib/loader/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,14 @@ function postInstantiate(extendedExports, instance) {
/** Allocates a new string in the module's memory and returns its pointer. */
function __newString(str) {
if (str == null) return 0;
const length = str.length;
const ptr = __new(length << 1, STRING_ID);
const U16 = new Uint16Array(memory.buffer);
for (var i = 0, p = ptr >>> 1; i < length; ++i) U16[p + i] = str.charCodeAt(i);
const size = str.length << 1;
const ptr = __new(size, STRING_ID);
const buf = new Uint8Array(memory.buffer, ptr, size);
for (let i = 0; i < size; i += 2) {
let c = str.charCodeAt(i >>> 1);
buf[i ] = c;
buf[i + 1] = c >>> 8;
}
return ptr;
}

Expand Down
16 changes: 16 additions & 0 deletions lib/loader/tests/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,22 @@ function test(file) {
assert.strictEqual(exports.strlen(ref), str.length);
}

// should be able to allocate and work with a string containing an isolated lead surrogate
{
let str = "𝄞".substring(0, 1);
let ref = exports.__newString(str);
assert.strictEqual(exports.__getString(ref), str);
assert.strictEqual(exports.strlen(ref), str.length);
}

// should be able to allocate and work with a string containing an isolated trail surrogate
{
let str = "𝄞".substring(1);
let ref = exports.__newString(str);
assert.strictEqual(exports.__getString(ref), str);
assert.strictEqual(exports.strlen(ref), str.length);
}

// should be able to allocate a typed array
{
let arr = [1, 2, 3, 4, 5, 0x80000000 | 0];
Expand Down
12 changes: 8 additions & 4 deletions lib/loader/umd/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,13 +173,17 @@ var loader = (function(exports) {

function __newString(str) {
if (str == null) return 0;
const length = str.length;
const size = str.length << 1;

const ptr = __new(length << 1, STRING_ID);
const ptr = __new(size, STRING_ID);

const U16 = new Uint16Array(memory.buffer);
const buf = new Uint8Array(memory.buffer, ptr, size);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it really faster with ptr and size in constructor? When I last time checked it was less efficient than original approach


for (var i = 0, p = ptr >>> 1; i < length; ++i) U16[p + i] = str.charCodeAt(i);
for (let i = 0; i < size; i += 2) {
let c = str.charCodeAt(i >>> 1);
buf[i] = c;
buf[i + 1] = c >>> 8;
}

return ptr;
}
Expand Down