Remove remnants of concurrent memory model (#467)

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

@@ -223,14 +223,14 @@ internal class CodeGenerator(override val context: Context) : ContextUtils {
     }
     fun storeAnyLocal(value: LLVMValueRef, ptr: LLVMValueRef) {
         if (isObjectRef(value)) {
-            updateLocalRef(value, ptr)
+            updateRef(value, ptr)
         } else {
             LLVMBuildStore(builder, value, ptr)
         }
     }
     fun storeAnyGlobal(value: LLVMValueRef, ptr: LLVMValueRef) {
         if (isObjectRef(value)) {
-            updateGlobalRef(value, ptr)
+            updateRef(value, ptr)
         } else {
             LLVMBuildStore(builder, value, ptr)
         }
@@ -245,13 +245,8 @@ internal class CodeGenerator(override val context: Context) : ContextUtils {
     }
 
     // Only use ignoreOld, when sure that memory is freshly inited and have no value.
-    fun updateLocalRef(value: LLVMValueRef, address: LLVMValueRef, ignoreOld: Boolean = false) {
+    fun updateRef(value: LLVMValueRef, address: LLVMValueRef, ignoreOld: Boolean = false) {
-        call(if (ignoreOld) context.llvm.setLocalRefFunction else context.llvm.updateLocalRefFunction,
+        call(if (ignoreOld) context.llvm.setRefFunction else context.llvm.updateRefFunction,
-                listOf(address, value))
-    }
-
-    fun updateGlobalRef(value: LLVMValueRef, address: LLVMValueRef, ignoreOld: Boolean = false) {
-        call(if (ignoreOld) context.llvm.setGlobalRefFunction else context.llvm.updateGlobalRefFunction,
                 listOf(address, value))
     }
 

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

@@ -249,14 +249,9 @@ internal class Llvm(val context: Context, val llvmModule: LLVMModuleRef) {
     val allocArrayFunction = importRtFunction("AllocArrayInstance")
     val initInstanceFunction = importRtFunction("InitInstance")
     val updateReturnRefFunction = importRtFunction("UpdateReturnRef")
-    val setLocalRefFunction = importRtFunction("SetLocalRef")
+    val setRefFunction = importRtFunction("SetRef")
-    val setGlobalRefFunction = importRtFunction("SetGlobalRef")
+    val updateRefFunction = importRtFunction("UpdateRef")
-    val updateLocalRefFunction = importRtFunction("UpdateLocalRef")
-    val updateGlobalRefFunction = importRtFunction("UpdateGlobalRef")
     val leaveFrameFunction = importRtFunction("LeaveFrame")
-    val setArrayFunction = importRtFunction("Kotlin_Array_set")
-    val copyImplArrayFunction = importRtFunction("Kotlin_Array_copyImpl")
-    val lookupFieldOffset = importRtFunction("LookupFieldOffset")
     val lookupOpenMethodFunction = importRtFunction("LookupOpenMethod")
     val isInstanceFunction = importRtFunction("IsInstance")
     val checkInstanceFunction = importRtFunction("CheckInstance")

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

@@ -642,9 +642,8 @@ internal class CodeGeneratorVisitor(val context: Context) : IrElementVisitorVoid
         val bbCurrent = codegen.currentBlock
         val bbInit    = codegen.basicBlock("label_init")
         val bbExit    = codegen.basicBlock("label_continue")
-        val onePtr    = codegen.intToPtr(kImmInt64One, codegen.kObjHeaderPtr)
         val objectVal = codegen.loadSlot(objectPtr, false)
-        val condition = codegen.ucmpGt(objectVal, onePtr)
+        val condition = codegen.icmpNe(objectVal, codegen.kNullObjHeaderPtr)
         codegen.condBr(condition, bbExit, bbInit)
 
         codegen.positionAtEnd(bbInit)

runtime/src/main/cpp/Arrays.cpp

@@ -57,7 +57,7 @@ void Kotlin_Array_set(KRef thiz, KInt index, KConstRef value) {
   if (static_cast<uint32_t>(index) >= array->count_) {
     ThrowArrayIndexOutOfBoundsException();
   }
-  UpdateGlobalRef(ArrayAddressOfElementAt(array, index), value);
+  UpdateRef(ArrayAddressOfElementAt(array, index), value);
 }
 
 KInt Kotlin_Array_getArrayLength(KConstRef thiz) {
@@ -71,7 +71,7 @@ void Kotlin_Array_fillImpl(KRef thiz, KInt fromIndex, KInt toIndex, KRef value)
     ThrowArrayIndexOutOfBoundsException();
   }
   for (KInt index = fromIndex; index < toIndex; ++index) {
-    UpdateGlobalRef(ArrayAddressOfElementAt(array, index), value);
+    UpdateRef(ArrayAddressOfElementAt(array, index), value);
   }
 }
 
@@ -86,12 +86,12 @@ void Kotlin_Array_copyImpl(KConstRef thiz, KInt fromIndex,
   }
   if (fromIndex >= toIndex) {
     for (int index = 0; index < count; index++) {
-      UpdateGlobalRef(ArrayAddressOfElementAt(destinationArray, toIndex + index),
+      UpdateRef(ArrayAddressOfElementAt(destinationArray, toIndex + index),
                       *ArrayAddressOfElementAt(array, fromIndex + index));
     }
   } else {
     for (int index = count - 1; index >= 0; index--) {
-      UpdateGlobalRef(ArrayAddressOfElementAt(destinationArray, toIndex + index),
+      UpdateRef(ArrayAddressOfElementAt(destinationArray, toIndex + index),
                       *ArrayAddressOfElementAt(array, fromIndex + index));
     }
   }

runtime/src/main/cpp/Memory.cpp

@@ -27,8 +27,6 @@
 #include "Memory.h"
 #include "Natives.h"
 
-// Define to 1 to use in the multithreaded environment.
-#define CONCURRENT 0
 // If garbage collection algorithm for cyclic garbage to be used.
 #define USE_GC 1
 // Define to 1 to print all memory operations.
@@ -283,16 +281,15 @@ void FreeContainer(ContainerHeader* header) {
   for (int index = 0; index < header->objectCount_; index++) {
     const TypeInfo* typeInfo = obj->type_info();
 
-    // We use *local* versions as no other threads could see dead objects.
     for (int index = 0; index < typeInfo->objOffsetsCount_; index++) {
       ObjHeader** location = reinterpret_cast<ObjHeader**>(
           reinterpret_cast<uintptr_t>(obj + 1) + typeInfo->objOffsets_[index]);
-      UpdateLocalRef(location, nullptr);
+      UpdateRef(location, nullptr);
     }
     // Object arrays are *special*.
     if (typeInfo == theArrayTypeInfo) {
       ArrayHeader* array = obj->array();
-      ReleaseLocalRefs(ArrayAddressOfElementAt(array, 0), array->count_);
+      ReleaseRefs(ArrayAddressOfElementAt(array, 0), array->count_);
     }
     obj = reinterpret_cast<ObjHeader*>(
       reinterpret_cast<uintptr_t>(obj) + objectSize(obj));
@@ -300,10 +297,6 @@ void FreeContainer(ContainerHeader* header) {
 
   // And release underlying memory.
   if (isFreeable(header)) {
-    // TODO: atomic decrement in concurrent case.
-#if CONCURRENT
-    #error "Atomic update of allocCount"
-#endif
     memoryState->allocCount--;
     freeMemory(header);
   }
@@ -487,9 +480,6 @@ MemoryState* InitMemory() {
   memoryState->containers = new ContainerHeaderSet();
 #endif
 #if USE_GC
-#if CONCURRENT
-  #error "Concurrent GC is not yet implemented"
-#endif
   memoryState->toFree = new ContainerHeaderSet();
   memoryState->gcInProgress = false;
   memoryState->gcThreshold = kGcThreshold;
@@ -503,7 +493,7 @@ void DeinitMemory(MemoryState* memoryState) {
   // Free all global objects, to ensure no memory leaks happens.
   for (auto location: *memoryState->globalObjects) {
     fprintf(stderr, "Release global in *%p: %p\n", location, *location);
-    UpdateGlobalRef(location, nullptr);
+    UpdateRef(location, nullptr);
   }
   delete memoryState->globalObjects;
   memoryState->globalObjects = nullptr;
@@ -556,14 +546,7 @@ OBJ_GETTER(AllocArrayInstance, const TypeInfo* type_info, uint32_t elements) {
 
 OBJ_GETTER(InitInstance,
     ObjHeader** location, const TypeInfo* type_info, void (*ctor)(ObjHeader*)) {
-  ObjHeader* sentinel = reinterpret_cast<ObjHeader*>(1);
+  ObjHeader* value = *location;
-  ObjHeader* value;
-  // Wait until other initializers.
-  // TODO: check CONCURRENT!
-  while ((value = __sync_val_compare_and_swap(
-             location, nullptr, sentinel)) == sentinel) {
-    // TODO: consider yielding.
-  }
 
   if (value != nullptr) {
     // OK'ish, inited by someone else.
@@ -571,26 +554,23 @@ OBJ_GETTER(InitInstance,
   }
 
   ObjHeader* object = AllocInstance(type_info, OBJ_RESULT);
-  UpdateGlobalRef(location, object);
+  UpdateRef(location, object);
   try {
     ctor(object);
-#if CONCURRENT
-    // TODO: locking or smth lock-free in MT case?
-#endif
 #if TRACE_MEMORY
     memoryState->globalObjects->push_back(location);
 #endif
     return object;
   } catch (...) {
-    UpdateLocalRef(OBJ_RESULT, nullptr);
+    UpdateRef(OBJ_RESULT, nullptr);
-    UpdateGlobalRef(location, nullptr);
+    UpdateRef(location, nullptr);
     throw;
   }
 }
 
-void SetLocalRef(ObjHeader** location, const ObjHeader* object) {
+void SetRef(ObjHeader** location, const ObjHeader* object) {
 #if TRACE_MEMORY
-  fprintf(stderr, "SetLocalRef *%p: %p\n", location, object);
+  fprintf(stderr, "SetRef *%p: %p\n", location, object);
 #endif
   *const_cast<const ObjHeader**>(location) = object;
   if (object != nullptr) {
@@ -598,19 +578,6 @@ void SetLocalRef(ObjHeader** location, const ObjHeader* object) {
   }
 }
 
-void SetGlobalRef(ObjHeader** location, const ObjHeader* object) {
-#if TRACE_MEMORY
-  fprintf(stderr, "SetGlobalRef *%p: %p\n", location, object);
-#endif
-  *const_cast<const ObjHeader**>(location) = object;
-   if (object != nullptr) {
-      AddRef(object);
-   }
-#if CONCURRENT
-   // TODO: memory fence here.
-#endif
-}
-
 void UpdateReturnRef(ObjHeader** returnSlot, const ObjHeader* object) {
   if (isArenaSlot(returnSlot)) return;
   ObjHeader* old = *returnSlot;
@@ -628,11 +595,11 @@ void UpdateReturnRef(ObjHeader** returnSlot, const ObjHeader* object) {
   }
 }
 
-void UpdateLocalRef(ObjHeader** location, const ObjHeader* object) {
+void UpdateRef(ObjHeader** location, const ObjHeader* object) {
   RuntimeAssert(!isArenaSlot(location), "must not be a slot");
   ObjHeader* old = *location;
 #if TRACE_MEMORY
-  fprintf(stderr, "UpdateLocalRef *%p: %p -> %p\n", location, old, object);
+  fprintf(stderr, "UpdateRef *%p: %p -> %p\n", location, old, object);
 #endif
   if (old != object) {
     if (object != nullptr) {
@@ -645,39 +612,11 @@ void UpdateLocalRef(ObjHeader** location, const ObjHeader* object) {
   }
 }
 
-void UpdateGlobalRef(ObjHeader** location, const ObjHeader* object) {
-  RuntimeAssert(!isArenaSlot(location), "Must not be an arena");
-#if CONCURRENT
-  ObjHeader* old = *location;
-#if TRACE_MEMORY
-  fprintf(stderr, "UpdateGlobalRef *%p: %p -> %p\n", location, old, object);
-#endif
-  if (old != object) {
-    if (object != nullptr) {
-      AddRef(object);
-    }
-    bool written = __sync_bool_compare_and_swap(
-        location, old, const_cast<ObjHeader*>(object));
-    if (written) {
-      if (old > reinterpret_cast<ObjHeader*>(1)) {
-        ReleaseRef(old);
-      }
-    } else {
-      if (object != nullptr) {
-        ReleaseRef(object);
-      }
-    }
-  }
-#else
-  UpdateLocalRef(location, object);
-#endif
-}
-
 void LeaveFrame(ObjHeader** start, int count) {
 #if TRACE_MEMORY
     fprintf(stderr, "LeaveFrame %p .. %p\n", start, start + count);
 #endif
-  ReleaseLocalRefs(start + 1, count - 1);
+  ReleaseRefs(start + 1, count - 1);
   if (*start != nullptr) {
     auto arena = initedArena(start);
 #if TRACE_MEMORY
@@ -688,9 +627,9 @@ void LeaveFrame(ObjHeader** start, int count) {
   }
 }
 
-void ReleaseLocalRefs(ObjHeader** start, int count) {
+void ReleaseRefs(ObjHeader** start, int count) {
 #if TRACE_MEMORY
-  fprintf(stderr, "ReleaseLocalRefs %p .. %p\n", start, start + count);
+  fprintf(stderr, "ReleaseRefs %p .. %p\n", start, start + count);
 #endif
   ObjHeader** current = start;
   while (count-- > 0) {
@@ -704,36 +643,6 @@ void ReleaseLocalRefs(ObjHeader** start, int count) {
   }
 }
 
-void ReleaseGlobalRefs(ObjHeader** start, int count) {
-#if TRACE_MEMORY
-  fprintf(stderr, "ReleaseGlobalRefs %p .. %p\n", start, start + count);
-#endif
-#if CONCURRENT
-  ObjHeader** current = start;
-  while (count-- > 0) {
-    ObjHeader* object = *current;
-    if (object != nullptr) {
-      bool written = __sync_bool_compare_and_swap(
-          current, object, nullptr);
-      if (written)
-        ReleaseRef(object);
-    }
-    current++;
-  }
-#else
-  ObjHeader** current = start;
-  while (count-- > 0) {
-    ObjHeader* object = *current;
-    if (object != nullptr) {
-      ReleaseRef(object);
-      // Usually required.
-      *current = nullptr;
-    }
-    current++;
-  }
-#endif
-}
-
 #if USE_GC
 void GarbageCollect() {
   RuntimeAssert(memoryState->toFree != nullptr, "GC must not be stopped");

runtime/src/main/cpp/Memory.h

@@ -25,8 +25,8 @@
 typedef enum {
   // Container is normal thread local container.
   CONTAINER_TAG_NORMAL = 0,
-   // Container shall be atomically refcounted.
+  // Container shall be atomically refcounted, currently disabled.
-  CONTAINER_TAG_SHARED = 1,
+  // CONTAINER_TAG_SHARED = 1,
   // Those container tags shall not be refcounted.
   // Permanent object, cannot refer to non-permanent objects, so no need to cleanup those.
   CONTAINER_TAG_PERMANENT = 2,
@@ -142,9 +142,6 @@ inline void AddRef(ContainerHeader* header) {
     case CONTAINER_TAG_NORMAL:
       header->refCount_ += CONTAINER_TAG_INCREMENT;
       break;
-    case CONTAINER_TAG_SHARED:
-      __sync_fetch_and_add(&header->refCount_, CONTAINER_TAG_INCREMENT);
-      break;
     default:
       RuntimeAssert(false, "unknown container type");
       break;
@@ -167,26 +164,6 @@ inline bool Release(ContainerHeader* header) {
         return true;
       }
       break;
-    // Note that shared containers have potentially subtle race, if object holds a
-    // reference to another object, stored in shorter living container. In this
-    // case there's unlikely, but possible case, where one mutator takes reference,
-    // from the field, but not yet AddRef'ed it, while another mutator updates
-    // field with another value, and thus Release's same field.
-    // If those two updates happens concurrently - it may lead to dereference of stale
-    // pointer.
-    // It seems not a very big issue as:
-    //  - if objects stored in the same container, race will never happen
-    //  - if concurrent field access is under lock, race will never happen
-    //  - container likely groups multiple objects, so object release will lead to
-    //    container release only in relatively few cases, which will decrease race
-    //    probability even further.
-    case CONTAINER_TAG_SHARED:
-      if (__sync_sub_and_fetch(
-              &header->refCount_, CONTAINER_TAG_INCREMENT) == CONTAINER_TAG_SHARED) {
-        FreeContainer(header);
-        return true;
-      }
-      break;
     default:
       RuntimeAssert(false, "unknown container type");
       break;
@@ -364,18 +341,14 @@ OBJ_GETTER(InitInstance, ObjHeader** location, const TypeInfo* type_info,
 //    in intermediate frames when throwing
 //
 
-// Sets locally visible location.
+// Sets location.
-void SetLocalRef(ObjHeader** location, const ObjHeader* object) RUNTIME_NOTHROW;
+void SetRef(ObjHeader** location, const ObjHeader* object) RUNTIME_NOTHROW;
-// Sets potentially globally visible location.
+// Updates location.
-void SetGlobalRef(ObjHeader** location, const ObjHeader* object) RUNTIME_NOTHROW;
+void UpdateRef(ObjHeader** location, const ObjHeader* object) RUNTIME_NOTHROW;
-// Update locally visible location.
+// Updates reference in return slot.
-void UpdateLocalRef(ObjHeader** location, const ObjHeader* object) RUNTIME_NOTHROW;
-// Update potentially globally visible location.
-void UpdateGlobalRef(ObjHeader** location, const ObjHeader* object) RUNTIME_NOTHROW;
-// Update reference in return slot.
 void UpdateReturnRef(ObjHeader** returnSlot, const ObjHeader* object) RUNTIME_NOTHROW;
 // Optimization: release all references in range.
-void ReleaseLocalRefs(ObjHeader** start, int count) RUNTIME_NOTHROW;
+void ReleaseRefs(ObjHeader** start, int count) RUNTIME_NOTHROW;
 // Called on frame leave, if it has object slots.
 void LeaveFrame(ObjHeader** start, int count) RUNTIME_NOTHROW;
 // Collect garbage, which cannot be found by reference counting (cycles).
@@ -391,10 +364,10 @@ class ObjHolder {
    ObjHolder() : obj_(nullptr) {}
 
    explicit ObjHolder(const ObjHeader* obj) {
-     ::SetLocalRef(&obj_, obj);
+     ::SetRef(&obj_, obj);
    }
    ~ObjHolder() {
-     ::UpdateLocalRef(&obj_, nullptr);
+     ::UpdateRef(&obj_, nullptr);
    }
 
    ObjHeader* obj() { return obj_; }