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.

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

Jump to bottom

node-api: refactor napi_set_property function for improved performance #50282

Closed
wants to merge 1 commit into from
Closed

node-api: refactor napi_set_property function for improved performance #50282

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
44 changes: 44 additions & 0 deletions doc/api/n-api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3000,6 +3000,50 @@ The native string is copied.
The JavaScript `string` type is described in
[Section 6.1.4][] of the ECMAScript Language Specification.

#### `node_api_create_property_key_utf16`

<!-- YAML
added: REPLACEME
-->

> Stability: 1 - Experimental

```c
napi_status NAPI_CDECL node_api_create_property_key_utf16(napi_env env,
const char16_t* str,
size_t length,
napi_value* result);
```

* `[in] env`: The environment that the API is invoked under.
* `[in] str`: Character buffer representing a UTF16-LE-encoded string.
* `[in] length`: The length of the string in two-byte code units, or
`NAPI_AUTO_LENGTH` if it is null-terminated.
* `[out] result`: A `napi_value` representing an optimized JavaScript `string`
to be used as a property key for objects.

Returns `napi_ok` if the API succeeded.

This API creates an optimized JavaScript `string` value from
a UTF16-LE-encoded C string to be used as a property key for objects.
The native string is copied.

Many JavaScript engines including V8 use internalized strings as keys
to set and get property values. They typically use a hash table to create
and lookup such strings. While it adds some cost per key creation, it improves
the performance after that by enabling comparison of string pointers instead
of the whole strings.

If a new JavaScript string is intended to be used as a property key, then for
some JavaScript engines it will be more efficient to use
the `node_api_create_property_key_utf16` function.
Otherwise, use the `napi_create_string_utf16` or
`node_api_create_external_string_utf16` functions as there may be additional
overhead in creating/storing strings with this method.

The JavaScript `string` type is described in
[Section 6.1.4][] of the ECMAScript Language Specification.

### Functions to convert from Node-API to C types

#### `napi_get_array_length`
Expand Down
7 changes: 7 additions & 0 deletions src/js_native_api.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,13 @@ node_api_create_external_string_utf16(napi_env env,
napi_value* result,
bool* copied);
#endif // NAPI_EXPERIMENTAL

#ifdef NAPI_EXPERIMENTAL
#define NODE_API_EXPERIMENTAL_HAS_PROPERTY_KEYS
NAPI_EXTERN napi_status NAPI_CDECL node_api_create_property_key_utf16(
napi_env env, const char16_t* str, size_t length, napi_value* result);
#endif // NAPI_EXPERIMENTAL

NAPI_EXTERN napi_status NAPI_CDECL napi_create_symbol(napi_env env,
napi_value description,
napi_value* result);
Expand Down
12 changes: 12 additions & 0 deletions src/js_native_api_v8.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1651,6 +1651,18 @@ node_api_create_external_string_utf16(napi_env env,
});
}

napi_status NAPI_CDECL node_api_create_property_key_utf16(napi_env env,
const char16_t* str,
size_t length,
napi_value* result) {
return v8impl::NewString(env, str, length, result, [&](v8::Isolate* isolate) {
return v8::String::NewFromTwoByte(isolate,
reinterpret_cast<const uint16_t*>(str),
v8::NewStringType::kInternalized,
static_cast<int>(length));
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to cast to int here? We don't cast to int in the regular version of this function.

Copy link
Member Author

@mertcanaltin mertcanaltin Dec 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Hello, I created this space based on the recommendation of @vmoroz, as mentioned in pull request #48339.

Copy link
Member

Choose a reason for hiding this comment

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

@gabrielschulhof , we are getting warnings in MSVC compiler if types are mismatch.
The NewFromTwoByte expects type int which is signed 32-bit integer.
In this function length is size_t which is unsigned 64-bit integer for 64 platforms.
MSVC warns about possible precision loss when the cast implicitly here.
The explicit cast suppresses the warning as we explicitly express our intent.

Ideally, we must add the cast in other places too to avoid the MSVC warnings.
But it should be another PR since currently Node.js has a lot of MSVC warnings anyway.

});
}

napi_status NAPI_CDECL napi_create_double(napi_env env,
double value,
napi_value* result) {
Expand Down
14 changes: 14 additions & 0 deletions test/js-native-api/test_string/test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@ assert.strictEqual(test_string.TestLatin1External(empty), empty);
assert.strictEqual(test_string.TestUtf16External(empty), empty);
assert.strictEqual(test_string.TestLatin1ExternalAutoLength(empty), empty);
assert.strictEqual(test_string.TestUtf16ExternalAutoLength(empty), empty);
assert.strictEqual(test_string.TestPropertyKeyUtf16(empty), empty);
assert.strictEqual(test_string.TestPropertyKeyUtf16AutoLength(empty), empty);
assert.strictEqual(test_string.Utf16Length(empty), 0);
assert.strictEqual(test_string.Utf8Length(empty), 0);

Expand All @@ -33,6 +35,8 @@ assert.strictEqual(test_string.TestUtf16ExternalAutoLength(str1), str1);
assert.strictEqual(test_string.TestLatin1Insufficient(str1), str1.slice(0, 3));
assert.strictEqual(test_string.TestUtf8Insufficient(str1), str1.slice(0, 3));
assert.strictEqual(test_string.TestUtf16Insufficient(str1), str1.slice(0, 3));
assert.strictEqual(test_string.TestPropertyKeyUtf16(str1), str1);
assert.strictEqual(test_string.TestPropertyKeyUtf16AutoLength(str1), str1);
assert.strictEqual(test_string.Utf16Length(str1), 11);
assert.strictEqual(test_string.Utf8Length(str1), 11);

Expand All @@ -50,6 +54,8 @@ assert.strictEqual(test_string.TestUtf16ExternalAutoLength(str2), str2);
assert.strictEqual(test_string.TestLatin1Insufficient(str2), str2.slice(0, 3));
assert.strictEqual(test_string.TestUtf8Insufficient(str2), str2.slice(0, 3));
assert.strictEqual(test_string.TestUtf16Insufficient(str2), str2.slice(0, 3));
assert.strictEqual(test_string.TestPropertyKeyUtf16(str2), str2);
assert.strictEqual(test_string.TestPropertyKeyUtf16AutoLength(str2), str2);
assert.strictEqual(test_string.Utf16Length(str2), 62);
assert.strictEqual(test_string.Utf8Length(str2), 62);

Expand All @@ -67,6 +73,8 @@ assert.strictEqual(test_string.TestUtf16ExternalAutoLength(str3), str3);
assert.strictEqual(test_string.TestLatin1Insufficient(str3), str3.slice(0, 3));
assert.strictEqual(test_string.TestUtf8Insufficient(str3), str3.slice(0, 3));
assert.strictEqual(test_string.TestUtf16Insufficient(str3), str3.slice(0, 3));
assert.strictEqual(test_string.TestPropertyKeyUtf16(str3), str3);
assert.strictEqual(test_string.TestPropertyKeyUtf16AutoLength(str3), str3);
assert.strictEqual(test_string.Utf16Length(str3), 27);
assert.strictEqual(test_string.Utf8Length(str3), 27);

Expand All @@ -84,6 +92,8 @@ assert.strictEqual(test_string.TestUtf16ExternalAutoLength(str4), str4);
assert.strictEqual(test_string.TestLatin1Insufficient(str4), str4.slice(0, 3));
assert.strictEqual(test_string.TestUtf8Insufficient(str4), str4.slice(0, 1));
assert.strictEqual(test_string.TestUtf16Insufficient(str4), str4.slice(0, 3));
assert.strictEqual(test_string.TestPropertyKeyUtf16(str4), str4);
assert.strictEqual(test_string.TestPropertyKeyUtf16AutoLength(str4), str4);
assert.strictEqual(test_string.Utf16Length(str4), 31);
assert.strictEqual(test_string.Utf8Length(str4), 62);

Expand All @@ -101,6 +111,8 @@ assert.strictEqual(test_string.TestUtf16ExternalAutoLength(str5), str5);
assert.strictEqual(test_string.TestLatin1Insufficient(str5), str5.slice(0, 3));
assert.strictEqual(test_string.TestUtf8Insufficient(str5), str5.slice(0, 1));
assert.strictEqual(test_string.TestUtf16Insufficient(str5), str5.slice(0, 3));
assert.strictEqual(test_string.TestPropertyKeyUtf16(str5), str5);
assert.strictEqual(test_string.TestPropertyKeyUtf16AutoLength(str5), str5);
assert.strictEqual(test_string.Utf16Length(str5), 63);
assert.strictEqual(test_string.Utf8Length(str5), 126);

Expand All @@ -113,6 +125,8 @@ assert.strictEqual(test_string.TestUtf16External(str6), str6);
assert.strictEqual(test_string.TestUtf16ExternalAutoLength(str6), str6);
assert.strictEqual(test_string.TestUtf8Insufficient(str6), str6.slice(0, 1));
assert.strictEqual(test_string.TestUtf16Insufficient(str6), str6.slice(0, 3));
assert.strictEqual(test_string.TestPropertyKeyUtf16(str6), str6);
assert.strictEqual(test_string.TestPropertyKeyUtf16AutoLength(str6), str6);
assert.strictEqual(test_string.Utf16Length(str6), 5);
assert.strictEqual(test_string.Utf8Length(str6), 14);

Expand Down
20 changes: 20 additions & 0 deletions test/js-native-api/test_string/test_string.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,23 @@ static napi_value TestUtf16Insufficient(napi_env env, napi_callback_info info) {
return output;
}

static napi_value TestPropertyKeyUtf16(napi_env env, napi_callback_info info) {
return TestTwoByteImpl(env,
info,
napi_get_value_string_utf16,
node_api_create_property_key_utf16,
actual_length);
}

static napi_value TestPropertyKeyUtf16AutoLength(napi_env env,
napi_callback_info info) {
return TestTwoByteImpl(env,
info,
napi_get_value_string_utf16,
node_api_create_property_key_utf16,
auto_length);
}

static napi_value Utf16Length(napi_env env, napi_callback_info info) {
napi_value args[1];
NODE_API_CALL(env, validate_and_retrieve_single_string_arg(env, info, args));
Expand Down Expand Up @@ -410,6 +427,9 @@ napi_value Init(napi_env env, napi_value exports) {
DECLARE_NODE_API_PROPERTY("TestLargeLatin1", TestLargeLatin1),
DECLARE_NODE_API_PROPERTY("TestLargeUtf16", TestLargeUtf16),
DECLARE_NODE_API_PROPERTY("TestMemoryCorruption", TestMemoryCorruption),
DECLARE_NODE_API_PROPERTY("TestPropertyKeyUtf16", TestPropertyKeyUtf16),
DECLARE_NODE_API_PROPERTY("TestPropertyKeyUtf16AutoLength",
TestPropertyKeyUtf16AutoLength),
};

init_test_null(env, exports);
Expand Down