From beea709d5eb5906d87116677f079fb2b0daf9cf0 Mon Sep 17 00:00:00 2001 From: Nikolay Igotti Date: Fri, 14 Feb 2020 14:14:13 +0300 Subject: [PATCH] Rework lazy due to cyclic collector. (#3861) --- backend.native/tests/runtime/workers/lazy1.kt | 52 ++++++++++-- runtime/src/main/cpp/Memory.cpp | 47 ----------- runtime/src/main/kotlin/kotlin/Lazy.kt | 2 +- .../kotlin/kotlin/native/concurrent/Lazy.kt | 82 +++++++++---------- .../kotlin/native/concurrent/MutableData.kt | 3 + 5 files changed, 87 insertions(+), 99 deletions(-) diff --git a/backend.native/tests/runtime/workers/lazy1.kt b/backend.native/tests/runtime/workers/lazy1.kt index 4b1f4299039..f15cc8f6bb2 100644 --- a/backend.native/tests/runtime/workers/lazy1.kt +++ b/backend.native/tests/runtime/workers/lazy1.kt @@ -6,17 +6,55 @@ package runtime.workers.lazy1 import kotlin.test.* - import kotlin.native.concurrent.* -class Leak { - val leak by lazy { this } +class Lazy { + val x = 17 + val self by lazy { this } + val recursion: Int by lazy { + if (x < 17) 42 else recursion + } + val freezer: Int by lazy { + freeze() + 42 + } + val thrower: String by lazy { + if (x < 100) throw IllegalArgumentException() + "FAIL" + } } -@Test fun runTest() { - assertFailsWith { - for (i in 1..100) - Leak().freeze().leak +@Test fun runTest1() { + assertFailsWith { + println(Lazy().recursion) + } + assertFailsWith { + println(Lazy().freeze().recursion) + } +} + +@Test fun runTest2() { + var sum = 0 + for (i in 1 .. 100) { + val self = Lazy().freeze() + assertEquals(self, self.self) + sum += self.self.hashCode() } println("OK") } + + +@Test fun runTest3() { + assertFailsWith { + println(Lazy().freezer) + } +} + +@Test fun runTest4() { + val self = Lazy() + repeat(10) { + assertFailsWith { + println(self.thrower) + } + } +} \ No newline at end of file diff --git a/runtime/src/main/cpp/Memory.cpp b/runtime/src/main/cpp/Memory.cpp index c79ed1824d4..e9435d98aae 100644 --- a/runtime/src/main/cpp/Memory.cpp +++ b/runtime/src/main/cpp/Memory.cpp @@ -2545,49 +2545,6 @@ void ensureNeverFrozen(ObjHeader* object) { object->meta_object()->flags_ |= MF_NEVER_FROZEN; } -// TODO: incorrect, use 3-color scheme. -KBoolean ensureAcyclicAndSet(ObjHeader* where, KInt index, ObjHeader* what) { - RuntimeAssert(where->container() != nullptr && where->container()->frozen(), "Must be used on frozen objects only"); - RuntimeAssert(what == nullptr || isPermanentOrFrozen(what), - "Must be used with an immutable value"); - if (what != nullptr) { - // Now we check that `where` is not reachable from `what`. - // As we cannot modify objects while traversing, instead we remember all seen objects in a set. - KStdUnorderedSet seen; - KStdDeque queue; - if (what->container() != nullptr) - queue.push_back(what->container()); - bool acyclic = true; - while (!queue.empty() && acyclic) { - ContainerHeader* current = queue.front(); - queue.pop_front(); - seen.insert(current); - if (isAggregatingFrozenContainer(current)) { - ContainerHeader** subContainer = reinterpret_cast(current + 1); - for (int i = 0; i < current->objectCount(); ++i) { - if (seen.count(*subContainer) == 0) - queue.push_back(*subContainer++); - } - } else { - traverseContainerReferredObjects(current, [where, &queue, &acyclic, &seen](ObjHeader* obj) { - if (obj == where) { - acyclic = false; - } else { - auto* objContainer = obj->container(); - if (objContainer != nullptr && seen.count(objContainer) == 0) - queue.push_back(objContainer); - } - }); - } - } - if (!acyclic) return false; - } - UpdateHeapRef(reinterpret_cast( - reinterpret_cast(where) + where->type_info()->objOffsets_[index]), what); - // Fence on updated location? - return true; -} - void shareAny(ObjHeader* obj) { auto* container = obj->container(); if (isShareable(container)) return; @@ -3040,10 +2997,6 @@ void EnsureNeverFrozen(ObjHeader* object) { ensureNeverFrozen(object); } -KBoolean Konan_ensureAcyclicAndSet(ObjHeader* where, KInt index, ObjHeader* what) { - return ensureAcyclicAndSet(where, index, what); -} - void Kotlin_Any_share(ObjHeader* obj) { shareAny(obj); } diff --git a/runtime/src/main/kotlin/kotlin/Lazy.kt b/runtime/src/main/kotlin/kotlin/Lazy.kt index ee6cac42fae..0631116caa2 100644 --- a/runtime/src/main/kotlin/kotlin/Lazy.kt +++ b/runtime/src/main/kotlin/kotlin/Lazy.kt @@ -34,7 +34,7 @@ public actual fun lazy(initializer: () -> T): Lazy = FreezeAwareLazyImpl( public actual fun lazy(mode: LazyThreadSafetyMode, initializer: () -> T): Lazy = when (mode) { LazyThreadSafetyMode.SYNCHRONIZED -> throw UnsupportedOperationException() - LazyThreadSafetyMode.PUBLICATION -> throw UnsupportedOperationException() + LazyThreadSafetyMode.PUBLICATION -> FreezeAwareLazyImpl(initializer) LazyThreadSafetyMode.NONE -> UnsafeLazyImpl(initializer) } diff --git a/runtime/src/main/kotlin/kotlin/native/concurrent/Lazy.kt b/runtime/src/main/kotlin/kotlin/native/concurrent/Lazy.kt index 0a80452c62d..e05d4fd0b5e 100644 --- a/runtime/src/main/kotlin/kotlin/native/concurrent/Lazy.kt +++ b/runtime/src/main/kotlin/kotlin/native/concurrent/Lazy.kt @@ -6,67 +6,61 @@ package kotlin.native.concurrent import kotlin.native.internal.Frozen -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, - // as an ordinal is used to refer a field. - private var value_: Any? = UNINITIALIZED - private var initializer_: (() -> T)? = initializer + private val value_ = FreezableAtomicReference(UNINITIALIZED) + private val initializer_ = FreezableAtomicReference<(() -> T)?>(initializer) private val lock_ = Lock() + private fun getOrInit(doFreeze: Boolean): T { + var result = value_.value + if (result !== UNINITIALIZED) { + if (result === INITIALIZING) { + value_.value = UNINITIALIZED + throw IllegalStateException("Recursive lazy computation") + } + @Suppress("UNCHECKED_CAST") + return result as T + } + // Set value_ to INITIALIZING. + value_.value = INITIALIZING + try { + result = initializer_.value!!() + if (doFreeze) result.freeze() + } catch (throwable: Throwable) { + value_.value = UNINITIALIZED + throw throwable + } + if (!doFreeze && this.isFrozen) { + value_.value = UNINITIALIZED + throw InvalidMutabilityException("Frozen during lazy computation") + } + // Set value_ to actual one. + value_.value = result + // Clear initializer. + initializer_.value = null + @Suppress("UNCHECKED_CAST") + return result as T + } + override val value: T get() { - if (isFrozen) { + return if (isFrozen) { locked(lock_) { - // Lock is already taken above. - var result = readHeapRefNoLock(this, 0) - if (result !== UNINITIALIZED) { - assert(result !== INITIALIZING) - @Suppress("UNCHECKED_CAST") - return result as T - } - // Set value_ to INITIALIZING. - ensureAcyclicAndSet(this, 0, INITIALIZING) - result = initializer_!!().freeze() - // Set value_. - if (!ensureAcyclicAndSet(this, 0, result)) { - throw InvalidMutabilityException("Setting cyclic data via lazy in $this: $result") - } - // Do not clear initializer_ reference, as it may break freezing invariants and zero out - // still valid object. It seems to be safe only in case when `this` is not reachable from - // initializer. - @Suppress("UNCHECKED_CAST") - return result as T + getOrInit(true) } } else { - var result: Any? = value_ - if (result === UNINITIALIZED) { - result = initializer_!!() - if (isFrozen) - throw InvalidMutabilityException("$this got frozen during lazy evaluation" ) - value_ = result - initializer_ = null - } - @Suppress("UNCHECKED_CAST") - return result as T + getOrInit(false) } } /** * This operation on shared objects may return value which is no longer reflect the current state of lazy. */ - override fun isInitialized(): Boolean = (value_ !== UNINITIALIZED) && (value_ !== INITIALIZING) + override fun isInitialized(): Boolean = (value_.value !== UNINITIALIZED) && (value_.value !== INITIALIZING) override fun toString(): String = if (isInitialized()) - value.toString() else "Lazy value not initialized yet." + value.toString() else "Lazy value not initialized yet" } internal object UNINITIALIZED { diff --git a/runtime/src/main/kotlin/kotlin/native/concurrent/MutableData.kt b/runtime/src/main/kotlin/kotlin/native/concurrent/MutableData.kt index 753fd72ba9e..4be56b1bd8b 100644 --- a/runtime/src/main/kotlin/kotlin/native/concurrent/MutableData.kt +++ b/runtime/src/main/kotlin/kotlin/native/concurrent/MutableData.kt @@ -14,6 +14,9 @@ external private fun Any.share() @SymbolName("Kotlin_CPointer_CopyMemory") external private fun CopyMemory(to: COpaquePointer?, from: COpaquePointer?, count: Int) +@SymbolName("ReadHeapRefNoLock") +internal external fun readHeapRefNoLock(where: Any, index: Int): Any? + /** * Mutable concurrently accessible data buffer. Could be accessed from several workers simulteniously. */