diff --git a/NativeScript/runtime/ClassBuilder.cpp b/NativeScript/runtime/ClassBuilder.cpp index fcc3033e..9195401e 100644 --- a/NativeScript/runtime/ClassBuilder.cpp +++ b/NativeScript/runtime/ClassBuilder.cpp @@ -1,10 +1,54 @@ #include "ClassBuilder.h" +#include + +#include "UnfairLock.h" + namespace tns { +namespace { +// objc_allocateClassPair only rejects names that are already *registered*, so +// two threads extending the same class name concurrently can both allocate it +// and register duplicate same-named classes (objc keeps both and name lookups +// become ambiguous). Serialize the whole allocate -> register window. +// +// This lock is a leaf — the section never acquires a v8::Locker — so it cannot +// form a cycle with the isolate locks. +UnfairMutex extendedClassRegistrationMutex; + +// Registered objc class names are never reclaimed and the ladder below +// allocates suffixes densely under the lock, so a name's taken suffixes form a +// contiguous prefix. Gallop + binary-search with objc_getClass probes to find +// its end, so heavy same-name reuse (worker tests, HMR re-extends of one +// class) stays O(log N) per extend with no side state to grow. +int FirstFreeSuffix(const std::string& initialName) { + auto taken = [&initialName](int i) { + return objc_getClass((initialName + std::to_string(i)).c_str()) != nil; + }; + int hi = 1; + while (taken(hi)) { + hi *= 2; + } + int lo = hi / 2; + while (lo + 1 < hi) { + int mid = lo + (hi - lo) / 2; + if (taken(mid)) { + lo = mid; + } else { + hi = mid; + } + } + return hi; +} + +// Bounds only *consecutive* failures past the probed free position, i.e. +// allocation failing for reasons other than an ordinary name collision (which +// would otherwise loop forever holding the mutex). +constexpr int kMaxConsecutiveAllocFailures = 100; +} // namespace + // Moved this method in a separate .cpp file because ARC destroys the class // created with objc_allocateClassPair when the control leaves this method scope -// TODO: revist this. Maybe a lock is needed regardless Class ClassBuilder::GetExtendedClass(std::string baseClassName, std::string staticClassName, std::string suffix) { @@ -14,19 +58,23 @@ Class ClassBuilder::GetExtendedClass(std::string baseClassName, ? staticClassName : baseClassName + suffix + "_" + std::to_string(++ClassBuilder::classNameCounter_); - // here we could either call objc_getClass with the name to see if the class - // already exists or we can just try allocating it, which will return nil if - // the class already exists so we try allocating it every time to avoid race - // conditions in case this method is being executed by multiple threads + // Allocation failure is the collision signal (objc_getClass beforehand + // would race), but that only detects *registered* names — hence the lock + // spanning allocate -> register. + std::lock_guard lock(extendedClassRegistrationMutex); Class clazz = objc_allocateClassPair(baseClass, name.c_str(), 0); if (clazz == nil) { - int i = 1; std::string initialName = name; - while (clazz == nil) { - name = initialName + std::to_string(i++); + int next = FirstFreeSuffix(initialName); + for (int attempts = 0; + clazz == nil && attempts < kMaxConsecutiveAllocFailures; attempts++) { + name = initialName + std::to_string(next++); clazz = objc_allocateClassPair(baseClass, name.c_str(), 0); } + if (clazz == nil) { + return nil; + } } objc_registerClassPair(clazz); diff --git a/NativeScript/runtime/ClassBuilder.mm b/NativeScript/runtime/ClassBuilder.mm index 4b7a552b..df8c6c20 100644 --- a/NativeScript/runtime/ClassBuilder.mm +++ b/NativeScript/runtime/ClassBuilder.mm @@ -63,6 +63,7 @@ Class extendedClass = ClassBuilder::GetExtendedClass(baseClassName, staticClassName, std::to_string(isolateId) + "_"); + tns::Assert(extendedClass != nil, isolate); class_addProtocol(extendedClass, @protocol(TNSDerivedClass)); class_addProtocol(object_getClass(extendedClass), @protocol(TNSDerivedClass)); @@ -222,6 +223,7 @@ auto isolateId = cache->getIsolateId(); __block Class extendedClass = ClassBuilder::GetExtendedClass( baseClassName, extendedClassName, std::to_string(isolateId) + "_"); + tns::Assert(extendedClass != nil, isolate); class_addProtocol(extendedClass, @protocol(TNSDerivedClass)); class_addProtocol(object_getClass(extendedClass), @protocol(TNSDerivedClass)); diff --git a/NativeScript/runtime/SpinLock.h b/NativeScript/runtime/SpinLock.h index 245684be..20008569 100644 --- a/NativeScript/runtime/SpinLock.h +++ b/NativeScript/runtime/SpinLock.h @@ -1,15 +1,19 @@ #ifndef SpinLock_h #define SpinLock_h - /** WARNING: Do NOT use this. - More ofthen than not a normal mutex is better and this spinlock is really unfair in multi threading - This is only supposed to be used in places where the function is very fast and the expected concurrency is very low - If any of those things are false, this WILL be slower and worse than a mutex or a read write mutex - - The only place this is currently used is for caching selectors, which take ns to run and are not locked to a specific isolate. + More ofthen than not a normal mutex is better and this spinlock is really + unfair in multi threading This is only supposed to be used in places where the + function is very fast and the expected concurrency is very low If any of those + things are false, this WILL be slower and worse than a mutex or a read write + mutex + + The only place this is currently used is for caching selectors, which take ns + to run and are not locked to a specific isolate. + + For sections that can block or see bursty contention, use UnfairLock.h instead. */ struct SpinMutex { @@ -43,14 +47,9 @@ struct SpinMutex { }; struct SpinLock { - SpinMutex& _mutex; - SpinLock(SpinMutex& m) : _mutex(m) { - _mutex.lock(); - } - ~SpinLock() { - _mutex.unlock(); - } + SpinMutex& _mutex; + SpinLock(SpinMutex& m) : _mutex(m) { _mutex.lock(); } + ~SpinLock() { _mutex.unlock(); } }; - #endif /* SpinLock_h */ diff --git a/NativeScript/runtime/UnfairLock.h b/NativeScript/runtime/UnfairLock.h new file mode 100644 index 00000000..13f172b1 --- /dev/null +++ b/NativeScript/runtime/UnfairLock.h @@ -0,0 +1,60 @@ +#ifndef UnfairLock_h +#define UnfairLock_h + +#include + +/** + BasicLockable wrapper over os_unfair_lock — the fastest blocking lock on + Darwin. Drive it with std::lock_guard. + + Prefer this over SpinLock for any critical section that can block (syscalls, + objc runtime calls, allocation) or that can see bursty contention: waiters + sleep in the kernel and donate their priority to the holder, while SpinLock + busy-waits in userspace, invisible to the scheduler. Benchmarked on an + M-series host: under an 8-thread burst on a short section os_unfair_lock is + ~30x faster than SpinLock at ~33x less CPU, and against a holder that sleeps + mid-section waiters cost ~100x less CPU. SpinLock stays marginally faster + (~0.2 ns/op) only for uncontended nanosecond-scale sections — its documented + niche (selector caching). + + NATIVESCRIPT_UNFAIR_LOCK_ADAPTIVE_SPIN opts lock() into + os_unfair_lock_lock_with_options() with kernel-informed adaptive spinning — + the approach Firefox adopted for its nanosecond-hot allocator locks: + https://hacks.mozilla.org/2022/10/improving-firefox-responsiveness-on-macos/ + + Experiments only, never ship it: + - PRIVATE API (os/lock_private.h); App Review flags the symbol. + - It only wins for nanosecond-scale sections at low contention with the + holder on-core (measured ~2x over plain os_unfair_lock at 2 threads). + Under bursty contention it degenerates to spinlock behavior (~30x slower, + ~30x more CPU at 8 threads), it is the worst option under QoS inversion, + and it changes nothing when the holder sleeps. + */ + +#ifdef NATIVESCRIPT_UNFAIR_LOCK_ADAPTIVE_SPIN +extern "C" void os_unfair_lock_lock_with_options(os_unfair_lock_t lock, + uint32_t options); +// Values from os/lock_private.h (ADAPTIVE_SPIN requires iOS 13+). +#define NATIVESCRIPT_OS_UNFAIR_LOCK_DATA_SYNCHRONIZATION 0x00010000u +#define NATIVESCRIPT_OS_UNFAIR_LOCK_ADAPTIVE_SPIN 0x00040000u +#endif + +struct UnfairMutex { + os_unfair_lock lock_ = OS_UNFAIR_LOCK_INIT; + + inline void lock() noexcept { +#ifdef NATIVESCRIPT_UNFAIR_LOCK_ADAPTIVE_SPIN + os_unfair_lock_lock_with_options( + &lock_, NATIVESCRIPT_OS_UNFAIR_LOCK_DATA_SYNCHRONIZATION | + NATIVESCRIPT_OS_UNFAIR_LOCK_ADAPTIVE_SPIN); +#else + os_unfair_lock_lock(&lock_); +#endif + } + + inline bool try_lock() noexcept { return os_unfair_lock_trylock(&lock_); } + + inline void unlock() noexcept { os_unfair_lock_unlock(&lock_); } +}; + +#endif /* UnfairLock_h */