Skip to content

Commit da67a6c

Browse files
hryhoriiK97claude
andcommitted
Address review: no string wrapper, unretained objects, no stack copy
- Call convertNSStringToJSI<jsi::String> directly instead of convertNSStringToJSIString. - Read a container's objects into one vector and return it. - Don't retain the objects: the container holds them while they are converted. - Drop the stack copy of short strings. With it back, the benchmarks were within 1%, so it didn't help. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 618107c commit da67a6c

1 file changed

Lines changed: 19 additions & 47 deletions

File tree

  • packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon

‎packages/react-native/ReactCommon/react/nativemodule/core/platform/ios/ReactCommon/RCTTurboModule.mm‎

Lines changed: 19 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
#import <objc/runtime.h>
2828
#import <atomic>
2929
#import <iostream>
30-
#import <memory>
3130
#import <mutex>
3231

3332
#import <sstream>
@@ -72,9 +71,7 @@ static T convertNSStringToJSI(jsi::Runtime &runtime, __unsafe_unretained NSStrin
7271
return jsi::String::createFromAscii(runtime, ascii, length);
7372
}
7473
};
75-
auto fromUtf16 = [&](const UniChar *characters, size_t length) {
76-
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
77-
auto utf16 = reinterpret_cast<const char16_t *>(characters);
74+
auto fromUtf16 = [&](const char16_t *utf16, size_t length) {
7875
if constexpr (std::is_same_v<T, jsi::PropNameID>) {
7976
return jsi::PropNameID::forUtf16(runtime, utf16, length);
8077
} else {
@@ -94,56 +91,31 @@ static T convertNSStringToJSI(jsi::Runtime &runtime, __unsafe_unretained NSStrin
9491
return fromAscii(ascii, static_cast<size_t>(length));
9592
}
9693
if (const UniChar *characters = CFStringGetCharactersPtr(string)) {
97-
return fromUtf16(characters, static_cast<size_t>(length));
98-
}
99-
// Short strings, such as most keys, are often stored in the pointer itself, so copy them to the stack.
100-
constexpr CFIndex kStackLength = 64;
101-
if (length <= kStackLength) {
102-
UInt8 ascii[kStackLength];
103-
CFIndex asciiLength = 0;
104-
if (CFStringGetBytes(
105-
string, CFRangeMake(0, length), kCFStringEncodingASCII, 0, false, ascii, length, &asciiLength) == length) {
106-
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
107-
return fromAscii(reinterpret_cast<const char *>(ascii), static_cast<size_t>(asciiLength));
108-
}
109-
UniChar characters[kStackLength];
110-
CFStringGetCharacters(string, CFRangeMake(0, length), characters);
111-
return fromUtf16(characters, static_cast<size_t>(length));
94+
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
95+
return fromUtf16(reinterpret_cast<const char16_t *>(characters), static_cast<size_t>(length));
11296
}
113-
std::unique_ptr<UniChar[]> characters(new UniChar[length]);
114-
CFStringGetCharacters(string, CFRangeMake(0, length), characters.get());
115-
return fromUtf16(characters.get(), static_cast<size_t>(length));
116-
}
117-
118-
static jsi::String convertNSStringToJSIString(jsi::Runtime &runtime, NSString *value)
119-
{
120-
return convertNSStringToJSI<jsi::String>(runtime, value);
97+
std::u16string utf16(length, u'\0');
98+
// NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast)
99+
CFStringGetCharacters(string, CFRangeMake(0, length), reinterpret_cast<UniChar *>(utf16.data()));
100+
return fromUtf16(utf16.data(), utf16.size());
121101
}
122102

123103
// Doesn't retain `value`: the caller holds it, and containers hold what they contain while it is converted.
124104
static jsi::Value convertUnretainedObjCObjectToJSIValue(jsi::Runtime &runtime, __unsafe_unretained id value);
125105

126-
// Reads a container's objects at once and holds them while they are converted, because JS code
127-
// that runs while properties are set could change the container.
106+
// Reads a container's objects at once. They aren't retained, as the container holds them.
128107
template <typename ReadObjects>
129-
static std::vector<id> getRetainedObjects(NSUInteger count, ReadObjects readObjects)
108+
static std::vector<__unsafe_unretained id> getObjects(NSUInteger count, ReadObjects readObjects)
130109
{
131-
constexpr NSUInteger kStackCount = 32;
132-
__unsafe_unretained id stackObjects[kStackCount];
133-
std::vector<__unsafe_unretained id> heapObjects;
134-
__unsafe_unretained id *objects = stackObjects;
135-
if (count > kStackCount) {
136-
heapObjects.resize(count);
137-
objects = heapObjects.data();
138-
}
139-
readObjects(objects);
140-
return std::vector<id>(objects, objects + count);
110+
std::vector<__unsafe_unretained id> objects(count);
111+
readObjects(objects.data());
112+
return objects;
141113
}
142114

143115
static jsi::Object convertNSDictionaryToJSIObject(jsi::Runtime &runtime, __unsafe_unretained NSDictionary *value)
144116
{
145117
NSUInteger count = value.count;
146-
std::vector<id> keysAndObjects = getRetainedObjects(count * 2, [&](__unsafe_unretained id *objects) {
118+
std::vector<__unsafe_unretained id> keysAndObjects = getObjects(count * 2, [&](__unsafe_unretained id *objects) {
147119
[value getObjects:objects + count andKeys:objects count:count];
148120
});
149121
jsi::Object result = jsi::Object(runtime);
@@ -156,16 +128,16 @@ static T convertNSStringToJSI(jsi::Runtime &runtime, __unsafe_unretained NSStrin
156128
return result;
157129
}
158130

159-
static std::vector<id> getRetainedObjects(__unsafe_unretained NSArray *value)
131+
static std::vector<__unsafe_unretained id> getObjects(__unsafe_unretained NSArray *value)
160132
{
161133
NSUInteger count = value.count;
162-
return getRetainedObjects(
134+
return getObjects(
163135
count, [&](__unsafe_unretained id *objects) { [value getObjects:objects range:NSMakeRange(0, count)]; });
164136
}
165137

166138
static jsi::Array convertNSArrayToJSIArray(jsi::Runtime &runtime, __unsafe_unretained NSArray *value)
167139
{
168-
std::vector<id> objects = getRetainedObjects(value);
140+
std::vector<__unsafe_unretained id> objects = getObjects(value);
169141
jsi::Array result = jsi::Array(runtime, objects.size());
170142
for (size_t i = 0; i < objects.size(); i++) {
171143
result.setValueAtIndex(runtime, i, convertUnretainedObjCObjectToJSIValue(runtime, objects[i]));
@@ -175,10 +147,10 @@ static T convertNSStringToJSI(jsi::Runtime &runtime, __unsafe_unretained NSStrin
175147

176148
static std::vector<jsi::Value> convertNSArrayToStdVector(jsi::Runtime &runtime, __unsafe_unretained NSArray *value)
177149
{
178-
std::vector<id> objects = getRetainedObjects(value);
150+
std::vector<__unsafe_unretained id> objects = getObjects(value);
179151
std::vector<jsi::Value> result;
180152
result.reserve(objects.size());
181-
for (id object : objects) {
153+
for (__unsafe_unretained id object : objects) {
182154
result.emplace_back(convertUnretainedObjCObjectToJSIValue(runtime, object));
183155
}
184156
return result;
@@ -716,7 +688,7 @@ TraceSection s(
716688
break;
717689
}
718690
case StringKind: {
719-
returnValue = convertNSStringToJSIString(runtime, (NSString *)result);
691+
returnValue = convertNSStringToJSI<jsi::String>(runtime, (NSString *)result);
720692
break;
721693
}
722694
case ObjectKind: {

0 commit comments

Comments
 (0)