Store initializing singletons in a separate thread local small map

backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/CodeGenerator.kt

@@ -921,7 +921,6 @@ internal class FunctionGenerationContext(val function: LLVMValueRef,
         }
         val singleton = context.llvmDeclarations.forSingleton(irClass)
         val instanceAddress = singleton.instanceStorage
-        val instanceShadowAddress = singleton.instanceShadowStorage
 
         if (storageKind == ObjectStorageKind.PERMANENT) {
             return loadSlot(instanceAddress.getAddress(this), false)
@@ -938,13 +937,13 @@ internal class FunctionGenerationContext(val function: LLVMValueRef,
         val typeInfo = codegen.typeInfoForAllocation(irClass)
         val defaultConstructor = irClass.constructors.single { it.valueParameters.size == 0 }
         val ctor = codegen.llvmFunction(defaultConstructor)
-        val (initFunction, args) =
+        val initFunction =
                 if (storageKind == ObjectStorageKind.SHARED && context.config.threadsAreAllowed) {
-                    val shadowObjectPtr = instanceShadowAddress!!.getAddress(this)
-                    context.llvm.initSharedInstanceFunction to listOf(objectPtr, shadowObjectPtr, typeInfo, ctor)
+                    context.llvm.initSharedInstanceFunction
                 } else {
-                    context.llvm.initInstanceFunction to listOf(objectPtr, typeInfo, ctor)
+                    context.llvm.initInstanceFunction
                 }
+        val args = listOf(objectPtr, typeInfo, ctor)
         val newValue = call(initFunction, args, Lifetime.GLOBAL, exceptionHandler)
         val bbInitResult = currentBlock
         br(bbExit)

backend.native/compiler/ir/backend.native/src/org/jetbrains/kotlin/backend/konan/llvm/LlvmDeclarations.kt

@@ -67,7 +67,7 @@ internal class ClassLlvmDeclarations(
         val singletonDeclarations: SingletonLlvmDeclarations?,
         val objCDeclarations: KotlinObjCClassLlvmDeclarations?)
 
-internal class SingletonLlvmDeclarations(val instanceStorage: AddressAccess, val instanceShadowStorage: AddressAccess?)
+internal class SingletonLlvmDeclarations(val instanceStorage: AddressAccess)
 
 internal class KotlinObjCClassLlvmDeclarations(
         val classPointerGlobal: StaticData.Global,
@@ -279,12 +279,7 @@ private class DeclarationsGeneratorVisitor(override val context: Context) :
         val symbolName = "kobjref:" + qualifyInternalName(irClass)
         val instanceAddress = addKotlinGlobal(symbolName, getLLVMType(irClass.defaultType), threadLocal = threadLocal)
 
-        val instanceShadowAddress = if (threadLocal || storageKind == ObjectStorageKind.PERMANENT) null else {
-            val shadowSymbolName = "kshadowobjref:" + qualifyInternalName(irClass)
-            addKotlinGlobal(shadowSymbolName, getLLVMType(irClass.defaultType), threadLocal = true)
-        }
-
-        return SingletonLlvmDeclarations(instanceAddress, instanceShadowAddress)
+        return SingletonLlvmDeclarations(instanceAddress)
     }
 
     private fun createKotlinObjCClassDeclarations(irClass: IrClass): KotlinObjCClassLlvmDeclarations {

backend.native/tests/build.gradle

@@ -2675,6 +2675,15 @@ task initializers5(type: KonanLocalTest) {
     source = "runtime/basic/initializers5.kt"
 }
 
+task initializers6(type: KonanLocalTest) {
+    disabled = project.testTarget == 'wasm32' // Needs workers.
+    source = "runtime/basic/initializers6.kt"
+}
+
+task initializers7(type: KonanLocalTest) {
+    source = "runtime/basic/initializers7.kt"
+}
+
 task expression_as_statement(type: KonanLocalTest) {
     expectedFail = (project.testTarget == 'wasm32') // uses exceptions.
     goldValue = "Ok\n"

backend.native/tests/runtime/basic/initializers6.kt

@@ -0,0 +1,75 @@
+/*
+ * Copyright 2010-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license
+ * that can be found in the LICENSE file.
+ */
+
+package runtime.basic.initializers6
+
+import kotlin.test.*
+
+import kotlin.native.concurrent.*
+
+val aWorkerId = 2
+val bWorkersCount = 3
+
+val aWorkerUnlocker = AtomicInt(0)
+val bWorkerUnlocker = AtomicInt(0)
+
+object A {
+    init {
+        // Must be called by aWorker only.
+        assertEquals(aWorkerId, Worker.current.id)
+        // Only allow b workers to run, when a worker has started initialization.
+        bWorkerUnlocker.increment()
+        // Only proceed with initialization, when all b workers have started executing.
+        while (aWorkerUnlocker.value < bWorkersCount) {}
+        // And now wait a bit, to increase probability of races.
+        Worker.current.park(1000 * 1000L)
+    }
+    val a = produceA()
+    val b = produceB()
+}
+
+fun produceA(): String {
+    // Must've been called by aWorker only.
+    assertEquals(aWorkerId, Worker.current.id)
+    return "A"
+}
+
+fun produceB(): String {
+    // Must've been called by aWorker only.
+    assertEquals(aWorkerId, Worker.current.id)
+    // Also check that it's ok to get A.a while initializing A.b.
+    return "B+${A.a}"
+}
+
+@Test fun runTest() {
+    val aWorker = Worker.start()
+    // This test relies on aWorkerId value.
+    assertEquals(aWorkerId, aWorker.id)
+    val bWorkers = Array(bWorkersCount, { _ -> Worker.start() })
+
+    val aFuture = aWorker.execute(TransferMode.SAFE, {}, {
+        A.b
+    })
+    val bFutures = Array(bWorkers.size, {
+        bWorkers[it].execute(TransferMode.SAFE, {}, {
+            // Wait until A has started to initialize.
+            while (bWorkerUnlocker.value < 1) {}
+            // Now allow A initialization to continue.
+            aWorkerUnlocker.increment()
+            // And this should not've tried to init A itself.
+            A.a + A.b
+        })
+    })
+
+    for (future in bFutures) {
+        assertEquals("AB+A", future.result)
+    }
+    assertEquals("B+A", aFuture.result)
+
+    for (worker in bWorkers) {
+        worker.requestTermination().result
+    }
+    aWorker.requestTermination().result
+}

backend.native/tests/runtime/basic/initializers7.kt

@@ -0,0 +1,33 @@
+/*
+ * Copyright 2010-2020 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license
+ * that can be found in the LICENSE file.
+ */
+
+package runtime.basic.initializers7
+
+import kotlin.test.*
+
+import kotlin.random.Random
+
+object A {
+    val a1 = Random.nextInt(100)
+    val a2 = Random.nextInt(100)
+}
+
+object B {
+    val b1 = A.a2
+    val b2 = C.c1
+}
+
+object C {
+    val c1 = Random.nextInt(100)
+    val c2 = A.a1
+    val c3 = B.b1
+    val c4 = B.b2
+}
+
+@Test fun runTest() {
+    assertEquals(A.a1, C.c2)
+    assertEquals(A.a2, C.c3)
+    assertEquals(C.c1, C.c4)
+}

runtime/src/main/cpp/Memory.cpp

@@ -451,6 +451,10 @@ struct MemoryState {
   uint64_t allocSinceLastGcThreshold;
 #endif // USE_GC
 
+  // This map is expected to be small, std::map consumes less memory than
+  // std::unordered_map and is just as efficient.
+  KStdOrderedMap<ObjHeader**, ObjHeader*> initializingSingletons;
+
 #if COLLECT_STATISTIC
   #define CONTAINER_ALLOC_STAT(state, size, container) state->statistic.incAlloc(size, container);
   #define CONTAINER_DESTROY_STAT(state, container) \
@@ -1999,7 +2003,7 @@ OBJ_GETTER(initInstance,
 
 template <bool Strict>
 OBJ_GETTER(initSharedInstance,
-    ObjHeader** location, ObjHeader** localLocation, const TypeInfo* typeInfo, void (*ctor)(ObjHeader*)) {
+    ObjHeader** location, const TypeInfo* typeInfo, void (*ctor)(ObjHeader*)) {
 #if KONAN_NO_THREADS
   ObjHeader* value = *location;
   if (value != nullptr) {
@@ -2025,25 +2029,32 @@ OBJ_GETTER(initSharedInstance,
   }
 #endif  // KONAN_NO_EXCEPTIONS
 #else  // KONAN_NO_THREADS
-  ObjHeader* value = *localLocation;
-  if (value != nullptr) RETURN_OBJ(value);
+  auto it = memoryState->initializingSingletons.find(location);
+  if (it != memoryState->initializingSingletons.end()) {
+    RETURN_OBJ(it->second);
+  }
 
   ObjHeader* initializing = reinterpret_cast<ObjHeader*>(1);
 
   // Spin lock.
+  ObjHeader* value = nullptr;
   while ((value = __sync_val_compare_and_swap(location, nullptr, initializing)) == initializing);
   if (value != nullptr) {
     // OK'ish, inited by someone else.
     RETURN_OBJ(value);
   }
   ObjHeader* object = AllocInstance(typeInfo, OBJ_RESULT);
-  UpdateHeapRef(localLocation, object);
+  auto insertIt = memoryState->initializingSingletons.insert({location, object});
+  RuntimeCheck(insertIt.second, "object cannot be assigned twice into initializingSingletons");
+  addHeapRef(object);
 #if KONAN_NO_EXCEPTIONS
   ctor(object);
   if (Strict)
     FreezeSubgraph(object);
   UpdateHeapRef(location, object);
   synchronize();
+  memoryState->initializingSingletons.erase(location);
+  releaseHeapRef<Strict>(object);
   return object;
 #else  // KONAN_NO_EXCEPTIONS
   try {
@@ -2052,11 +2063,14 @@ OBJ_GETTER(initSharedInstance,
       FreezeSubgraph(object);
     UpdateHeapRef(location, object);
     synchronize();
+    memoryState->initializingSingletons.erase(location);
+    releaseHeapRef<Strict>(object);
     return object;
   } catch (...) {
     UpdateReturnRef(OBJ_RESULT, nullptr);
     zeroHeapRef(location);
-    zeroHeapRef(localLocation);
+    memoryState->initializingSingletons.erase(location);
+    releaseHeapRef<Strict>(object);
     synchronize();
     throw;
   }
@@ -2803,12 +2817,12 @@ OBJ_GETTER(InitInstanceRelaxed,
 }
 
 OBJ_GETTER(InitSharedInstanceStrict,
-    ObjHeader** location, ObjHeader** localLocation, const TypeInfo* typeInfo, void (*ctor)(ObjHeader*)) {
-  RETURN_RESULT_OF(initSharedInstance<true>, location, localLocation, typeInfo, ctor);
+    ObjHeader** location, const TypeInfo* typeInfo, void (*ctor)(ObjHeader*)) {
+  RETURN_RESULT_OF(initSharedInstance<true>, location, typeInfo, ctor);
 }
 OBJ_GETTER(InitSharedInstanceRelaxed,
-    ObjHeader** location, ObjHeader** localLocation, const TypeInfo* typeInfo, void (*ctor)(ObjHeader*)) {
-  RETURN_RESULT_OF(initSharedInstance<false>, location, localLocation, typeInfo, ctor);
+    ObjHeader** location, const TypeInfo* typeInfo, void (*ctor)(ObjHeader*)) {
+  RETURN_RESULT_OF(initSharedInstance<false>, location, typeInfo, ctor);
 }
 
 void SetStackRefStrict(ObjHeader** location, const ObjHeader* object) {

runtime/src/main/cpp/Memory.h

@@ -461,11 +461,11 @@ OBJ_GETTER(InitInstance,
     ObjHeader** location, const TypeInfo* typeInfo, void (*ctor)(ObjHeader*));
 
 OBJ_GETTER(InitSharedInstanceStrict,
-    ObjHeader** location, ObjHeader** localLocation, const TypeInfo* typeInfo, void (*ctor)(ObjHeader*));
+    ObjHeader** location, const TypeInfo* typeInfo, void (*ctor)(ObjHeader*));
 OBJ_GETTER(InitSharedInstanceRelaxed,
-    ObjHeader** location, ObjHeader** localLocation, const TypeInfo* typeInfo, void (*ctor)(ObjHeader*));
+    ObjHeader** location, const TypeInfo* typeInfo, void (*ctor)(ObjHeader*));
 OBJ_GETTER(InitSharedInstance,
-    ObjHeader** location, ObjHeader** localLocation, const TypeInfo* typeInfo, void (*ctor)(ObjHeader*));
+    ObjHeader** location, const TypeInfo* typeInfo, void (*ctor)(ObjHeader*));
 
 // Weak reference operations.
 // Atomically clears counter object reference.

runtime/src/relaxed/cpp/MemoryImpl.cpp

@@ -25,8 +25,8 @@ OBJ_GETTER(InitInstance,
 }
 
 OBJ_GETTER(InitSharedInstance,
-    ObjHeader** location, ObjHeader** localLocation, const TypeInfo* typeInfo, void (*ctor)(ObjHeader*)) {
-  RETURN_RESULT_OF(InitSharedInstanceRelaxed, location, localLocation, typeInfo, ctor);
+    ObjHeader** location, const TypeInfo* typeInfo, void (*ctor)(ObjHeader*)) {
+  RETURN_RESULT_OF(InitSharedInstanceRelaxed, location, typeInfo, ctor);
 }
 
 void ReleaseHeapRef(const ObjHeader* object) {

runtime/src/strict/cpp/MemoryImpl.cpp

@@ -25,8 +25,8 @@ OBJ_GETTER(InitInstance,
 }
 
 OBJ_GETTER(InitSharedInstance,
-    ObjHeader** location, ObjHeader** localLocation, const TypeInfo* typeInfo, void (*ctor)(ObjHeader*)) {
-  RETURN_RESULT_OF(InitSharedInstanceStrict, location, localLocation, typeInfo, ctor);
+    ObjHeader** location, const TypeInfo* typeInfo, void (*ctor)(ObjHeader*)) {
+  RETURN_RESULT_OF(InitSharedInstanceStrict, location, typeInfo, ctor);
 }
 
 void ReleaseHeapRef(const ObjHeader* object) {