Skip to content

Commit 80ea789

Browse files
Consider changes in objects via immutable refs
1 parent f653554 commit 80ea789

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

+20-1
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

+2-1
Original file line numberDiff line numberDiff line change
@@ -430,7 +430,8 @@ class ReplForJupyterImpl(
430430
notebook.updateVariablesState(internalEvaluator)
431431
// printVars()
432432
// printUsagesInfo(jupyterId, cellVariables[jupyterId - 1])
433-
val serializedData = variablesSerializer.serializeVariables(jupyterId - 1, notebook.variablesState, notebook.unchangedVariables())
433+
val variablesCells: Map<String, Int> = notebook.variablesState.mapValues { internalEvaluator.findVariableCell(it.key) }
434+
val serializedData = variablesSerializer.serializeVariables(jupyterId - 1, notebook.variablesState, variablesCells, notebook.unchangedVariables())
434435

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

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

+11-6
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

+50-2
Original file line numberDiff line numberDiff line change
@@ -744,7 +744,7 @@ class ReplVarsTest : AbstractSingleReplTest() {
744744

745745
@Test
746746
fun testRecursiveVarsState() {
747-
eval(
747+
val res = eval(
748748
"""
749749
val l = mutableListOf<Any>()
750750
l.add(listOf(l))
@@ -754,7 +754,7 @@ class ReplVarsTest : AbstractSingleReplTest() {
754754
val z = setOf(1, 2, 4)
755755
""".trimIndent(),
756756
jupyterId = 1
757-
)
757+
).metadata
758758
val state = repl.notebook.variablesState
759759
assertTrue(state.contains("l"))
760760
assertTrue(state.contains("m"))
@@ -763,6 +763,54 @@ class ReplVarsTest : AbstractSingleReplTest() {
763763
assertEquals("ArrayList: recursive structure", state["l"]!!.stringValue)
764764
assertTrue(state["m"]!!.stringValue!!.contains(" recursive structure"))
765765
assertEquals("[1, 2, 4]", state["z"]!!.stringValue)
766+
767+
val serializer = repl.variablesSerializer
768+
val descriptor = res.evaluatedVariablesState["l"]!!.fieldDescriptor
769+
val innerList = descriptor["elementData"]!!.fieldDescriptor["data"]
770+
val newData = serializer.doIncrementalSerialization(0, "data", innerList!!)
771+
assertEquals(2, newData.fieldDescriptor.size)
772+
}
773+
774+
@Test
775+
fun testUnchangedVars() {
776+
eval(
777+
"""
778+
var l = 11111
779+
val m = "abc"
780+
""".trimIndent(),
781+
jupyterId = 1
782+
)
783+
var state = repl.notebook.unchangedVariables()
784+
val res = eval(
785+
"""
786+
l += 11111
787+
""".trimIndent(),
788+
jupyterId = 2
789+
).metadata.evaluatedVariablesState
790+
state = repl.notebook.unchangedVariables()
791+
assertEquals(1, state.size)
792+
assertTrue(state.contains("m"))
793+
}
794+
795+
@Test
796+
fun testMutableList() {
797+
eval(
798+
"""
799+
val l = mutableListOf(1, 2, 3, 4)
800+
""".trimIndent(),
801+
jupyterId = 1
802+
)
803+
val serializer = repl.variablesSerializer
804+
val res = eval(
805+
"""
806+
l.add(5)
807+
""".trimIndent(),
808+
jupyterId = 2
809+
).metadata.evaluatedVariablesState
810+
val innerList = res["l"]!!.fieldDescriptor["elementData"]!!.fieldDescriptor["data"]
811+
val newData = serializer.doIncrementalSerialization(0, "data", innerList!!)
812+
assertTrue(newData.isContainer)
813+
assertTrue(newData.fieldDescriptor.size > 4)
766814
}
767815

768816
@Test

0 commit comments

Comments
 (0)