Very preliminary relaxed mode draft. #3129
Conversation
olonho
force-pushed
the
relaxed-mode
branch
3 times, most recently
from
a6e3145 to
a8f1e8b
Compare
| void AddRefFromAssociatedObject(const ObjHeader* object) RUNTIME_NOTHROW; | ||
| void ReleaseRefFromAssociatedObject(const ObjHeader* object) RUNTIME_NOTHROW; | ||
| void ReleaseRefFromAssociatedObject(const ObjHeader* object); |
There was a problem hiding this comment.
Is RUNTIME_NOTHROW removed intentionally?
| if (container != nullptr) | ||
| enqueueDecrementRC</* CanCollect = */ true>(container); | ||
| if (value != nullptr) | ||
| ReleaseHeapRef(value); |
There was a problem hiding this comment.
Can this remove object before return? (like it was in swapHeapRefLocked).
runtime/src/main/cpp/Memory.cpp
Outdated
| @@ -1825,12 +1792,12 @@ OBJ_GETTER(swapHeapRefLocked, | |||
| shallRelease = oldValue != nullptr; | |||
| } | |||
| unlock(spinlock); | |||
| if (shallRelease) { | |||
| releaseHeapRef(oldValue); | |||
| } | |||
| // No need to rememberNewContainer(), as oldValue is already | |||
| // present on this worker. | |||
There was a problem hiding this comment.
Btw, is this correct? Doesn't seem so when oldValue != expectedValue.
There was a problem hiding this comment.
Comment was indeed wrong.
There was a problem hiding this comment.
Comment is still wrong, and the behaviour is wrong too.
The following test crashes:
import kotlin.test.*
import kotlin.native.concurrent.*
val atomicRef = AtomicReference<Any?>(Any().freeze())
val semaphore = AtomicInt(0)
@Test fun runTest() {
semaphore.value = 0
val worker = Worker.start()
val future = worker.execute(TransferMode.SAFE, { null }) {
val value = atomicRef.compareAndSwap(null, null)
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
}(based on worker10).
Generally atomicRef.compareAndSwap(Any().freeze(), null) is the same as atomicRef.value, and if the latter requires rememberNewContainer, then the former requires this too.
There was a problem hiding this comment.
Thanks for bringing my attention to this problem. Fixed.
runtime/src/main/cpp/Memory.cpp
Outdated
| zeroStackRef<true>(location); | ||
| } | ||
| void ZeroStackRefRelaxed(ObjHeader** location) { | ||
| zeroStackRef<true>(location); |
There was a problem hiding this comment.
Is true correct here?
| /** | ||
| * If binary was compiled in debug mode. | ||
| */ | ||
| public val isDebugBinary: Boolean |
There was a problem hiding this comment.
Maybe related: #2562
| @@ -163,4 +163,22 @@ val stableHolder2 = StableRef.create(("hello" to "world").freeze()) | |||
| semaphore.increment() | |||
| future.result | |||
| worker.requestTermination().result | |||
| } | |||
|
|
|||
| @Test fun runTest6() { | |||
There was a problem hiding this comment.
This test doesn't fail even without the fix applied.
There was a problem hiding this comment.
Yeah, better use fresh atomic ref here.
runtime/src/main/cpp/Memory.cpp
Outdated
| ReleaseHeapRef(oldValue); | ||
| } else { | ||
| if (IsStrictMemoryModel && oldValue != expectedValue) | ||
| rememberNewContainer(oldValue->container()); |
There was a problem hiding this comment.
Isn't it too late to perform this after unlock?
|
Could we get some documentation on how to enable this relaxed mode? |
No description provided.