From 1161e2c6b460002d7b854b032a6cfe07cae74df7 Mon Sep 17 00:00:00 2001 From: Nikolay Igotti Date: Mon, 29 Apr 2019 17:03:40 +0300 Subject: [PATCH] Improve stack slots object references handling. (#2912) --- .../backend/konan/llvm/CodeGenerator.kt | 10 +- .../kotlin/backend/konan/llvm/ContextUtils.kt | 2 - backend.native/tests/build.gradle | 2 +- .../tests/runtime/workers/worker10.kt | 87 ++++++- runtime/src/main/cpp/Memory.cpp | 223 +++++++----------- runtime/src/main/cpp/Memory.h | 5 +- runtime/src/main/cpp/Weak.cpp | 6 +- .../kotlin/kotlin/native/concurrent/Lazy.kt | 6 +- .../kotlin/native/concurrent/MutableData.kt | 6 +- .../kotlin/kotlin/native/ref/WeakPrivate.kt | 4 +- 10 files changed, 188 insertions(+), 163 deletions(-) diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/CodeGenerator.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/CodeGenerator.kt index 1e1388351db..4c286460bf1 100644 --- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/CodeGenerator.kt +++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/CodeGenerator.kt @@ -278,8 +278,6 @@ internal class FunctionGenerationContext(val function: LLVMValueRef, } fun store(value: LLVMValueRef, ptr: LLVMValueRef) { - // Use updateRef() or storeAny() API for that. - assert(!isObjectRef(value)) LLVMBuildStore(builder, value, ptr) } @@ -309,11 +307,15 @@ internal class FunctionGenerationContext(val function: LLVMValueRef, } private fun updateReturnRef(value: LLVMValueRef, address: LLVMValueRef) { - call(context.llvm.updateReturnRefFunction, listOf(address, value)) + store(value, address) } private fun updateRef(value: LLVMValueRef, address: LLVMValueRef, onStack: Boolean) { - call(if (onStack) context.llvm.updateStackRefFunction else context.llvm.updateHeapRefFunction, listOf(address, value)) + if (onStack) { + store(value, address) + } else { + call(context.llvm.updateHeapRefFunction, listOf(address, value)) + } } //-------------------------------------------------------------------------// diff --git a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/ContextUtils.kt b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/ContextUtils.kt index 928886e5a42..925739e3905 100644 --- a/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/ContextUtils.kt +++ b/backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/ContextUtils.kt @@ -422,9 +422,7 @@ internal class Llvm(val context: Context, val llvmModule: LLVMModuleRef) { val allocArrayFunction = importRtFunction("AllocArrayInstance") val initInstanceFunction = importRtFunction("InitInstance") val initSharedInstanceFunction = importRtFunction("InitSharedInstance") - val updateReturnRefFunction = importRtFunction("UpdateReturnRef") val updateHeapRefFunction = importRtFunction("UpdateHeapRef") - val updateStackRefFunction = importRtFunction("UpdateStackRef") val enterFrameFunction = importRtFunction("EnterFrame") val leaveFrameFunction = importRtFunction("LeaveFrame") val getReturnSlotIfArenaFunction = importRtFunction("GetReturnSlotIfArena") diff --git a/backend.native/tests/build.gradle b/backend.native/tests/build.gradle index 8c3efee1d87..2cfa709574f 100644 --- a/backend.native/tests/build.gradle +++ b/backend.native/tests/build.gradle @@ -768,7 +768,7 @@ task worker9(type: RunKonanTest) { task worker10(type: RunKonanTest) { disabled = (project.testTarget == 'wasm32') // Workers need pthreads. - goldValue = "OK\n" + goldValue = "OK\ntrue\ntrue\n" source = "runtime/workers/worker10.kt" } diff --git a/backend.native/tests/runtime/workers/worker10.kt b/backend.native/tests/runtime/workers/worker10.kt index fcebcd0a47d..c1438f8e2ba 100644 --- a/backend.native/tests/runtime/workers/worker10.kt +++ b/backend.native/tests/runtime/workers/worker10.kt @@ -3,6 +3,8 @@ package runtime.workers.worker10 import kotlin.test.* import kotlin.native.concurrent.* +import kotlin.native.ref.WeakReference +import kotlinx.cinterop.StableRef class Data(val x: Int) @@ -15,7 +17,7 @@ val topData = Data(42) @SharedImmutable val topSharedData = Data(43) -@Test fun runTest() { +@Test fun runTest1() { val worker = Worker.start() assertEquals(1, topInt) @@ -78,4 +80,87 @@ val topSharedData = Data(43) worker.requestTermination().result println("OK") +} + +val atomicRef = AtomicReference(Any().freeze()) +@SharedImmutable +val stableRef = StableRef.create(Any().freeze()) +val semaphore = AtomicInt(0) + +@Test fun runTest2() { + semaphore.value = 0 + val worker = Worker.start() + val future = worker.execute(TransferMode.SAFE, { null }) { + val value = atomicRef.value + semaphore.increment() + while (semaphore.value != 2) {} + println(value.toString() != "") + } + while (semaphore.value != 1) {} + atomicRef.value = null + kotlin.native.internal.GC.collect() + semaphore.increment() + future.result + worker.requestTermination().result +} + +@Test fun runTest3() { + semaphore.value = 0 + val worker = Worker.start() + val future = worker.execute(TransferMode.SAFE, { null }) { + val value = stableRef.get() + semaphore.increment() + while (semaphore.value != 2) {} + println(value.toString() != "") + } + while (semaphore.value != 1) {} + stableRef.dispose() + kotlin.native.internal.GC.collect() + semaphore.increment() + future.result + worker.requestTermination().result +} + +fun ensureWeakIs(weak: WeakReference, expected: T?) { + assertEquals(expected, weak.get()) +} + +val stableHolder1 = StableRef.create(("hello" to "world").freeze()) + +@Test fun runTest4() { + val worker = Worker.start() + semaphore.value = 0 + val future = worker.execute(TransferMode.SAFE, { WeakReference(stableHolder1.get()) }) { + ensureWeakIs(it, "hello" to "world") + semaphore.increment() + while (semaphore.value != 2) {} + kotlin.native.internal.GC.collect() + ensureWeakIs(it, null) + } + while (semaphore.value != 1) {} + stableHolder1.dispose() + kotlin.native.internal.GC.collect() + semaphore.increment() + future.result + worker.requestTermination().result +} + +val stableHolder2 = StableRef.create(("hello" to "world").freeze()) + +@Test fun runTest5() { + val worker = Worker.start() + semaphore.value = 0 + val future = worker.execute(TransferMode.SAFE, { WeakReference(stableHolder2.get()) }) { + val value = it.get() + semaphore.increment() + while (semaphore.value != 2) {} + kotlin.native.internal.GC.collect() + assertEquals("hello" to "world", value) + } + while (semaphore.value != 1) {} + stableHolder2.dispose() + kotlin.native.internal.GC.collect() + semaphore.increment() + future.result + worker.requestTermination().result } \ No newline at end of file diff --git a/runtime/src/main/cpp/Memory.cpp b/runtime/src/main/cpp/Memory.cpp index 5802fcd96de..a8d1a66b987 100644 --- a/runtime/src/main/cpp/Memory.cpp +++ b/runtime/src/main/cpp/Memory.cpp @@ -106,6 +106,7 @@ volatile int aliveMemoryStatesCount = 0; void freeContainer(ContainerHeader* header) NO_INLINE; #if USE_GC void garbageCollect(MemoryState* state, bool force) NO_INLINE; +void rememberNewContainer(ContainerHeader* container); #endif // USE_GC #if COLLECT_STATISTIC @@ -555,6 +556,11 @@ inline void unlock(KInt* spinlock) { } // namespace +ObjHeader* KRefSharedHolder::ref() const { + verifyRefOwner(); + return obj_; +} + void KRefSharedHolder::initRefOwner() { RuntimeAssert(owner_ == nullptr, "Must be uninitialized"); owner_ = memoryState; @@ -824,8 +830,8 @@ void dumpObject(ObjHeader* ref, int indent) { } void dumpContainerContent(ContainerHeader* container) { - if (container->refCount() <= 0) { - MEMORY_LOG("%p has non-positive RC, likely a memory bug\n", container) + if (container->refCount() < 0) { + MEMORY_LOG("%p has negative RC %d, likely a memory bug\n", container, container->refCount()) return; } if (isAggregatingFrozenContainer(container)) { @@ -1076,40 +1082,11 @@ inline void AddHeapRef(const ObjHeader* header) { AddHeapRef(const_cast(container)); } -inline void AddStackRef(ContainerHeader* container) { - UPDATE_ADDREF_STAT(memoryState, container, needAtomicAccess(container), 1); - if (container->shareable()) { - IncrementRC(container); - } -} - -inline void AddStackRef(const ObjHeader* header) { - auto* container = header->container(); - if (container != nullptr) { - AddStackRef(const_cast(container)); - } -} - inline void ReleaseHeapRef(ContainerHeader* container) { MEMORY_LOG("ReleaseHeapRef %p: rc=%d\n", container, container->refCount()) UPDATE_RELEASEREF_STAT(memoryState, container, needAtomicAccess(container), canBeCyclic(container), 0) - switch (container->tag()) { - case CONTAINER_TAG_STACK: - break; - case CONTAINER_TAG_NORMAL: - EnqueueDecrementRC(container); - break; - /* case CONTAINER_TAG_FROZEN: case CONTAINER_TAG_ATOMIC: */ - default: - DecrementRC(container); - break; - } -} - -inline void ReleaseStackRef(ContainerHeader* container) { - UPDATE_RELEASEREF_STAT(memoryState, container, needAtomicAccess(container), canBeCyclic(container), 1); - if (container->shareable() && container->decRefCount() == 0) { - freeContainer(container); + if (container->tag() != CONTAINER_TAG_STACK) { + EnqueueDecrementRC(container); } } @@ -1119,12 +1096,6 @@ inline void ReleaseHeapRef(const ObjHeader* header) { ReleaseHeapRef(const_cast(container)); } -inline void ReleaseStackRef(const ObjHeader* header) { - auto* container = header->container(); - if (container != nullptr) - ReleaseStackRef(const_cast(container)); -} - // We use first slot as place to store frame-local arena container. // TODO: create ArenaContainer object on the stack, so that we don't // do two allocations per frame (ArenaContainer + actual container). @@ -1424,48 +1395,16 @@ void incrementStack(MemoryState* state) { ObjHeader* obj = *current++; if (obj != nullptr) { auto* container = obj->container(); - if (container != nullptr && container->tag() == CONTAINER_TAG_NORMAL) + if (container == nullptr) continue; + if (container->shareable()) { + IncrementRC(container); + } else { IncrementRC(container); - } - } - frame = frame->previous; - } -} - -void actualizeNewlySharedOnStack(MemoryState* state, const ContainerHeaderSet* newlyShared) { - // For all frozen objects in stack slots - perform reference increment. - FrameOverlay* frame = currentFrame; - MEMORY_LOG("actualizeNewlySharedOnStack: newly shared size is %d\n", newlyShared->size()) - while (frame != nullptr) { - MEMORY_LOG("current frame %p: %d parameters %d locals\n", frame, frame->parameters, frame->count) - ObjHeader** current = reinterpret_cast(frame + 1) + frame->parameters; - ObjHeader** end = current + frame->count - kFrameOverlaySlots - frame->parameters; - while (current < end) { - ObjHeader* obj = *current; - current++; - if (obj != nullptr) { - auto* container = obj->container(); - // No need to use atomic increment yet, object is still local. - if (container != nullptr && container->shareable() && newlyShared->count(container) != 0) { - container->incRefCount(); - MEMORY_LOG("incremented rc of %p to %d\n", container, container->refCount()); } } } frame = frame->previous; } - - // And actualize RC of those objects using toRelease set. - for (auto& container : *(state->toRelease)) { - if (!isMarkedAsRemoved(container) && container->shareable()) { - RuntimeAssert(newlyShared->count(container) != 0, "Must be newly shared"); - // To account for aggregating containers. - ContainerHeader* realContainer = realShareableContainer(container); - auto newRc = realContainer->decRefCount(); - MEMORY_LOG("decremented rc of %p to %d\n", realContainer, newRc); - container = markAsRemoved(container); - } - } } void processDecrements(MemoryState* state) { @@ -1476,6 +1415,8 @@ void processDecrements(MemoryState* state) { toRelease->pop_back(); if (isMarkedAsRemoved(container)) continue; + if (container->shareable()) + container = realShareableContainer(container); DecrementRC(container); } state->gcSuspendCount--; @@ -1491,7 +1432,7 @@ void decrementStack(MemoryState* state) { ObjHeader* obj = *current++; if (obj != nullptr) { auto* container = obj->container(); - if (container != nullptr && container->tag() == CONTAINER_TAG_NORMAL) + if (container != nullptr) EnqueueDecrementRC(container); } } @@ -1554,6 +1495,16 @@ void garbageCollect(MemoryState* state, bool force) { GC_LOG("<<< GC: toFree %d toRelease %d\n", state->toFree->size(), state->toRelease->size()) } +void rememberNewContainer(ContainerHeader* container) { + if (container == nullptr) return; + // Instances can be allocated before actual runtime init - be prepared for that. + if (memoryState != nullptr) { + IncrementRC(container); + // We cannot collect until reference will be stored into the stack slot. + EnqueueDecrementRC(container); + } +} + #endif // USE_GC extern "C" { @@ -1633,26 +1584,22 @@ void ResumeMemory(MemoryState* state) { OBJ_GETTER(AllocInstance, const TypeInfo* type_info) { RuntimeAssert(type_info->instanceSize_ >= 0, "must be an object"); - auto container = ObjectContainer(memoryState, type_info); - ContainerHeader* header = container.header(); - // We cannot collect until reference will be stored into the stack slot. - if (header->tag() == CONTAINER_TAG_NORMAL) { - IncrementRC(header); - EnqueueDecrementRC(header); - } + auto* state = memoryState; + auto container = ObjectContainer(state, type_info); +#if USE_GC + rememberNewContainer(container.header()); +#endif // USE_GC RETURN_OBJ(container.GetPlace()); } OBJ_GETTER(AllocArrayInstance, const TypeInfo* type_info, int32_t elements) { RuntimeAssert(type_info->instanceSize_ < 0, "must be an array"); if (elements < 0) ThrowIllegalArgumentException(); - auto container = ArrayContainer(memoryState, type_info, elements); - ContainerHeader* header = container.header(); - // We cannot collect until reference will be stored into the stack slot. - if (header->tag() == CONTAINER_TAG_NORMAL) { - IncrementRC(header); - EnqueueDecrementRC(header); - } + auto* state = memoryState; + auto container = ArrayContainer(state, type_info, elements); +#if USE_GC + rememberNewContainer(container.header()); +#endif // USE_GC RETURN_OBJ(container.GetPlace()->obj()); } @@ -1747,8 +1694,6 @@ OBJ_GETTER(InitSharedInstance, void SetStackRef(ObjHeader** location, const ObjHeader* object) { MEMORY_LOG("SetStackRef *%p: %p\n", location, object) UPDATE_REF_EVENT(memoryState, nullptr, object, location, 1); - if (object != nullptr) - AddStackRef(const_cast(object)); *const_cast(location) = object; } @@ -1772,27 +1717,21 @@ void ZeroHeapRef(ObjHeader** location) { void ZeroStackRef(ObjHeader** location) { MEMORY_LOG("ZeroStackRef %p\n", location) +#if TRACE_MEMORY auto* value = *location; if (value != nullptr) { UPDATE_REF_EVENT(memoryState, value, nullptr, location, 1); *location = nullptr; - ReleaseStackRef(value); } +#else + *location = nullptr; +#endif } void UpdateStackRef(ObjHeader** location, const ObjHeader* object) { UPDATE_REF_EVENT(memoryState, *location, object, location, 1) RuntimeAssert(object != reinterpret_cast(1), "Markers disallowed here"); - ObjHeader* old = *location; - if (old != object) { - if (object != nullptr) { - AddStackRef(object); - } - *const_cast(location) = object; - if (old != nullptr ) { - ReleaseStackRef(old); - } - } + *const_cast(location) = object; } void UpdateHeapRef(ObjHeader** location, const ObjHeader* object) { @@ -1819,16 +1758,6 @@ ObjHeader** GetParamSlotIfArena(ObjHeader** returnSlot, ObjHeader** localSlot) { return nullptr; } -inline void updateReturnRefAdded(ObjHeader** returnSlot, const ObjHeader* value) { - MEMORY_LOG("updateReturnRefAdded %p\n", returnSlot) - ObjHeader* old = *returnSlot; - UPDATE_REF_EVENT(memoryState, old, value, returnSlot, 1) - *const_cast(returnSlot) = value; - if (old != nullptr) { - ReleaseStackRef(old); - } -} - void UpdateReturnRef(ObjHeader** returnSlot, const ObjHeader* value) { UpdateStackRef(returnSlot, value); } @@ -1865,15 +1794,6 @@ void EnterFrame(ObjHeader** start, int parameters, int count) { void LeaveFrame(ObjHeader** start, int parameters, int count) { MEMORY_LOG("LeaveFrame %p: %d parameters %d locals\n", start, parameters, count) - MemoryState* state = memoryState; - ObjHeader** current = start + parameters + kFrameOverlaySlots; - ObjHeader** end = start + count; - while (current < end) { - ObjHeader* object = *current++; - if (object != nullptr) { - ReleaseStackRef(object); - } - } FrameOverlay* frame = reinterpret_cast(start); currentFrame = frame->previous; } @@ -1976,6 +1896,10 @@ void DisposeStablePointer(KNativePtr pointer) { OBJ_GETTER(DerefStablePointer, KNativePtr pointer) { KRef ref = reinterpret_cast(pointer); +#if USE_GC + if (ref != nullptr) + rememberNewContainer(ref->container()); +#endif // USE_GC RETURN_OBJ(ref); } @@ -2168,7 +2092,7 @@ void freezeAcyclic(ContainerHeader* rootContainer, ContainerHeaderSet* newlyFroz // color and similar attributes shall not be used. if (current->tag() == CONTAINER_TAG_NORMAL) newlyFrozen->insert(current); - MEMORY_LOG("freezeing %p\n", current) + MEMORY_LOG("freezing %p\n", current) current->freeze(); traverseContainerReferredObjects(current, [current, &queue](ObjHeader* obj) { ContainerHeader* objContainer = obj->container(); @@ -2239,7 +2163,7 @@ void freezeCyclic(ContainerHeader* rootContainer, newlyFrozen->insert(container); // Note, that once object is frozen, it could be concurrently accessed, so // color and similar attributes shall not be used. - MEMORY_LOG("freezeing %p\n", container) + MEMORY_LOG("freezing %p\n", container) container->freeze(); // We set refcount of original container to zero, so that it is seen as such after removal // meta-object, where aggregating container is stored. @@ -2285,6 +2209,8 @@ void FreezeSubgraph(ObjHeader* root) { ContainerHeader* rootContainer = root->container(); if (Shareable(rootContainer)) return; + MEMORY_LOG("Freeze subgraph of %p\n", root) + // Do DFS cycle detection. bool hasCycles = false; KRef firstBlocker = root->has_meta_object() && ((root->meta_object()->flags_ & MF_NEVER_FROZEN) != 0) ? @@ -2292,6 +2218,7 @@ void FreezeSubgraph(ObjHeader* root) { KStdVector order; depthFirstTraversal(rootContainer, &hasCycles, &firstBlocker, &order); if (firstBlocker != nullptr) { + MEMORY_LOG("See freeze blocker for %p: %p\n", root, firstBlocker) ThrowFreezingException(root, firstBlocker); } ContainerHeaderSet newlyFrozen; @@ -2301,6 +2228,7 @@ void FreezeSubgraph(ObjHeader* root) { } else { freezeAcyclic(rootContainer, &newlyFrozen); } + MEMORY_LOG("Graph of %p is %s with %d elements\n", root, hasCycles ? "cyclic" : "acyclic", newlyFrozen.size()) #if USE_GC // Now remove frozen objects from the toFree list. @@ -2313,8 +2241,6 @@ void FreezeSubgraph(ObjHeader* root) { container = markAsRemoved(container); } } - // Actualize reference counters of newly frozen objects. - actualizeNewlySharedOnStack(state, &newlyFrozen); #endif } @@ -2329,19 +2255,19 @@ OBJ_GETTER(SwapHeapRefLocked, ObjHeader** location, ObjHeader* expectedValue, ObjHeader* newValue, int32_t* spinlock) { lock(spinlock); ObjHeader* oldValue = *location; + bool shallRelease = false; // We do not use UpdateRef() here to avoid having ReleaseRef() on return slot under the lock. if (oldValue == expectedValue) { SetHeapRef(location, newValue); - } else { - // We create an additional reference to the [oldValue] in the return slot. - if (oldValue != nullptr && isRefCounted(oldValue)) { - AddHeapRef(oldValue); - } + shallRelease = oldValue != nullptr; } unlock(spinlock); - // [oldValue] ownership was either transferred from *location to return slot if CAS succeeded, or - // we explicitly added a new reference if CAS failed. - updateReturnRefAdded(OBJ_RESULT, oldValue); + if (shallRelease) { + ReleaseHeapRef(oldValue); + } + // No need to rememberNewContainer(), as oldValue is already + // present on this worker. + UpdateReturnRef(OBJ_RESULT, oldValue); return oldValue; } @@ -2359,13 +2285,25 @@ OBJ_GETTER(ReadHeapRefLocked, ObjHeader** location, int32_t* spinlock) { MEMORY_LOG("ReadHeapRefLocked: %p\n", location) lock(spinlock); ObjHeader* value = *location; - // We do not use UpdateRef() here to avoid having ReleaseRef() on return slot under the lock. - if (value != nullptr) { - AddStackRef(value); - } + auto* container = value ? value->container() : nullptr; + if (container != nullptr) + IncrementRC(container); unlock(spinlock); - updateReturnRefAdded(OBJ_RESULT, value); - return value; + if (container != nullptr) + EnqueueDecrementRC(container); + RETURN_OBJ(value); +} + +OBJ_GETTER(ReadHeapRefNoLock, ObjHeader* object, KInt index) { + MEMORY_LOG("ReadHeapRefNoLock: %p index %d\n", object, index) + ObjHeader** location = reinterpret_cast( + reinterpret_cast(object) + object->type_info()->objOffsets_[index]); + ObjHeader* value = *location; +#if USE_GC + if (value != nullptr) + rememberNewContainer(value->container()); +#endif // USE_GC + RETURN_OBJ(value); } void EnsureNeverFrozen(ObjHeader* object) { @@ -2419,14 +2357,13 @@ KBoolean Konan_ensureAcyclicAndSet(ObjHeader* where, KInt index, ObjHeader* what return true; } + + void Kotlin_Any_share(ObjHeader* obj) { auto* container = obj->container(); if (Shareable(container)) return; RuntimeCheck(container->objectCount() == 1, "Must be a single object container"); container->makeShareable(); - ContainerHeaderSet newlyShared; - newlyShared.insert(container); - actualizeNewlySharedOnStack(memoryState, &newlyShared); } } // extern "C" diff --git a/runtime/src/main/cpp/Memory.h b/runtime/src/main/cpp/Memory.h index 7aaa4d763fe..b1f50628f4a 100644 --- a/runtime/src/main/cpp/Memory.h +++ b/runtime/src/main/cpp/Memory.h @@ -581,10 +581,7 @@ class KRefSharedHolder { SetHeapRef(slotToInit(), obj); } - inline ObjHeader* ref() const { - verifyRefOwner(); - return obj_; - } + ObjHeader* ref() const; inline void dispose() { verifyRefOwner(); diff --git a/runtime/src/main/cpp/Weak.cpp b/runtime/src/main/cpp/Weak.cpp index d6fca1ae139..a9bcf35768d 100644 --- a/runtime/src/main/cpp/Weak.cpp +++ b/runtime/src/main/cpp/Weak.cpp @@ -77,11 +77,7 @@ OBJ_GETTER(Konan_WeakReferenceCounter_get, ObjHeader* counter) { RETURN_OBJ(*referredAddress); #else int32_t* lockAddress = &asWeakReferenceCounter(counter)->lock; - // Spinlock. - lock(lockAddress); - ObjHolder holder(*referredAddress); - unlock(lockAddress); - RETURN_OBJ(holder.obj()); + RETURN_RESULT_OF(ReadHeapRefLocked, referredAddress, lockAddress); #endif } diff --git a/runtime/src/main/kotlin/kotlin/native/concurrent/Lazy.kt b/runtime/src/main/kotlin/kotlin/native/concurrent/Lazy.kt index 6c5284ab033..0a80452c62d 100644 --- a/runtime/src/main/kotlin/kotlin/native/concurrent/Lazy.kt +++ b/runtime/src/main/kotlin/kotlin/native/concurrent/Lazy.kt @@ -11,6 +11,9 @@ import kotlin.native.internal.NoReorderFields @SymbolName("Konan_ensureAcyclicAndSet") private external fun ensureAcyclicAndSet(where: Any, index: Int, what: Any?): Boolean +@SymbolName("ReadHeapRefNoLock") +internal external fun readHeapRefNoLock(where: Any, index: Int): Any? + @NoReorderFields internal class FreezeAwareLazyImpl(initializer: () -> T) : Lazy { // IMPORTANT: due to simplified ensureAcyclicAndSet() semantics fields here must be ordered like this, @@ -23,7 +26,8 @@ internal class FreezeAwareLazyImpl(initializer: () -> T) : Lazy { get() { if (isFrozen) { locked(lock_) { - var result: Any? = value_ + // Lock is already taken above. + var result = readHeapRefNoLock(this, 0) if (result !== UNINITIALIZED) { assert(result !== INITIALIZING) @Suppress("UNCHECKED_CAST") diff --git a/runtime/src/main/kotlin/kotlin/native/concurrent/MutableData.kt b/runtime/src/main/kotlin/kotlin/native/concurrent/MutableData.kt index cc8f3ff88d6..753fd72ba9e 100644 --- a/runtime/src/main/kotlin/kotlin/native/concurrent/MutableData.kt +++ b/runtime/src/main/kotlin/kotlin/native/concurrent/MutableData.kt @@ -18,6 +18,7 @@ external private fun CopyMemory(to: COpaquePointer?, from: COpaquePointer?, coun * Mutable concurrently accessible data buffer. Could be accessed from several workers simulteniously. */ @Frozen +@NoReorderFields public class MutableData constructor(capacity: Int = 16) { init { if (capacity <= 0) throw IllegalArgumentException() @@ -25,7 +26,10 @@ public class MutableData constructor(capacity: Int = 16) { share() } - private var buffer = ByteArray(capacity).apply { share() } + private var buffer_ = ByteArray(capacity).apply { share() } + private var buffer: ByteArray + get() = readHeapRefNoLock(this, 0) as ByteArray + set(value) { buffer_ = value} private var size_ = 0 private val lock = Lock() diff --git a/runtime/src/main/kotlin/kotlin/native/ref/WeakPrivate.kt b/runtime/src/main/kotlin/kotlin/native/ref/WeakPrivate.kt index 0c3f1e1e50e..045ccb7fd5c 100644 --- a/runtime/src/main/kotlin/kotlin/native/ref/WeakPrivate.kt +++ b/runtime/src/main/kotlin/kotlin/native/ref/WeakPrivate.kt @@ -6,8 +6,9 @@ package kotlin.native.ref import kotlinx.cinterop.COpaquePointer -import kotlin.native.internal.NoReorderFields import kotlin.native.internal.ExportForCppRuntime +import kotlin.native.internal.Frozen +import kotlin.native.internal.NoReorderFields /** * Theory of operations: @@ -33,6 +34,7 @@ import kotlin.native.internal.ExportForCppRuntime // Clear holding the counter object, which refers to the actual object. @NoReorderFields +@Frozen internal class WeakReferenceCounter(var referred: COpaquePointer?) : WeakReferenceImpl() { // Spinlock, potentially taken when materializing or removing 'referred' object. var lock: Int = 0