azalea/kotlin-fork
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[LL FIR] do not rethrow exceptions from caches

Browse Source 
Previously, the exceptions from ValueWithPostCompute was saved in cache and rethrown.
This was needed to avoid multiple heavy calculation which will be still uncecessfull.
Currently we do not have much of such exceptions and such thing may break perf test
by throwing exceptions from old test invocation
This commit is contained in:
Ilya Kirillov
2023-01-13 16:22:57 +01:00
committed by Space Team
parent 06074e5e2d
commit a5082e5d94
2 changed files with 5 additions and 106 deletions
Download Patch File Download Diff File Expand all files Collapse all files
analysis/low-level-api-fir/src/org/jetbrains/kotlin/analysis/low/level/api/fir/caches/ValueWithPostCompute.kt
+5 -27
View File
@@ -6,7 +6,6 @@
package org.jetbrains.kotlin.analysis.low.level.api.fir.fir.caches
import org.jetbrains.kotlin.analysis.low.level.api.fir.util.lockWithPCECheck
import org.jetbrains.kotlin.util.shouldIjPlatformExceptionBeRethrown
import java.util.concurrent.locks.ReentrantLock
/**
@@ -36,7 +35,6 @@ internal class ValueWithPostCompute<KEY, VALUE, DATA>(
/**
* can be in one of the following three states:
* [ValueIsNotComputed] -- value is not initialized and thread are now executing [_postCompute]
* [ExceptionWasThrownDuringValueComputation] -- exception was thrown during value computation, it will be rethrown on every value access
* [ValueIsPostComputingNow] -- thread with threadId has computed the value and only it can access it during post compute
* some value of type [VALUE] -- value is computed and post compute was executed, values is visible for all threads
*
@@ -61,16 +59,13 @@ internal class ValueWithPostCompute<KEY, VALUE, DATA>(
return stateSnapshot.value as VALUE
} else {
lock?.lockWithPCECheck(LOCKING_INTERVAL_MS) { // wait until other thread which holds the lock now computes the value
when (val newStateSnapshot = value) {
when (value) {
ValueIsNotComputed -> {
// if we have a PCE during value computation, then we will enter the critical section with `value == ValueIsNotComputed`
// in this case, we should try to recalculate the value
return computeValueWithoutLock()
}
is ExceptionWasThrownDuringValueComputation -> {
// if some other thread tried to compute the value but failed with the exception
throw newStateSnapshot.error
}
else -> {
// other thread computed the value for us
return value as VALUE
@@ -83,9 +78,6 @@ internal class ValueWithPostCompute<KEY, VALUE, DATA>(
return computeValueWithoutLock()
} ?: return value as VALUE
is ExceptionWasThrownDuringValueComputation -> {
throw stateSnapshot.error
}
else -> {
return stateSnapshot as VALUE
}
@@ -98,15 +90,11 @@ internal class ValueWithPostCompute<KEY, VALUE, DATA>(
// if we entered synchronized section that's mean that the value is not yet calculated and was not started to be calculated
// or the some other thread calculated the value while we were waiting to acquire the lock
when (val newStateSnapshot = value) {
when (value) {
ValueIsNotComputed -> {
// will be computed later, the read of `ValueIsNotComputed` guarantees that lock is not null
require(lock!!.isHeldByCurrentThread)
}
is ExceptionWasThrownDuringValueComputation -> {
// if some other thread tried to compute the value but failed with the exception
throw newStateSnapshot.error
}
else -> {
// other thread computed the value for us and set `lock` to null
require(lock == null)
@@ -122,11 +110,7 @@ internal class ValueWithPostCompute<KEY, VALUE, DATA>(
_postCompute!!(key, calculated, data)
calculated
} catch (e: Throwable) {
if (e is Exception && exceptionShouldBeSavedInCache(e)) {
value = ExceptionWasThrownDuringValueComputation(e)
} else {
value = ValueIsNotComputed
}
value = ValueIsNotComputed
throw e
}
// reading lock = null implies that the value is calculated and stored
@@ -138,20 +122,14 @@ internal class ValueWithPostCompute<KEY, VALUE, DATA>(
return calculatedValue
}
private fun exceptionShouldBeSavedInCache(exception: Exception): Boolean =
!shouldIjPlatformExceptionBeRethrown(exception)
@Suppress("UNCHECKED_CAST")
fun getValueIfComputed(): VALUE? = when (val snapshot = value) {
fun getValueIfComputed(): VALUE? = when (value) {
ValueIsNotComputed -> null
is ValueIsPostComputingNow -> null
is ExceptionWasThrownDuringValueComputation -> throw snapshot.error
else -> value as VALUE
}
private class ValueIsPostComputingNow(val value: Any?, val threadId: Long)
private class ExceptionWasThrownDuringValueComputation(val error: Throwable)
private object ValueIsNotComputed
}
analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/caches/ValueWithPostComputeTest.kt
-79
View File
@@ -69,25 +69,6 @@ class ValueWithPostComputeTest {
Assertions.assertNotEquals(pceOnFirstAccess, pceOnSecondAccess, "different PCE should be thrown on every access")
}
@Test
fun testNonRecoverableExceptionISavedInCache() {
val valueWithPostCompute = ValueWithPostCompute(
key = 1,
calculate = { "value" to Unit },
postCompute = { _, _, _ ->
throw IllegalStateException()
}
)
val exceptionOnFirstAccess = kotlin.runCatching { valueWithPostCompute.getValue() }.exceptionOrNull()
Assertions.assertInstanceOf(IllegalStateException::class.java, exceptionOnFirstAccess)
val exceptionOnSecondAccess = kotlin.runCatching { valueWithPostCompute.getValue() }.exceptionOrNull()
Assertions.assertInstanceOf(IllegalStateException::class.java, exceptionOnSecondAccess)
Assertions.assertEquals(exceptionOnFirstAccess, exceptionOnSecondAccess, "the same exception should be thrown on every access")
}
/**
* Tests the following scenario:
* - thread `t1` access the cache and executes `calculate()` and then `postCompute()` under a lock hold
@@ -145,64 +126,4 @@ class ValueWithPostComputeTest {
}
}
}
/**
* Tests the following scenario:
* - thread `t1` access the cache and executes `calculate()` and then `postCompute()` under a lock hold
* - while the lock hold by `t1`, `t2` tries to also access the value and waits for the lock to be released by `t1`
* - t1: during the post compute, some non-recoverable exception happens inside the `postCompute()` and exception is saved in the cache
* - t1 releases the lock with the `value` set to `ExceptionWasThrownDuringValueComputation`
* - t2 acquires the lock and should correctly see `ExceptionWasThrownDuringValueComputation` and rethrow the exception
*/
@Test
fun testSavedExceptionInCacheFromPostCompute() {
class ExceptionWhichWillBeSavedInCache : Exception()
for (i in 1..100) {
val t1CalledCalculate = CountDownLatch(1)
val t2AccessedTheCache = CountDownLatch(1)
val resultRef = AtomicReference<Any?>(null)
val valueWithPostCompute = ValueWithPostCompute(
key = 1,
calculate = {
if (Thread.currentThread().name == "t1") {
t1CalledCalculate.countDown()
}
Thread.currentThread().name to Unit
},
postCompute = { _, _, _ ->
t2AccessedTheCache.await()
if (Thread.currentThread().name == "t1") {
throw ExceptionWhichWillBeSavedInCache()
}
}
)
val t1 = thread(name = "t1") {
try {
valueWithPostCompute.getValue()
} catch (_: Throwable) {
}
}
val t2 = thread(name = "t2") {
t1CalledCalculate.await()
t2AccessedTheCache.countDown()
try {
resultRef.set(valueWithPostCompute.getValue())
} catch (e: Throwable) {
resultRef.set(e)
}
}
t2.join()
t1.join()
when (val result = resultRef.get()) {
is ExceptionWhichWillBeSavedInCache -> {}
is Throwable -> throw result
else -> Assertions.fail("Unexpected value $result")
}
}
}
}