From 851ef95672703be9f860bd58edd29723f8c1d721 Mon Sep 17 00:00:00 2001 From: Nikolay Igotti Date: Wed, 29 Jan 2020 15:58:06 +0300 Subject: [PATCH] Optimize atomics access by keeping thread affinity info an atomic. (#3780) --- runtime/src/main/cpp/Atomic.cpp | 11 ++- runtime/src/main/cpp/Exceptions.cpp | 6 +- runtime/src/main/cpp/Memory.cpp | 77 ++++++++++++------- runtime/src/main/cpp/Memory.h | 8 +- runtime/src/main/cpp/Weak.cpp | 5 +- .../kotlin/native/concurrent/Atomics.kt | 6 ++ .../kotlin/kotlin/native/ref/WeakPrivate.kt | 3 + 7 files changed, 79 insertions(+), 37 deletions(-) diff --git a/runtime/src/main/cpp/Atomic.cpp b/runtime/src/main/cpp/Atomic.cpp index e9e20dc5ef2..5f059b8a956 100644 --- a/runtime/src/main/cpp/Atomic.cpp +++ b/runtime/src/main/cpp/Atomic.cpp @@ -26,6 +26,7 @@ struct AtomicReferenceLayout { ObjHeader header; KRef value_; KInt lock_; + KInt cookie_; }; template struct AtomicPrimitive { @@ -184,7 +185,8 @@ OBJ_GETTER(Kotlin_AtomicReference_compareAndSwap, KRef thiz, KRef expectedValue, Kotlin_AtomicReference_checkIfFrozen(newValue); // See Kotlin_AtomicReference_get() for explanations, why locking is needed. AtomicReferenceLayout* ref = asAtomicReference(thiz); - RETURN_RESULT_OF(SwapHeapRefLocked, &ref->value_, expectedValue, newValue, &ref->lock_); + RETURN_RESULT_OF(SwapHeapRefLocked, &ref->value_, expectedValue, newValue, + &ref->lock_, &ref->cookie_); } KBoolean Kotlin_AtomicReference_compareAndSet(KRef thiz, KRef expectedValue, KRef newValue) { @@ -192,14 +194,15 @@ KBoolean Kotlin_AtomicReference_compareAndSet(KRef thiz, KRef expectedValue, KRe // See Kotlin_AtomicReference_get() for explanations, why locking is needed. AtomicReferenceLayout* ref = asAtomicReference(thiz); ObjHolder holder; - auto old = SwapHeapRefLocked(&ref->value_, expectedValue, newValue, &ref->lock_, holder.slot()); + auto old = SwapHeapRefLocked(&ref->value_, expectedValue, newValue, + &ref->lock_, &ref->cookie_, holder.slot()); return old == expectedValue; } void Kotlin_AtomicReference_set(KRef thiz, KRef newValue) { Kotlin_AtomicReference_checkIfFrozen(newValue); AtomicReferenceLayout* ref = asAtomicReference(thiz); - SetHeapRefLocked(&ref->value_, newValue, &ref->lock_); + SetHeapRefLocked(&ref->value_, newValue, &ref->lock_, &ref->cookie_); } OBJ_GETTER(Kotlin_AtomicReference_get, KRef thiz) { @@ -208,7 +211,7 @@ OBJ_GETTER(Kotlin_AtomicReference_get, KRef thiz) { // rescheduled unluckily, between the moment value is read from the field and RC is incremented, // object may go away. AtomicReferenceLayout* ref = asAtomicReference(thiz); - RETURN_RESULT_OF(ReadHeapRefLocked, &ref->value_, &ref->lock_); + RETURN_RESULT_OF(ReadHeapRefLocked, &ref->value_, &ref->lock_, &ref->cookie_); } } // extern "C" diff --git a/runtime/src/main/cpp/Exceptions.cpp b/runtime/src/main/cpp/Exceptions.cpp index 2314280046f..ad535102cc5 100644 --- a/runtime/src/main/cpp/Exceptions.cpp +++ b/runtime/src/main/cpp/Exceptions.cpp @@ -52,6 +52,7 @@ extern "C" void ExceptionReporterLaunchpad(KRef reporter, KRef throwable); KRef currentUnhandledExceptionHook = nullptr; int32_t currentUnhandledExceptionHookLock = 0; +int32_t currentUnhandledExceptionHookCookie = 0; #if USE_GCC_UNWIND struct Backtrace { @@ -211,13 +212,14 @@ void ThrowException(KRef exception) { OBJ_GETTER(Kotlin_setUnhandledExceptionHook, KRef hook) { RETURN_RESULT_OF(SwapHeapRefLocked, - ¤tUnhandledExceptionHook, currentUnhandledExceptionHook, hook, ¤tUnhandledExceptionHookLock); + ¤tUnhandledExceptionHook, currentUnhandledExceptionHook, hook, ¤tUnhandledExceptionHookLock, + ¤tUnhandledExceptionHookCookie); } void OnUnhandledException(KRef throwable) { ObjHolder handlerHolder; auto* handler = SwapHeapRefLocked(¤tUnhandledExceptionHook, currentUnhandledExceptionHook, nullptr, - ¤tUnhandledExceptionHookLock, handlerHolder.slot()); + ¤tUnhandledExceptionHookLock, ¤tUnhandledExceptionHookCookie, handlerHolder.slot()); if (handler == nullptr) { ReportUnhandledException(throwable); } else { diff --git a/runtime/src/main/cpp/Memory.cpp b/runtime/src/main/cpp/Memory.cpp index 8861f72d3e7..55339ec0339 100644 --- a/runtime/src/main/cpp/Memory.cpp +++ b/runtime/src/main/cpp/Memory.cpp @@ -445,6 +445,7 @@ struct MemoryState { bool gcErgonomics; uint64_t lastGcTimestamp; + uint32_t gcEpoque; uint64_t allocSinceLastGc; uint64_t allocSinceLastGcThreshold; @@ -1596,6 +1597,7 @@ void garbageCollect(MemoryState* state, bool force) { auto gcStartTime = konan::getTimeMicros(); state->gcInProgress = true; + state->gcEpoque++; incrementStack(state); processDecrements(state); @@ -2056,51 +2058,72 @@ OBJ_GETTER(initSharedInstance, #endif // KONAN_NO_THREADS } +/** + * We keep thread affinity and reference value based cookie in the atomic references, so that + * repeating read operation of the same value do not lead to the repeating rememberNewContainer() operation. + * We must invalidate cookie after the local GC, as otherwise fact that container of the `value` is retained + * may change, if the last reference to the value read is lost during GC and we re-read same value from + * the same atomic reference. Thus we also include GC epoque into the cookie. + */ +inline int32_t computeCookie() { + auto* state = memoryState; + auto epoque = state->gcEpoque; + return (static_cast(reinterpret_cast(state))) ^ static_cast(epoque); +} + OBJ_GETTER(swapHeapRefLocked, - ObjHeader** location, ObjHeader* expectedValue, ObjHeader* newValue, int32_t* spinlock) { + ObjHeader** location, ObjHeader* expectedValue, ObjHeader* newValue, int32_t* spinlock, int32_t* cookie) { lock(spinlock); ObjHeader* oldValue = *location; - bool shallRelease = false; - // We do not use UpdateRef() here to avoid having ReleaseRef() on return slot under the lock. + bool shallRemember = false; + if (IsStrictMemoryModel) { + auto realCookie = computeCookie(); + shallRemember = *cookie != realCookie; + if (shallRemember) *cookie = realCookie; + } if (oldValue == expectedValue) { SetHeapRef(location, newValue); - shallRelease = oldValue != nullptr; - } else { - if (IsStrictMemoryModel && oldValue != nullptr) - rememberNewContainer(oldValue->container()); + } + UpdateReturnRef(OBJ_RESULT, oldValue); + if (IsStrictMemoryModel && shallRemember && oldValue != nullptr && oldValue != expectedValue) { + // Only remember container if it is not known to this thread (i.e. != expectedValue). + rememberNewContainer(oldValue->container()); } unlock(spinlock); - UpdateReturnRef(OBJ_RESULT, oldValue); - if (shallRelease) { - // No need to rememberNewContainer() on this path, as if `oldValue` is not null - it is explicitly released - // anyway, and thus can not escape GC. + if (oldValue != nullptr && oldValue == expectedValue) { ReleaseHeapRef(oldValue); } return oldValue; } -void setHeapRefLocked(ObjHeader** location, ObjHeader* newValue, int32_t* spinlock) { + +void setHeapRefLocked(ObjHeader** location, ObjHeader* newValue, int32_t* spinlock, int32_t* cookie) { lock(spinlock); ObjHeader* oldValue = *location; // We do not use UpdateRef() here to avoid having ReleaseRef() on old value under the lock. SetHeapRef(location, newValue); + *cookie = computeCookie(); unlock(spinlock); if (oldValue != nullptr) ReleaseHeapRef(oldValue); } -OBJ_GETTER(readHeapRefLocked, ObjHeader** location, int32_t* spinlock) { +OBJ_GETTER(readHeapRefLocked, ObjHeader** location, int32_t* spinlock, int32_t* cookie) { MEMORY_LOG("ReadHeapRefLocked: %p\n", location) lock(spinlock); ObjHeader* value = *location; - auto* container = value ? value->container() : nullptr; - if (container != nullptr) - incrementRC(container); - unlock(spinlock); + auto realCookie = computeCookie(); + bool shallRemember = *cookie != realCookie; + if (shallRemember) *cookie = realCookie; UpdateReturnRef(OBJ_RESULT, value); - if (value != nullptr) - ReleaseHeapRef(value); +#if USE_GC + if (IsStrictMemoryModel && shallRemember && value != nullptr) { + auto* container = value->container(); + rememberNewContainer(container); + } +#endif // USE_GC + unlock(spinlock); return value; } @@ -2110,8 +2133,10 @@ OBJ_GETTER(readHeapRefNoLock, ObjHeader* object, KInt index) { reinterpret_cast(object) + object->type_info()->objOffsets_[index]); ObjHeader* value = *location; #if USE_GC - if (IsStrictMemoryModel && value != nullptr) + if (IsStrictMemoryModel && (value != nullptr)) { + // Maybe not so good to do that under lock. rememberNewContainer(value->container()); + } #endif // USE_GC RETURN_OBJ(value); } @@ -2956,16 +2981,16 @@ void UpdateHeapRefIfNull(ObjHeader** location, const ObjHeader* object) { } OBJ_GETTER(SwapHeapRefLocked, - ObjHeader** location, ObjHeader* expectedValue, ObjHeader* newValue, int32_t* spinlock) { - RETURN_RESULT_OF(swapHeapRefLocked, location, expectedValue, newValue, spinlock); + ObjHeader** location, ObjHeader* expectedValue, ObjHeader* newValue, int32_t* spinlock, int32_t* cookie) { + RETURN_RESULT_OF(swapHeapRefLocked, location, expectedValue, newValue, spinlock, cookie); } -void SetHeapRefLocked(ObjHeader** location, ObjHeader* newValue, int32_t* spinlock) { - setHeapRefLocked(location, newValue, spinlock); +void SetHeapRefLocked(ObjHeader** location, ObjHeader* newValue, int32_t* spinlock, int32_t* cookie) { + setHeapRefLocked(location, newValue, spinlock, cookie); } -OBJ_GETTER(ReadHeapRefLocked, ObjHeader** location, int32_t* spinlock) { - RETURN_RESULT_OF(readHeapRefLocked, location, spinlock); +OBJ_GETTER(ReadHeapRefLocked, ObjHeader** location, int32_t* spinlock, int32_t* cookie) { + RETURN_RESULT_OF(readHeapRefLocked, location, spinlock, cookie); } OBJ_GETTER(ReadHeapRefNoLock, ObjHeader* object, KInt index) { diff --git a/runtime/src/main/cpp/Memory.h b/runtime/src/main/cpp/Memory.h index a83b024b17e..3ec4039fe91 100644 --- a/runtime/src/main/cpp/Memory.h +++ b/runtime/src/main/cpp/Memory.h @@ -521,11 +521,13 @@ MODEL_VARIANTS(void, UpdateHeapRefIfNull, ObjHeader** location, const ObjHeader* MODEL_VARIANTS(void, UpdateReturnRef, ObjHeader** returnSlot, const ObjHeader* object); // Compares and swaps reference with taken lock. OBJ_GETTER(SwapHeapRefLocked, - ObjHeader** location, ObjHeader* expectedValue, ObjHeader* newValue, int32_t* spinlock) RUNTIME_NOTHROW; + ObjHeader** location, ObjHeader* expectedValue, ObjHeader* newValue, int32_t* spinlock, + int32_t* cookie) RUNTIME_NOTHROW; // Sets reference with taken lock. -void SetHeapRefLocked(ObjHeader** location, ObjHeader* newValue, int32_t* spinlock) RUNTIME_NOTHROW; +void SetHeapRefLocked(ObjHeader** location, ObjHeader* newValue, int32_t* spinlock, + int32_t* cookie) RUNTIME_NOTHROW; // Reads reference with taken lock. -OBJ_GETTER(ReadHeapRefLocked, ObjHeader** location, int32_t* spinlock) RUNTIME_NOTHROW; +OBJ_GETTER(ReadHeapRefLocked, ObjHeader** location, int32_t* spinlock, int32_t* cookie) RUNTIME_NOTHROW; // Called on frame enter, if it has object slots. MODEL_VARIANTS(void, EnterFrame, ObjHeader** start, int parameters, int count); // Called on frame leave, if it has object slots. diff --git a/runtime/src/main/cpp/Weak.cpp b/runtime/src/main/cpp/Weak.cpp index c99313439f7..0a3c0b577c5 100644 --- a/runtime/src/main/cpp/Weak.cpp +++ b/runtime/src/main/cpp/Weak.cpp @@ -23,6 +23,7 @@ struct WeakReferenceCounter { ObjHeader header; KRef referred; KInt lock; + KInt cookie; }; inline WeakReferenceCounter* asWeakReferenceCounter(ObjHeader* obj) { @@ -81,8 +82,8 @@ OBJ_GETTER(Konan_WeakReferenceCounter_get, ObjHeader* counter) { #if KONAN_NO_THREADS RETURN_OBJ(*referredAddress); #else - int32_t* lockAddress = &asWeakReferenceCounter(counter)->lock; - RETURN_RESULT_OF(ReadHeapRefLocked, referredAddress, lockAddress); + auto* weakCounter = asWeakReferenceCounter(counter); + RETURN_RESULT_OF(ReadHeapRefLocked, referredAddress, &weakCounter->lock, &weakCounter->cookie); #endif } diff --git a/runtime/src/main/kotlin/kotlin/native/concurrent/Atomics.kt b/runtime/src/main/kotlin/kotlin/native/concurrent/Atomics.kt index f2a15cc8664..4df66e34ad1 100644 --- a/runtime/src/main/kotlin/kotlin/native/concurrent/Atomics.kt +++ b/runtime/src/main/kotlin/kotlin/native/concurrent/Atomics.kt @@ -226,6 +226,9 @@ public class AtomicReference(private var value_: T) { // A spinlock to fix potential ARC race. private var lock: Int = 0 + // Optimization for speeding up access. + private var cookie: Int = 0 + /** * Creates a new atomic reference pointing to given [ref]. * @throws InvalidMutabilityException if reference is not frozen. @@ -298,6 +301,9 @@ public class FreezableAtomicReference(private var value_: T) { // A spinlock to fix potential ARC race. private var lock: Int = 0 + // Optimization for speeding up access. + private var cookie: Int = 0 + /** * The referenced value. * Gets the value or sets the [new] value. If [new] value is not null, diff --git a/runtime/src/main/kotlin/kotlin/native/ref/WeakPrivate.kt b/runtime/src/main/kotlin/kotlin/native/ref/WeakPrivate.kt index 17b0a35df57..994b682bc80 100644 --- a/runtime/src/main/kotlin/kotlin/native/ref/WeakPrivate.kt +++ b/runtime/src/main/kotlin/kotlin/native/ref/WeakPrivate.kt @@ -39,6 +39,9 @@ internal class WeakReferenceCounter(var referred: COpaquePointer?) : WeakReferen // Spinlock, potentially taken when materializing or removing 'referred' object. var lock: Int = 0 + // Optimization for concurrent access. + var cookie: Int = 0 + @SymbolName("Konan_WeakReferenceCounter_get") external override fun get(): Any? }