diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt index 2d30ea7dc..c703c7128 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt @@ -402,6 +402,8 @@ class ReplForJupyterImpl( val compiledData: SerializedCompiledScriptsData val newImports: List + val oldDeclarations: MutableMap = mutableMapOf() + oldDeclarations.putAll(internalEvaluator.getVariablesDeclarationInfo()) val result = try { log.debug("Current cell id: $jupyterId") executor.execute(code, displayHandler, currentCellId = jupyterId - 1) { internalId, codeToExecute -> @@ -431,7 +433,7 @@ class ReplForJupyterImpl( // printVars() // printUsagesInfo(jupyterId, cellVariables[jupyterId - 1]) val variablesCells: Map = notebook.variablesState.mapValues { internalEvaluator.findVariableCell(it.key) } - val serializedData = variablesSerializer.serializeVariables(jupyterId - 1, notebook.variablesState, variablesCells, notebook.unchangedVariables()) + val serializedData = variablesSerializer.serializeVariables(jupyterId - 1, notebook.variablesState, oldDeclarations, variablesCells, notebook.unchangedVariables()) GlobalScope.launch(Dispatchers.Default) { variablesSerializer.tryValidateCache(jupyterId - 1, notebook.cellVariables) diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt index 6082b5891..b2c33b8f1 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/InternalEvaluator.kt @@ -36,6 +36,8 @@ interface InternalEvaluator { */ fun findVariableCell(variableName: String): Int + fun getVariablesDeclarationInfo(): Map + /** * Returns a set of unaffected variables after execution */ 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 index 95c280885..308c1025b 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl/impl/InternalEvaluatorImpl.kt @@ -52,6 +52,8 @@ internal class InternalEvaluatorImpl( return variablesWatcher.findDeclarationAddress(variableName) ?: -1 } + override fun getVariablesDeclarationInfo(): Map = variablesWatcher.variablesDeclarationInfo + override fun getUnchangedVariables(): Set { return variablesWatcher.getUnchangedVariables() } diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/serializationUtils.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/serializationUtils.kt index 2bafee86b..3fcb23586 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/serializationUtils.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/serializationUtils.kt @@ -56,8 +56,7 @@ class ProcessedSerializedVarsState( data class ProcessedDescriptorsState( val processedSerializedVarsToJavaProperties: MutableMap = mutableMapOf(), val processedSerializedVarsToKTProperties: MutableMap = mutableMapOf(), - val instancesPerState: MutableMap = mutableMapOf(), - val parent: ProcessedDescriptorsState? = null + val instancesPerState: MutableMap = mutableMapOf() ) data class RuntimeObjectWrapper( @@ -73,6 +72,7 @@ data class RuntimeObjectWrapper( return objectInstance === other } + // TODO: it's not changing after recreation override fun hashCode(): Int { return if (isRecursive) Random.nextInt() else objectInstance?.hashCode() ?: 0 } @@ -93,7 +93,7 @@ fun Any?.getToStringValue(isRecursive: Boolean = false): String { } fun Any?.getUniqueID(isRecursive: Boolean = false): String { - return if (this != null) { + return if (this != null && this !is Map.Entry<*, *>) { val hashCode = if (isRecursive) { Random.nextLong() } else { @@ -197,8 +197,9 @@ class VariablesSerializer( val isRecursive = stringedValue.contains(": recursive structure") if (!isRecursive && simpleTypeName == "LinkedEntrySet") { getProperEntrySetRepresentation(value) - } else - value.getUniqueID(isRecursive) + } else { + value.getUniqueID(isRecursive) + } } else { "" } @@ -285,12 +286,12 @@ class VariablesSerializer( } /** - * Map of Map of seen objects. - * First Key: cellId + * Map of Map of seen objects related to a particular variable serialization + * First Key: topLevel variable Name * Second Key: actual value * Value: serialized VariableState */ - private val seenObjectsPerCell: MutableMap> = mutableMapOf() + private val seenObjectsPerVariable: MutableMap> = mutableMapOf() private var currentSerializeCount: Int = 0 @@ -355,7 +356,7 @@ class VariablesSerializer( override suspend fun clearStateInfo(currentState: Int) { computedDescriptorsPerCell.remove(currentState) - seenObjectsPerCell.remove(currentState) + // seenObjectsPerVariable.remove(currentState) } suspend fun tryValidateCache(currentCellId: Int, cellVariables: Map>) { @@ -363,10 +364,11 @@ class VariablesSerializer( clearOldData(currentCellId, cellVariables) } - fun serializeVariables(cellId: Int, variablesState: Map, variablesCells: Map, unchangedVariables: Set): Map { + fun serializeVariables(cellId: Int, variablesState: Map, oldDeclarations: Map, variablesCells: Map, unchangedVariables: Set): Map { fun removeNonExistingEntries() { val toRemoveSet = mutableSetOf() serializedVariablesCache.forEach { (name, _) -> + // seems like this never gonna happen if (!variablesState.containsKey(name)) { toRemoveSet.add(name) } @@ -376,9 +378,6 @@ class VariablesSerializer( if (!isSerializationActive) return emptyMap() - if (seenObjectsPerCell.containsKey(cellId)) { - seenObjectsPerCell[cellId]!!.clear() - } if (variablesState.isEmpty()) { return emptyMap() } @@ -388,21 +387,26 @@ class VariablesSerializer( if (wasRedeclared) { removedFromSightVariables.remove(it) } - // todo: might consider self-recursive elements always to recompute since it's non comparable via strings + // TODO: might consider self-recursive elements always to recompute since it's non comparable via strings if (serializedVariablesCache.isEmpty()) { true - } else - (!unchangedVariables.contains(it) || serializedVariablesCache[it]?.value != variablesState[it]?.stringValue) && - !removedFromSightVariables.contains(it) + } else { + (!unchangedVariables.contains(it) || serializedVariablesCache[it]?.value != variablesState[it]?.stringValue) && + !removedFromSightVariables.contains(it) + } } log.debug("Variables state as is: $variablesState") log.debug("Serializing variables after filter: $neededEntries") log.debug("Unchanged variables: ${unchangedVariables - neededEntries.keys}") // remove previous data - // computedDescriptorsPerCell[cellId]?.instancesPerState?.clear() val serializedData = neededEntries.mapValues { val actualCell = variablesCells[it.key] ?: cellId + if (oldDeclarations.containsKey(it.key)) { + val oldCell = oldDeclarations[it.key]!! + computedDescriptorsPerCell[oldCell]?.remove(it.key) + seenObjectsPerVariable.remove(it.key) + } serializeVariableState(actualCell, it.key, it.value) } @@ -467,10 +471,13 @@ class VariablesSerializer( return doActualSerialization(cellId, topLevelName, processedData, wrapper, isRecursive, isOverride) } - private fun doActualSerialization(cellId: Int, topLevelName:String, processedData: ProcessedSerializedVarsState, value: RuntimeObjectWrapper, isRecursive: Boolean, isOverride: Boolean = true): SerializedVariablesState { + private fun doActualSerialization(cellId: Int, topLevelName: String, processedData: ProcessedSerializedVarsState, value: RuntimeObjectWrapper, isRecursive: Boolean, isOverride: Boolean = true): SerializedVariablesState { + fun checkIsNotStandardDescriptor(descriptor: MutableMap): Boolean { + return descriptor.isNotEmpty() && !descriptor.containsKey("size") && !descriptor.containsKey("data") + } val serializedVersion = processedData.serializedVariablesState - seenObjectsPerCell.putIfAbsent(cellId, mutableMapOf()) + seenObjectsPerVariable.putIfAbsent(topLevelName, mutableMapOf()) computedDescriptorsPerCell.putIfAbsent(cellId, mutableMapOf()) if (isOverride) { @@ -482,17 +489,21 @@ class VariablesSerializer( } val currentCellDescriptors = computedDescriptorsPerCell[cellId]?.get(topLevelName) // TODO should we stack? + // i guess, not currentCellDescriptors!!.processedSerializedVarsToJavaProperties[serializedVersion] = processedData.propertiesData currentCellDescriptors.processedSerializedVarsToKTProperties[serializedVersion] = processedData.kPropertiesData if (value.objectInstance != null) { - seenObjectsPerCell[cellId]!!.putIfAbsent(value, serializedVersion) + seenObjectsPerVariable[topLevelName]!!.putIfAbsent(value, serializedVersion) } if (serializedVersion.isContainer) { // check for seen - if (seenObjectsPerCell[cellId]!!.containsKey(value)) { - val previouslySerializedState = seenObjectsPerCell[cellId]!![value] ?: return processedData.serializedVariablesState + if (seenObjectsPerVariable[topLevelName]!!.containsKey(value)) { + val previouslySerializedState = seenObjectsPerVariable[topLevelName]!![value] ?: return processedData.serializedVariablesState serializedVersion.fieldDescriptor += previouslySerializedState.fieldDescriptor + if (checkIsNotStandardDescriptor(serializedVersion.fieldDescriptor)) { + return serializedVersion + } } val type = processedData.propertiesType if (type == PropertiesType.KOTLIN) { @@ -535,8 +546,8 @@ class VariablesSerializer( val serializedIteration = mutableMapOf() - seenObjectsPerCell.putIfAbsent(cellId, mutableMapOf()) - val seenObjectsPerCell = seenObjectsPerCell[cellId] + seenObjectsPerVariable.putIfAbsent(topLevelName, mutableMapOf()) + val seenObjectsPerCell = seenObjectsPerVariable[topLevelName] val currentCellDescriptors = computedDescriptorsPerCell[cellId]!![topLevelName]!! // ok, it's a copy on the left for some reason val instancesPerState = currentCellDescriptors.instancesPerState @@ -645,7 +656,7 @@ class VariablesSerializer( getSimpleTypeNameFrom(elem, value.objectInstance) ?: "null" } serializedIteration[name] = if (standardContainersUtilizer.isStandardType(simpleType)) { - // todo might add isRecursive + // TODO might add isRecursive standardContainersUtilizer.serializeContainer(simpleType, value.objectInstance, true) } else { createSerializeVariableState(name, simpleType, value) diff --git a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt index 85af1230a..5330d6437 100644 --- a/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt +++ b/src/main/kotlin/org/jetbrains/kotlinx/jupyter/util.kt @@ -81,7 +81,7 @@ class VariablesUsagesPerCellWatcher { /** * Tells in which cell a variable was declared */ - private val variablesDeclarationInfo: MutableMap = mutableMapOf() + val variablesDeclarationInfo: MutableMap = mutableMapOf() private val unchangedVariables: MutableSet = mutableSetOf() 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 index 46d42623c..32f782ece 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt @@ -666,7 +666,7 @@ class ReplVarsTest : AbstractSingleReplTest() { """.trimIndent(), jupyterId = 1 ) - val state = repl.notebook.cellVariables + var state = repl.notebook.cellVariables assertTrue(state.isNotEmpty()) // f is not accessible from here @@ -677,10 +677,10 @@ class ReplVarsTest : AbstractSingleReplTest() { """.trimIndent(), jupyterId = 2 ) + state = repl.notebook.cellVariables assertTrue(state.isNotEmpty()) - - // TODO discuss if we really want "z", "f", "x" - val setOfCell = setOf("z") + // ignore primitive references precise check for Java > 8 + val setOfCell = setOf("z", "x") assertTrue(state.containsValue(setOfCell)) } @@ -808,7 +808,7 @@ class ReplVarsTest : AbstractSingleReplTest() { jupyterId = 2 ).metadata.evaluatedVariablesState val innerList = res["l"]!!.fieldDescriptor["elementData"]!!.fieldDescriptor["data"] - val newData = serializer.doIncrementalSerialization(0, "l","data", innerList!!) + val newData = serializer.doIncrementalSerialization(0, "l", "data", innerList!!) assertTrue(newData.isContainer) assertTrue(newData.fieldDescriptor.size > 4) } @@ -860,17 +860,6 @@ class ReplVarsTest : AbstractSingleReplTest() { var state = repl.notebook.unchangedVariables() assertEquals(3, state.size) - eval( - """ - private val x = "abcd" - internal val z = 47 - """.trimIndent(), - jupyterId = 2 - ) - state = repl.notebook.unchangedVariables() - assertEquals(1, state.size) - assertTrue(state.contains("f")) - eval( """ private val x = "abcd" @@ -889,7 +878,7 @@ class ReplVarsTest : AbstractSingleReplTest() { jupyterId = 3 ) state = repl.notebook.unchangedVariables() - assertEquals(2, state.size) + assertEquals(1, state.size) } } @@ -924,7 +913,7 @@ class ReplVarsSerializationTest : AbstractSingleReplTest() { assertEquals(listOf(1, 2, 3, 4).toString().substring(1, actualContainer.value!!.length + 1), actualContainer.value) val serializer = repl.variablesSerializer - val newData = serializer.doIncrementalSerialization(0, "x","data", actualContainer) + val newData = serializer.doIncrementalSerialization(0, "x", "data", actualContainer) } @Test @@ -1382,8 +1371,7 @@ class ReplVarsSerializationTest : AbstractSingleReplTest() { """ val x = listOf(1, 2, 3, 4) """.trimIndent(), - jupyterId = 2 + jupyterId = 2 ).metadata.evaluatedVariablesState - val a = 1 } } diff --git a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/TrackedCellExecutor.kt b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/TrackedCellExecutor.kt index 0fc63cc33..2350c3f2a 100644 --- a/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/TrackedCellExecutor.kt +++ b/src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/TrackedCellExecutor.kt @@ -68,6 +68,10 @@ internal class MockedInternalEvaluator : TrackedInternalEvaluator { return -1 } + override fun getVariablesDeclarationInfo(): Map { + return variablesWatcher.variablesDeclarationInfo + } + override fun getUnchangedVariables(): Set { return variablesWatcher.getUnchangedVariables() }