Skip to content

Commit be1abb2

Browse files
DjDeveloperrDjDeveloperr
committed
fix(hermes): stabilize native callback dispatch
1 parent de6be51 commit be1abb2

17 files changed

Lines changed: 541 additions & 432 deletions

‎NativeScript/CMakeLists.txt‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -409,20 +409,32 @@ elseif(TARGET_ENGINE_JSC)
409409
target_compile_definitions(${NAME} PRIVATE TARGET_ENGINE_JSC)
410410
endif()
411411

412+
set(NS_GSD_BACKEND_V8_VALUE 0)
413+
set(NS_GSD_BACKEND_JSC_VALUE 0)
414+
set(NS_GSD_BACKEND_QUICKJS_VALUE 0)
415+
set(NS_GSD_BACKEND_HERMES_VALUE 0)
416+
set(NS_GSD_BACKEND_NAPI_VALUE 0)
417+
412418
if(NS_EFFECTIVE_GSD_BACKEND STREQUAL "v8")
413-
target_compile_definitions(${NAME} PRIVATE NS_GSD_BACKEND_V8=1 NS_GSD_BACKEND_JSC=0 NS_GSD_BACKEND_QUICKJS=0 NS_GSD_BACKEND_HERMES=0 NS_GSD_BACKEND_NAPI=0)
419+
set(NS_GSD_BACKEND_V8_VALUE 1)
414420
elseif(NS_EFFECTIVE_GSD_BACKEND STREQUAL "jsc")
415-
target_compile_definitions(${NAME} PRIVATE NS_GSD_BACKEND_V8=0 NS_GSD_BACKEND_JSC=1 NS_GSD_BACKEND_QUICKJS=0 NS_GSD_BACKEND_HERMES=0 NS_GSD_BACKEND_NAPI=0)
421+
set(NS_GSD_BACKEND_JSC_VALUE 1)
416422
elseif(NS_EFFECTIVE_GSD_BACKEND STREQUAL "quickjs")
417-
target_compile_definitions(${NAME} PRIVATE NS_GSD_BACKEND_V8=0 NS_GSD_BACKEND_JSC=0 NS_GSD_BACKEND_QUICKJS=1 NS_GSD_BACKEND_HERMES=0 NS_GSD_BACKEND_NAPI=0)
423+
set(NS_GSD_BACKEND_QUICKJS_VALUE 1)
418424
elseif(NS_EFFECTIVE_GSD_BACKEND STREQUAL "hermes")
419-
target_compile_definitions(${NAME} PRIVATE NS_GSD_BACKEND_V8=0 NS_GSD_BACKEND_JSC=0 NS_GSD_BACKEND_QUICKJS=0 NS_GSD_BACKEND_HERMES=1 NS_GSD_BACKEND_NAPI=0)
425+
set(NS_GSD_BACKEND_HERMES_VALUE 1)
420426
elseif(NS_EFFECTIVE_GSD_BACKEND STREQUAL "napi")
421-
target_compile_definitions(${NAME} PRIVATE NS_GSD_BACKEND_V8=0 NS_GSD_BACKEND_JSC=0 NS_GSD_BACKEND_QUICKJS=0 NS_GSD_BACKEND_HERMES=0 NS_GSD_BACKEND_NAPI=1)
422-
else()
423-
target_compile_definitions(${NAME} PRIVATE NS_GSD_BACKEND_V8=0 NS_GSD_BACKEND_JSC=0 NS_GSD_BACKEND_QUICKJS=0 NS_GSD_BACKEND_HERMES=0 NS_GSD_BACKEND_NAPI=0)
427+
set(NS_GSD_BACKEND_NAPI_VALUE 1)
424428
endif()
425429

430+
target_compile_definitions(${NAME} PRIVATE
431+
NS_GSD_BACKEND_V8=${NS_GSD_BACKEND_V8_VALUE}
432+
NS_GSD_BACKEND_JSC=${NS_GSD_BACKEND_JSC_VALUE}
433+
NS_GSD_BACKEND_QUICKJS=${NS_GSD_BACKEND_QUICKJS_VALUE}
434+
NS_GSD_BACKEND_HERMES=${NS_GSD_BACKEND_HERMES_VALUE}
435+
NS_GSD_BACKEND_NAPI=${NS_GSD_BACKEND_NAPI_VALUE}
436+
)
437+
426438
set(FRAMEWORK_VERSION_VALUE "${VERSION}")
427439
if(TARGET_PLATFORM_MACOS)
428440
# macOS framework consumers (including Xcode's copy/sign phases) expect

‎NativeScript/ffi/CFunction.mm‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,9 @@ inline void ensureCFunctionDispatchLookup(CFunction* function, Cif* cif) {
344344
auto preparedInvoker = reinterpret_cast<CFunctionPreparedInvoker>(func->preparedInvoker);
345345
auto napiInvoker = reinterpret_cast<CFunctionNapiInvoker>(func->napiInvoker);
346346
auto engineDirectInvoker =
347-
reinterpret_cast<CFunctionEngineDirectInvoker>(func->engineDirectInvoker);
347+
!cif->skipGeneratedNapiDispatch
348+
? reinterpret_cast<CFunctionEngineDirectInvoker>(func->engineDirectInvoker)
349+
: nullptr;
348350

349351
MDFunctionFlag functionFlags =
350352
bridgeState->metadata->getFunctionFlag(offset + sizeof(MDSectionOffset) * 2);

‎NativeScript/ffi/CallbackThreading.h‎

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -21,26 +21,27 @@ inline thread_local int native_caller_thread_callback_depth = 0;
2121

2222
} // namespace detail
2323

24-
inline bool shouldInvokeCallbackOnNativeCallerThread() {
24+
inline bool isNativeCallRuntimeUnlockedForCallbacks() {
2525
#if defined(ENABLE_JS_RUNTIME) && defined(TARGET_ENGINE_HERMES)
26-
return true;
26+
return detail::native_call_unlocked_runtime_count.load(
27+
std::memory_order_acquire) > 0;
2728
#else
2829
return false;
2930
#endif
3031
}
3132

32-
inline bool isNativeCallRuntimeUnlockedForCallbacks() {
33+
inline bool isNativeCallerThreadCallbackActive() {
3334
#if defined(ENABLE_JS_RUNTIME) && defined(TARGET_ENGINE_HERMES)
34-
return detail::native_call_unlocked_runtime_count.load(
35-
std::memory_order_acquire) > 0;
35+
return detail::native_caller_thread_callback_depth > 0;
3636
#else
3737
return false;
3838
#endif
3939
}
4040

41-
inline bool isNativeCallerThreadCallbackActive() {
41+
inline bool shouldInvokeCallbackOnNativeCallerThread() {
4242
#if defined(ENABLE_JS_RUNTIME) && defined(TARGET_ENGINE_HERMES)
43-
return detail::native_caller_thread_callback_depth > 0;
43+
return isNativeCallRuntimeUnlockedForCallbacks() ||
44+
isNativeCallerThreadCallbackActive();
4445
#else
4546
return false;
4647
#endif
@@ -114,15 +115,19 @@ class NativeCallbackScope final {
114115
public:
115116
explicit NativeCallbackScope(napi_env env) : env_(env) {
116117
#if defined(ENABLE_JS_RUNTIME) && defined(TARGET_ENGINE_HERMES)
117-
if (isNativeCallRuntimeUnlockedForCallbacks()) {
118-
auto it = JSR::env_to_jsr_cache.find(env_);
119-
if (it != JSR::env_to_jsr_cache.end() && it->second != nullptr) {
120-
jsr_ = it->second;
121-
jsr_->js_mutex.lock();
122-
detail::native_caller_thread_callback_depth += 1;
123-
napi_open_handle_scope(env_, &napiHandleScope_);
124-
return;
125-
}
118+
if (isNativeCallerThreadCallbackActive() ||
119+
js_current_env_lock_depth(env_) > 0) {
120+
napi_open_handle_scope(env_, &napiHandleScope_);
121+
return;
122+
}
123+
124+
auto it = JSR::env_to_jsr_cache.find(env_);
125+
if (it != JSR::env_to_jsr_cache.end() && it->second != nullptr) {
126+
jsr_ = it->second;
127+
jsr_->lock();
128+
detail::native_caller_thread_callback_depth += 1;
129+
napi_open_handle_scope(env_, &napiHandleScope_);
130+
return;
126131
}
127132
#endif
128133
#if defined(ENABLE_JS_RUNTIME)
@@ -139,7 +144,7 @@ class NativeCallbackScope final {
139144
}
140145
if (jsr_ != nullptr) {
141146
detail::native_caller_thread_callback_depth -= 1;
142-
jsr_->js_mutex.unlock();
147+
jsr_->unlock();
143148
}
144149
#endif
145150
}

‎NativeScript/ffi/Cif.mm‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,10 @@ inline bool typeRequiresSlowGeneratedNapiDispatch(const std::shared_ptr<TypeConv
6464
switch (type->kind) {
6565
case mdTypeUChar:
6666
case mdTypeUInt8:
67+
case mdTypeString:
68+
case mdTypePointer:
69+
case mdTypeStruct:
70+
case mdTypeArray:
6771
case mdTypeBlock:
6872
case mdTypeFunctionPointer:
6973
case mdTypeVector:

‎NativeScript/ffi/ClassMember.mm‎

Lines changed: 22 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -363,11 +363,14 @@ inline bool tryObjCNapiDispatch(napi_env env, Cif* cif, id self, bool classMetho
363363
}
364364
}
365365

366-
ObjCEngineDirectInvoker engineInvoker =
367-
descriptor != nullptr
368-
? reinterpret_cast<ObjCEngineDirectInvoker>(descriptor->engineDirectInvoker)
369-
: lookupObjCEngineDirectInvoker(composeSignatureDispatchId(
370-
cif->signatureHash, SignatureCallKind::ObjCMethod, dispatchFlags));
366+
ObjCEngineDirectInvoker engineInvoker = nullptr;
367+
if (!cif->skipGeneratedNapiDispatch) {
368+
engineInvoker =
369+
descriptor != nullptr
370+
? reinterpret_cast<ObjCEngineDirectInvoker>(descriptor->engineDirectInvoker)
371+
: lookupObjCEngineDirectInvoker(composeSignatureDispatchId(
372+
cif->signatureHash, SignatureCallKind::ObjCMethod, dispatchFlags));
373+
}
371374
ObjCNapiInvoker invoker =
372375
engineInvoker == nullptr && !cif->skipGeneratedNapiDispatch
373376
? (descriptor != nullptr
@@ -1474,23 +1477,6 @@ explicit CifReturnStorage(Cif* cif) {
14741477
if (handledDirect) {
14751478
return directResult;
14761479
}
1477-
1478-
napi_value stackArgs[16];
1479-
if (actualArgc <= 16) {
1480-
for (size_t i = 0; i < actualArgc; i++) {
1481-
stackArgs[i] = HermesFastArg(fastInfo, i);
1482-
}
1483-
1484-
return jsCallDirect(env, static_cast<ObjCClassMember*>(HermesFastData(fastInfo)),
1485-
HermesFastThisArg(fastInfo), actualArgc, stackArgs);
1486-
}
1487-
1488-
std::vector<napi_value> heapArgs(actualArgc);
1489-
for (size_t i = 0; i < actualArgc; i++) {
1490-
heapArgs[i] = HermesFastArg(fastInfo, i);
1491-
}
1492-
return jsCallDirect(env, static_cast<ObjCClassMember*>(HermesFastData(fastInfo)),
1493-
HermesFastThisArg(fastInfo), actualArgc, heapArgs.data());
14941480
}
14951481
#endif
14961482

@@ -1567,6 +1553,20 @@ explicit CifReturnStorage(Cif* cif) {
15671553
method->cif = selectedCif;
15681554
}
15691555

1556+
if (selectedCif != nullptr && !selectedCif->isVariadic &&
1557+
selectedCif->argc != actualArgc) {
1558+
Method runtimeMethod = receiverIsClass
1559+
? class_getClassMethod(receiverClass, selectedMethod->selector)
1560+
: class_getInstanceMethod(receiverClass, selectedMethod->selector);
1561+
if (runtimeMethod != nullptr) {
1562+
Cif* runtimeCif = method->bridgeState->getMethodCif(env, runtimeMethod);
1563+
if (runtimeCif != nullptr && runtimeCif->argc == actualArgc) {
1564+
selectedCif = runtimeCif;
1565+
method->cif = selectedCif;
1566+
}
1567+
}
1568+
}
1569+
15701570
if (!method->overloads.empty()) {
15711571
struct Candidate {
15721572
MethodDescriptor* descriptor;
@@ -1871,8 +1871,6 @@ NativeScriptException nativeScriptException(errorMessage != nullptr ? errorMessa
18711871
if (handledDirect) {
18721872
return directResult;
18731873
}
1874-
return jsGetterDirect(env, static_cast<ObjCClassMember*>(HermesFastData(fastInfo)),
1875-
HermesFastThisArg(fastInfo));
18761874
}
18771875
#endif
18781876

@@ -1986,15 +1984,6 @@ NativeScriptException nativeScriptException(errorMessage != nullptr ? errorMessa
19861984
if (handledDirect) {
19871985
return directResult;
19881986
}
1989-
1990-
napi_value value = nullptr;
1991-
if (actualArgc > 0) {
1992-
value = HermesFastArg(fastInfo, 0);
1993-
} else {
1994-
napi_get_undefined(env, &value);
1995-
}
1996-
return jsSetterDirect(env, static_cast<ObjCClassMember*>(HermesFastData(fastInfo)),
1997-
HermesFastThisArg(fastInfo), value);
19981987
}
19991988
#endif
20001989

‎NativeScript/ffi/HermesFastNativeApi.mm‎

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -525,6 +525,50 @@ inline bool needsRoundTripCacheFrame(Cif* cif) {
525525
return cif != nullptr && cif->generatedDispatchHasRoundTripCacheArgument;
526526
}
527527

528+
inline bool canUseHermesFrameArgument(MDTypeKind kind) {
529+
switch (kind) {
530+
case mdTypeBool:
531+
case mdTypeChar:
532+
case mdTypeUChar:
533+
case mdTypeUInt8:
534+
case mdTypeSShort:
535+
case mdTypeUShort:
536+
case mdTypeSInt:
537+
case mdTypeUInt:
538+
case mdTypeSLong:
539+
case mdTypeULong:
540+
case mdTypeSInt64:
541+
case mdTypeUInt64:
542+
case mdTypeFloat:
543+
case mdTypeDouble:
544+
case mdTypeAnyObject:
545+
case mdTypeProtocolObject:
546+
case mdTypeClassObject:
547+
case mdTypeInstanceObject:
548+
case mdTypeNSStringObject:
549+
case mdTypeNSMutableStringObject:
550+
case mdTypeClass:
551+
case mdTypeSelector:
552+
return true;
553+
default:
554+
return false;
555+
}
556+
}
557+
558+
inline bool canUseHermesFrameArguments(Cif* cif) {
559+
if (cif == nullptr) {
560+
return false;
561+
}
562+
563+
for (const auto& argType : cif->argTypes) {
564+
if (argType == nullptr || !canUseHermesFrameArgument(argType->kind)) {
565+
return false;
566+
}
567+
}
568+
569+
return true;
570+
}
571+
528572
class HermesFastRoundTripCacheFrameGuard {
529573
public:
530574
HermesFastRoundTripCacheFrameGuard(napi_env env, ObjCBridgeState* bridgeState,
@@ -1952,6 +1996,10 @@ napi_value TryCallHermesObjCMemberFastImpl(
19521996
return nullptr;
19531997
}
19541998

1999+
if (hermesArgsBase != nullptr && !canUseHermesFrameArguments(cif)) {
2000+
return nullptr;
2001+
}
2002+
19552003
if (isNSErrorOutMethodSignature(descriptor, cif)) {
19562004
if (!cif->isVariadic &&
19572005
(actualArgc > cif->argc || actualArgc + 1 < cif->argc)) {
@@ -1963,6 +2011,10 @@ napi_value TryCallHermesObjCMemberFastImpl(
19632011
return nullptr;
19642012
}
19652013

2014+
if (!cif->isVariadic && actualArgc != cif->argc) {
2015+
return nullptr;
2016+
}
2017+
19662018
if (isBlockFallbackSelector(descriptor)) {
19672019
return nullptr;
19682020
}
@@ -2118,7 +2170,7 @@ HermesFastRoundTripCacheFrameGuard roundTripCacheFrame(
21182170
env, member->bridgeState, needsRoundTripFrame);
21192171

21202172
ObjCEngineDirectInvoker invoker =
2121-
cif->signatureHash != 0
2173+
!cif->skipGeneratedNapiDispatch && cif->signatureHash != 0
21222174
? ensureHermesObjCEngineDirectInvoker(cif, descriptor,
21232175
descriptor->dispatchFlags)
21242176
: nullptr;
@@ -2193,6 +2245,14 @@ napi_value TryCallHermesCFunctionFastImpl(
21932245
return nullptr;
21942246
}
21952247

2248+
if (hermesArgsBase != nullptr && !canUseHermesFrameArguments(cif)) {
2249+
return nullptr;
2250+
}
2251+
2252+
if (actualArgc != cif->argc) {
2253+
return nullptr;
2254+
}
2255+
21962256
const bool hasExactHermesFrameArgs =
21972257
hermesArgsBase != nullptr && actualArgc == cif->argc;
21982258
CFunctionHermesFrameDirectReturnInvoker frameDirectReturnInvoker =
@@ -2291,7 +2351,7 @@ HermesFastRoundTripCacheFrameGuard roundTripCacheFrame(
22912351

22922352
bool didInvoke = false;
22932353
CFunctionEngineDirectInvoker invoker =
2294-
cif->signatureHash != 0
2354+
!cif->skipGeneratedNapiDispatch && cif->signatureHash != 0
22952355
? ensureHermesCFunctionEngineDirectInvoker(function, cif)
22962356
: nullptr;
22972357
@try {

0 commit comments

Comments
 (0)