Lazy calculations for strings

Kotlin · Aug 12, 2021 · 12a60d8 · 12a60d8
1 parent 7b286a3
commit 12a60d8
Show file tree

Hide file tree

Showing 8 changed files with 58 additions and 60 deletions.
diff --git a/build-plugin/src/build/BuildSettingsExtension.kt b/build-plugin/src/build/BuildSettingsExtension.kt
@@ -48,6 +48,7 @@ class BuildSettingsExtension(private val project: Project) {
 
         fun skipPrereleaseCheck() = args.add("-Xskip-prerelease-check")
         fun requiresOptIn() = args.add("-Xopt-in=kotlin.RequiresOptIn")
+        fun allowResultReturnType() = args.add("-Xallow-result-return-type")
     }
 
     fun withCompilerArgs(configure: KotlinCompilerArgsBuilder.() -> Unit) {

diff --git a/jupyter-lib/api/build.gradle.kts b/jupyter-lib/api/build.gradle.kts
@@ -27,6 +27,7 @@ buildSettings {
     withTests()
     withCompilerArgs {
         requiresOptIn()
+        allowResultReturnType()
     }
 }
 

diff --git a/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableState.kt b/jupyter-lib/api/src/main/kotlin/org/jetbrains/kotlinx/jupyter/api/VariableState.kt
@@ -1,6 +1,5 @@
 package org.jetbrains.kotlinx.jupyter.api
 
-import java.lang.reflect.InvocationTargetException
 import kotlin.reflect.KProperty
 import kotlin.reflect.KProperty1
 import kotlin.reflect.full.declaredMemberProperties
@@ -10,60 +9,61 @@ interface VariableState {
     val property: KProperty<*>
     val scriptInstance: Any?
     val stringValue: String?
-    val value: Any?
+    val value: Result<Any?>
 }
 
 data class VariableStateImpl(
     override val property: KProperty1<Any, *>,
     override val scriptInstance: Any,
 ) : VariableState {
-    private var cachedValue: Any? = null
+    private var cachedValue: Result<Any?> = Result.success(null)
     private var isRecursive: Boolean = false
 
-    fun update() {
+    fun update(): Boolean {
         val wasAccessible = property.isAccessible
         property.isAccessible = true
-        val fieldValue = property.get(scriptInstance)
+        val fieldValue = try {
+            Result.success(property.get(scriptInstance))
+        } catch (ex: Throwable) {
+            Result.failure<Any?>(ex)
+        }
         property.isAccessible = wasAccessible
-        cachedValue = fieldValue
-        handleRecursiveCase()
-    }
-
-    override val stringValue: String?
-        get() = getProperString()
 
-    override val value: Any?
-        get() = cachedValue
-
-    private fun handleRecursiveCase() {
-        if (cachedValue == null) return
-        val seenNodes = mutableSetOf<Any>()
-        goThrough(cachedValue, seenNodes)
+        val isChanged = cachedValue.getOrNull() !== fieldValue.getOrNull()
+        cachedValue = fieldValue
+        return isChanged
     }
 
-    private fun getProperString(): String? {
-        fun getRecursiveObjectCode(): String {
-            val kClassName = cachedValue!!::class.simpleName
-            return buildString {
-                append("$kClassName: ")
-                append("recursive structure")
-            }
+    override val stringValue: String? by lazy {
+        fun getRecursiveObjectName(): String {
+            val kClassName = cachedValue.getOrNull()!!::class.simpleName
+            return "$kClassName: recursive structure"
         }
-        if (cachedValue == null) return null
-        return if (!isRecursive) {
-            cachedValue.toString()
+        if (cachedValue.getOrNull() == null) {
+            return@lazy null
+        }
+        handleIfRecursiveStructure()
+
+        if (!isRecursive) {
+            cachedValue.getOrNull().toString()
         } else {
-            getRecursiveObjectCode()
+            getRecursiveObjectName()
         }
     }
 
-    private fun goThrough(value: Any?, seenNodes: MutableSet<Any>) {
+    override val value: Result<Any?>
+        get() = cachedValue
+
+    private fun handleIfRecursiveStructure() {
+        if (cachedValue.getOrNull() == null) return
+        traverseObject(cachedValue.getOrNull(), mutableSetOf())
+    }
+
+    private fun traverseObject(value: Any?, seenNodes: MutableSet<Any>) {
         if (value == null) return
         val membersProperties = try {
             value::class.declaredMemberProperties
-        } catch (e: Error) {
-            emptyList<Collection<KProperty1<Any, *>>>()
-        } catch (e: Exception) {
+        } catch (ex: Throwable) {
             emptyList<Collection<KProperty1<Any, *>>>()
         }
 
@@ -75,24 +75,23 @@ data class VariableStateImpl(
                 property.isAccessible = true
                 val callInstance = property.get(value)
                 property.isAccessible = wasAccessible
-                val wasSeen = if (callInstance != null) !seenNodes.add(callInstance) else false
+                val wasSeen = callInstance != null && !seenNodes.add(callInstance)
 
                 if (wasSeen) {
                     isRecursive = true
                     return
                 }
                 receivedInstances.add(callInstance)
-            } catch (ex: Error) {
+            } catch (ex: Throwable) {
+                // there might be recursive elements inside the container
                 if (ex is StackOverflowError) {
                     isRecursive = true
                 }
                 return
-            } catch (ex: InvocationTargetException) {
-                return
             }
         }
         receivedInstances.forEach {
-            goThrough(it, seenNodes)
+            traverseObject(it, seenNodes)
         }
     }
 }
diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt
@@ -419,7 +419,7 @@ class ReplForJupyterImpl(
             // printUsagesInfo(jupyterId, cellVariables[jupyterId - 1])
 
 
-            val variablesStateUpdate = notebook.variablesState.mapValues { it.value.stringValue }
+            val variablesStateUpdate = notebook.variablesState.mapValues { "" }
             EvalResultEx(
                 result.result.value,
                 rendered,

diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt
@@ -146,10 +146,8 @@ internal class InternalEvaluatorImpl(
 
         variablesHolder.forEach {
             val state = it.value as VariableStateImpl
-            val oldValue = state.value
-            state.update()
 
-            if (state.value?.equals(oldValue) == false) {
+            if (state.update()) {
                 variablesWatcher.addUsage(cellId, it.key)
             }
         }

diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/ApiTest.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/ApiTest.kt
@@ -59,7 +59,6 @@ class ApiTest : AbstractSingleReplTest() {
             "y" to "abc",
             "z" to "47"
         )
-        assertEquals(res.metadata.evaluatedVariablesState, varsUpdate)
         val htmlText = generateHTMLVarsReport(repl.notebook.variablesState)
         assertEquals(
             """
@@ -94,5 +93,6 @@ class ApiTest : AbstractSingleReplTest() {
             """.trimIndent(),
             htmlText
         )
+        assertEquals(varsUpdate, repl.notebook.variablesState.mapToStringValues())
     }
 }
diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt
@@ -14,6 +14,7 @@ import org.jetbrains.kotlinx.jupyter.repl.ListErrorsResult
 import org.jetbrains.kotlinx.jupyter.test.getOrFail
 import org.jetbrains.kotlinx.jupyter.test.getStringValue
 import org.jetbrains.kotlinx.jupyter.test.getValue
+import org.jetbrains.kotlinx.jupyter.test.mapToStringValues
 import org.jetbrains.kotlinx.jupyter.withPath
 import org.junit.jupiter.api.Assertions.assertTrue
 import org.junit.jupiter.api.Test
@@ -450,7 +451,7 @@ class ReplVarsTest : AbstractSingleReplTest() {
     @Test
     fun testVarsStateConsistency() {
         assertTrue(repl.notebook.variablesState.isEmpty())
-        val res = eval(
+        eval(
             """
             val x = 1 
             val y = 0
@@ -463,7 +464,7 @@ class ReplVarsTest : AbstractSingleReplTest() {
             "y" to "0",
             "z" to "47"
         )
-        assertEquals(res.metadata.evaluatedVariablesState, varsUpdate)
+        assertEquals(varsUpdate, repl.notebook.variablesState.mapToStringValues())
         assertFalse(repl.notebook.variablesState.isEmpty())
         val varsState = repl.notebook.variablesState
         assertEquals("1", varsState.getStringValue("x"))
@@ -489,7 +490,7 @@ class ReplVarsTest : AbstractSingleReplTest() {
 
     @Test
     fun testVarsCapture() {
-        val res = eval(
+        eval(
             """
             val x = 1 
             val y = "abc"
@@ -498,13 +499,10 @@ class ReplVarsTest : AbstractSingleReplTest() {
         )
         val varsState = repl.notebook.variablesState
         assertTrue(varsState.isNotEmpty())
-        val strState = mutableMapOf<String, String>()
-        varsState.forEach {
-            strState[it.key] = it.value.stringValue ?: return@forEach
-        }
+        val strState = varsState.mapToStringValues()
 
-        val returnedState = res.metadata.evaluatedVariablesState
-        assertEquals(strState, returnedState)
+        val goalState = mapOf("x" to "1", "y" to "abc", "z" to "1")
+        assertEquals(strState, goalState)
         assertEquals(1, varsState.getValue("x"))
         assertEquals("abc", varsState.getStringValue("y"))
         assertEquals("1", varsState.getStringValue("z"))
@@ -552,7 +550,7 @@ class ReplVarsTest : AbstractSingleReplTest() {
 
     @Test
     fun testPrivateVarsCapture() {
-        val res = eval(
+        eval(
             """
             private val x = 1 
             private val y = "abc"
@@ -561,13 +559,10 @@ class ReplVarsTest : AbstractSingleReplTest() {
         )
         val varsState = repl.notebook.variablesState
         assertTrue(varsState.isNotEmpty())
-        val strState = mutableMapOf<String, String>()
-        varsState.forEach {
-            strState[it.key] = it.value.stringValue ?: return@forEach
-        }
+        val strState = varsState.mapValues { it.value.stringValue }
 
-        val returnedState = res.metadata.evaluatedVariablesState
-        assertEquals(strState, returnedState)
+        val goalState = mapOf("x" to "1", "y" to "abc", "z" to "1")
+        assertEquals(strState, goalState)
         assertEquals(1, varsState.getValue("x"))
         assertEquals("abc", varsState.getStringValue("y"))
         assertEquals("1", varsState.getStringValue("z"))

diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/testUtil.kt
@@ -103,12 +103,16 @@ fun CompletionResult.getOrFail(): CompletionResult.Success = when (this) {
     else -> fail("Result should be success")
 }
 
+fun Map<String, VariableState>.mapToStringValues(): Map<String, String?> {
+    return mapValues { it.value.stringValue }
+}
+
 fun Map<String, VariableState>.getStringValue(variableName: String): String? {
     return get(variableName)?.stringValue
 }
 
 fun Map<String, VariableState>.getValue(variableName: String): Any? {
-    return get(variableName)?.value
+    return get(variableName)?.value?.getOrNull()
 }
 
 class InMemoryLibraryResolver(