Skip to content

Commit 9e21a26

Browse files
Consider changes in objects via immutable refs
1 parent 3cab715 commit 9e21a26

File tree

4 files changed

+83
-10
lines changed

4 files changed

+83
-10
lines changed

jupyter-lib/shared-compiler/src/main/kotlin/org/jetbrains/kotlinx/jupyter/compiler/util/serializedCompiledScript.kt

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,29 @@ data class SerializedVariablesState(
2525
val type: String = "",
2626
val value: String? = null,
2727
val isContainer: Boolean = false,
28-
val ID: String = ""
28+
val stateId: String = ""
2929
) {
3030
// todo: not null
3131
val fieldDescriptor: MutableMap<String, SerializedVariablesState?> = mutableMapOf()
32+
override fun equals(other: Any?): Boolean {
33+
if (this === other) return true
34+
if (javaClass != other?.javaClass) return false
35+
36+
other as SerializedVariablesState
37+
38+
if (type != other.type) return false
39+
if (value != other.value) return false
40+
if (isContainer != other.isContainer) return false
41+
42+
return true
43+
}
44+
45+
override fun hashCode(): Int {
46+
var result = type.hashCode()
47+
result = 31 * result + (value?.hashCode() ?: 0)
48+
result = 31 * result + isContainer.hashCode()
49+
return result
50+
}
3251
}
3352

3453
@Serializable

src/main/kotlin/org/jetbrains/kotlinx/jupyter/repl.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -457,7 +457,8 @@ class ReplForJupyterImpl(
457457
notebook.updateVariablesState(internalEvaluator)
458458
// printVars()
459459
// printUsagesInfo(jupyterId, cellVariables[jupyterId - 1])
460-
val serializedData = variablesSerializer.serializeVariables(jupyterId - 1, notebook.variablesState, notebook.unchangedVariables())
460+
val variablesCells: Map<String, Int> = notebook.variablesState.mapValues { internalEvaluator.findVariableCell(it.key) }
461+
val serializedData = variablesSerializer.serializeVariables(jupyterId - 1, notebook.variablesState, variablesCells, notebook.unchangedVariables())
461462

462463
GlobalScope.launch(Dispatchers.Default) {
463464
variablesSerializer.tryValidateCache(jupyterId - 1, notebook.cellVariables)

src/main/kotlin/org/jetbrains/kotlinx/jupyter/serializationUtils.kt

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ class VariablesSerializer(
358358
clearOldData(currentCellId, cellVariables)
359359
}
360360

361-
fun serializeVariables(cellId: Int, variablesState: Map<String, VariableState>, unchangedVariables: Set<String>): Map<String, SerializedVariablesState> {
361+
fun serializeVariables(cellId: Int, variablesState: Map<String, VariableState>, variablesCells: Map<String, Int>, unchangedVariables: Set<String>): Map<String, SerializedVariablesState> {
362362
fun removeNonExistingEntries() {
363363
val toRemoveSet = mutableSetOf<String>()
364364
serializedVariablesCache.forEach { (name, _) ->
@@ -382,17 +382,22 @@ class VariablesSerializer(
382382
val wasRedeclared = !unchangedVariables.contains(it)
383383
if (wasRedeclared) {
384384
removedFromSightVariables.remove(it)
385-
}
386-
(unchangedVariables.contains(it) || serializedVariablesCache[it]?.value != variablesState[it]?.stringValue) &&
385+
} /*
386+
(unchangedVariables.contains(it) || serializedVariablesCache[it]?.value != variablesState[it]?.value?.getOrNull().toString()) &&
387+
!removedFromSightVariables.contains(it)*/
388+
(unchangedVariables.contains(it)) &&
387389
!removedFromSightVariables.contains(it)
388390
}
389391
log.debug("Variables state as is: $variablesState")
390392
log.debug("Serializing variables after filter: $neededEntries")
391-
log.debug("Unchanged variables: $unchangedVariables")
393+
log.debug("Unchanged variables: ${unchangedVariables - neededEntries.keys}")
392394

393395
// remove previous data
394396
computedDescriptorsPerCell[cellId]?.instancesPerState?.clear()
395-
val serializedData = neededEntries.mapValues { serializeVariableState(cellId, it.key, it.value) }
397+
val serializedData = neededEntries.mapValues {
398+
val actualCell = variablesCells[it.key] ?: cellId
399+
serializeVariableState(actualCell, it.key, it.value)
400+
}
396401

397402
serializedVariablesCache.putAll(serializedData)
398403
removeNonExistingEntries()
@@ -730,7 +735,7 @@ class VariablesSerializer(
730735
""
731736
}
732737

733-
val serializedVariablesState = SerializedVariablesState(type, getProperString(value), isContainer, ID = finalID)
738+
val serializedVariablesState = SerializedVariablesState(type, getProperString(value), isContainer, finalID)
734739

735740
return ProcessedSerializedVarsState(serializedVariablesState, membersProperties?.toTypedArray())
736741
}

src/test/kotlin/org/jetbrains/kotlinx/jupyter/test/repl/ReplTests.kt

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -734,7 +734,7 @@ class ReplVarsTest : AbstractSingleReplTest() {
734734

735735
@Test
736736
fun testRecursiveVarsState() {
737-
eval(
737+
val res = eval(
738738
"""
739739
val l = mutableListOf<Any>()
740740
l.add(listOf(l))
@@ -744,7 +744,7 @@ class ReplVarsTest : AbstractSingleReplTest() {
744744
val z = setOf(1, 2, 4)
745745
""".trimIndent(),
746746
jupyterId = 1
747-
)
747+
).metadata
748748
val state = repl.notebook.variablesState
749749
assertTrue(state.contains("l"))
750750
assertTrue(state.contains("m"))
@@ -753,6 +753,54 @@ class ReplVarsTest : AbstractSingleReplTest() {
753753
assertEquals("ArrayList: recursive structure", state["l"]!!.stringValue)
754754
assertTrue(state["m"]!!.stringValue!!.contains(" recursive structure"))
755755
assertEquals("[1, 2, 4]", state["z"]!!.stringValue)
756+
757+
val serializer = repl.variablesSerializer
758+
val descriptor = res.evaluatedVariablesState["l"]!!.fieldDescriptor
759+
val innerList = descriptor["elementData"]!!.fieldDescriptor["data"]
760+
val newData = serializer.doIncrementalSerialization(0, "data", innerList!!)
761+
assertEquals(2, newData.fieldDescriptor.size)
762+
}
763+
764+
@Test
765+
fun testUnchangedVars() {
766+
eval(
767+
"""
768+
var l = 11111
769+
val m = "abc"
770+
""".trimIndent(),
771+
jupyterId = 1
772+
)
773+
var state = repl.notebook.unchangedVariables()
774+
val res = eval(
775+
"""
776+
l += 11111
777+
""".trimIndent(),
778+
jupyterId = 2
779+
).metadata.evaluatedVariablesState
780+
state = repl.notebook.unchangedVariables()
781+
assertEquals(1, state.size)
782+
assertTrue(state.contains("m"))
783+
}
784+
785+
@Test
786+
fun testMutableList() {
787+
eval(
788+
"""
789+
val l = mutableListOf(1, 2, 3, 4)
790+
""".trimIndent(),
791+
jupyterId = 1
792+
)
793+
val serializer = repl.variablesSerializer
794+
val res = eval(
795+
"""
796+
l.add(5)
797+
""".trimIndent(),
798+
jupyterId = 2
799+
).metadata.evaluatedVariablesState
800+
val innerList = res["l"]!!.fieldDescriptor["elementData"]!!.fieldDescriptor["data"]
801+
val newData = serializer.doIncrementalSerialization(0, "data", innerList!!)
802+
assertTrue(newData.isContainer)
803+
assertTrue(newData.fieldDescriptor.size > 4)
756804
}
757805

758806
@Test

0 commit comments

Comments
 (0)