Skip to content

Commit

Permalink
test: set test-structuredclone-* as flaky
Browse files Browse the repository at this point in the history
PR-URL: nodejs#50261
Refs: nodejs#50260
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Vinícius Lourenço Claro Cardoso <contact@viniciusl.com.br>

meta: move Trott to TSC regular member

At the current time, I'm not able to give Node.js the time and attention
it needs and deserves from a voting TSC member. I'm proud of the work
and efforts I've made as a TSC voting member, and I want to leave before
I'm less happy with my efforts. Thanks for all the trust and good will
over the years.

PR-URL: nodejs#50297
Reviewed-By: Tierney Cyren <hello@bnb.im>
Reviewed-By: Richard Lau <rlau@redhat.com>

node-api: refactor napi set property function for improved performance

fix: lint

fix: lint

refactor: Improve performance by using internalized property keys

refactor: Update node_api_create_property_key_utf16 signature and remove napi_set_property_utf16

lint

lint

fix: Resolve compilation error in node_api_create_property_key_utf16

fix: Resolve type conversion error in node_api_create_property_key_utf16

refactor: Simplify node_api_create_property_key_utf16 implementation

lint

add node_api_create_property_key_utf16 property name

added doc for node_api_create_property_key_utf16

lint

lint

update napi doc for node_api_create_property_key_utf16

update: code snipet

test: added test for node_api_create_property_key_utf16

lint

lint

lint

call node_api_create_property_key_utf16

Update doc/api/n-api.md

Co-authored-by: Chengzhong Wu <legendecas@gmail.com>

Update test/js-native-api/test_string/test_string.c

Co-authored-by: Chengzhong Wu <legendecas@gmail.com>

Update test/js-native-api/test_string/test_string.c

Co-authored-by: Chengzhong Wu <legendecas@gmail.com>

Update doc/api/n-api.md

Co-authored-by: Chengzhong Wu <legendecas@gmail.com>

added TestPropertyKeyUtf16 napi_property_descriptor

lint doc

lint cpp

minor updates to PR nodejs#50282

feat: removed not used function for test

Update doc/api/n-api.md

Co-authored-by: Michael Dawson <mdawson@devrus.com>

Update doc/api/n-api.md

Co-authored-by: Michael Dawson <mdawson@devrus.com>
  • Loading branch information
2 people authored and mertcanaltin committed Dec 19, 2023
1 parent e45e73b commit df357c8
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 2 deletions.
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,6 @@ For information about the governance of the Node.js project, see
**Michaël Zasso** <<targos@protonmail.com>> (he/him)
* [tniessen](https://github.com/tniessen) -
**Tobias Nießen** <<tniessen@tnie.de>> (he/him)
* [Trott](https://github.com/Trott) -
**Rich Trott** <<rtrott@gmail.com>> (he/him)

#### TSC regular members

Expand Down Expand Up @@ -233,6 +231,8 @@ For information about the governance of the Node.js project, see
**Rod Vagg** <<r@va.gg>>
* [TimothyGu](https://github.com/TimothyGu) -
**Tiancheng "Timothy" Gu** <<timothygu99@gmail.com>> (he/him)
* [Trott](https://github.com/Trott) -
**Rich Trott** <<rtrott@gmail.com>> (he/him)

<details>

Expand Down
44 changes: 44 additions & 0 deletions doc/api/n-api.md
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
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
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));
});
}

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
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
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
2 changes: 2 additions & 0 deletions test/pummel/pummel.status
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ prefix pummel
[$system==win32]
# https://github.com/nodejs/node/issues/40728
test-fs-watch-non-recursive: PASS,FLAKY
# https://github.com/nodejs/node/issues/50260
test-structuredclone-jstransferable: PASS,FLAKY

[$system==linux]
# https://github.com/nodejs/node/issues/38226
Expand Down

0 comments on commit df357c8

Please sign in to comment.