Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Serializable Vars State with nested fields descriptors #314

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
eb38537
Add incremental serialization support prototype; support cyclic refer…
nikolay-egorov Jul 20, 2021
e72e266
Add variable serializer to repl state; fixed some tests
nikolay-egorov Jul 22, 2021
f3bbb28
Consider anonymous classes; still need a bit of to do
nikolay-egorov Jul 23, 2021
0131e41
Make RunTimeWrapper
nikolay-egorov Jul 29, 2021
79bebea
Add new message type for vars serialization
nikolay-egorov Jul 29, 2021
e9aefaa
Change main reflection to jvm Fields; use KReflection for built-ins
nikolay-egorov Jul 30, 2021
e5fac02
Add possibility to use serialization request with only top-level desc…
nikolay-egorov Aug 4, 2021
f3a3ce1
Get unchangedVariables from notebook to reflect after execution state…
nikolay-egorov Aug 6, 2021
dc305f7
Add support for set container; make standard Arrays have values descr…
nikolay-egorov Aug 6, 2021
18e844c
Add suspended cache validation; Add possibility to remove old variabl…
nikolay-egorov Aug 9, 2021
42751f2
Prevent complete cyclic serialization on server side
nikolay-egorov Aug 12, 2021
445583e
Fix top level recursive serialization
nikolay-egorov Aug 13, 2021
a00fcec
Add ID for serialized objects
nikolay-egorov Aug 17, 2021
0355749
Consider changes in objects via immutable refs
nikolay-egorov Aug 17, 2021
b4ac2ce
Use custom lazy delegate
nikolay-egorov Aug 18, 2021
7ac866a
Store all meta-data related to a top-level variable name
nikolay-egorov Aug 20, 2021
d6f6d39
Delete meta-data of redeclared variables
nikolay-egorov Aug 20, 2021
4ddb8ea
Improve skeleton lists handling
nikolay-egorov Aug 24, 2021
f9a67d5
Remove serialization cache
nikolay-egorov Aug 24, 2021
ec951f7
Fix recursion detection
nikolay-egorov Aug 24, 2021
5aebd3d
Make serialization contain comm_id to respect jupyter comm handlers
nikolay-egorov Aug 26, 2021
c1b2344
Add special type classes in shared-compiler
nikolay-egorov Sep 9, 2021
2a1081a
Sync with master, do some refactor
nikolay-egorov Nov 15, 2021
ef7df1e
Tmp fix one strangely falling test
nikolay-egorov Dec 6, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,8 @@ tasks {

"junit.jupiter.execution.parallel.enabled" to doParallelTesting.toString() as Any,
"junit.jupiter.execution.parallel.mode.default" to "concurrent",
"junit.jupiter.execution.parallel.mode.classes.default" to "concurrent"
"junit.jupiter.execution.parallel.mode.classes.default" to "concurrent",
"jupyter.serialization.enabled" to "true"
)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,33 +1,49 @@
package org.jetbrains.kotlinx.jupyter.api

import java.lang.reflect.Field
import kotlin.reflect.KProperty
import kotlin.reflect.KProperty1
import kotlin.reflect.jvm.isAccessible

interface VariableState {
val property: KProperty<*>
val property: Field
val scriptInstance: Any?
val stringValue: String?
val value: Result<Any?>
val isRecursive: Boolean
}

class DependentLazyDelegate<T>(val initializer: () -> T?) {
private var cachedPropertyValue: T? = null
var isChanged: Boolean = true

operator fun getValue(thisRef: Any?, property: KProperty<*>): T? {
if (isChanged) {
cachedPropertyValue = initializer()
}
return cachedPropertyValue
}
}

data class VariableStateImpl(
override val property: KProperty1<Any, *>,
override val property: Field,
override val scriptInstance: Any,
) : VariableState {
private val stringCache = VariableStateCache<String?> {
value.getOrNull()?.let { value ->
try {
value.toString()
} catch (e: Throwable) {
if (e is StackOverflowError) {
isRecursive = true
}
"${value::class.simpleName}: [exception thrown: $e]"
}
}
}
override var isRecursive: Boolean = false

private val valCache = VariableStateCache<Result<Any?>> (
{
oldValue, newValue ->
private val valCache = VariableStateCache<Result<Any?>>(
{ oldValue, newValue ->
oldValue.getOrNull() !== newValue.getOrNull()
},
{
Expand All @@ -47,49 +63,50 @@ data class VariableStateImpl(
}
}

override val stringValue: String? get() = stringCache.get()
override val stringValue: String? get() = stringCache.getOrNull()

override val value: Result<Any?> get() = valCache.get()

companion object {
private fun <T : KProperty<*>, R> T.asAccessible(action: (T) -> R): R {
@SuppressWarnings("DEPRECATED")
private fun <R> Field.asAccessible(action: (Field) -> R): R {
val wasAccessible = isAccessible
isAccessible = true
val res = action(this)
isAccessible = wasAccessible
return res
}
}
}

private class VariableStateCache<T>(
val equalityChecker: (T, T) -> Boolean = { x, y -> x == y },
val calculate: (T?) -> T
) {
private var cachedVal: T? = null
private var shouldRenew: Boolean = true
private class VariableStateCache<T>(
val equalityChecker: (T, T) -> Boolean = { x, y -> x == y },
val calculate: (T?) -> T
) {
private var cachedVal: T? = null
private var shouldRenew: Boolean = true

fun getOrNull(): T? {
return if (shouldRenew) {
calculate(cachedVal).also {
cachedVal = it
shouldRenew = false
fun getOrNull(): T? {
return if (shouldRenew) {
calculate(cachedVal).also {
cachedVal = it
shouldRenew = false
}
} else {
cachedVal
}
} else {
cachedVal
}
}

fun get(): T = getOrNull()!!
fun get(): T = getOrNull()!!

fun update() {
shouldRenew = true
}
fun update() {
shouldRenew = true
}

fun forceUpdate(): Boolean {
val oldVal = getOrNull()
update()
val newVal = get()
return oldVal != null && equalityChecker(oldVal, newVal)
fun forceUpdate(): Boolean {
val oldVal = getOrNull()
update()
val newVal = get()
return oldVal != null && equalityChecker(oldVal, newVal)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,88 @@ data class SerializedCompiledScriptsData(
}
}

@Serializable
data class SerializableTypeInfo(val type: Type = Type.Custom, val isPrimitive: Boolean = false, val fullType: String = "") {
companion object {
val ignoreSet = setOf("int", "double", "boolean", "char", "float", "byte", "string", "entry")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это поле не используется нигде. Если что-то используется только в плагине, надо написать тесты на то, как это там используется. Если не используется нигде, удалить


val propertyNamesForNullFilter = setOf("data", "size")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Аналогично (unused)


fun makeFromSerializedVariablesState(type: String?, isContainer: Boolean?): SerializableTypeInfo {
val fullType = type.orEmpty()
val enumType = fullType.toTypeEnum()
val isPrimitive = !(
if (fullType != "Entry") (isContainer ?: false)
else true
)

Comment on lines +33 to +37
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fullType != "Entry" isContainer Result
true true false
true false or null true
false true false
false false or null false

Это таблица для AND, значит всё это выражение можно упростить до isPrimitive = fullType!="Entry" && isContainer != true

return SerializableTypeInfo(enumType, isPrimitive, fullType)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Зачем нам тип передавать в двух видах? Честно говоря, выглядит стрёмно

}
}
}

@Serializable
enum class Type {
Map,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Капсом лучше писать

Entry,
Array,
List,
Custom
}

fun String.toTypeEnum(): Type {
return when (this) {
"Map" -> Type.Map
"Entry" -> Type.Entry
"Array" -> Type.Array
"List" -> Type.List
else -> Type.Custom
}
}

@Serializable
data class SerializedVariablesState(
val type: SerializableTypeInfo = SerializableTypeInfo(),
val value: String? = null,
val isContainer: Boolean = false,
val stateId: String = ""
) {
// todo: not null
val fieldDescriptor: MutableMap<String, SerializedVariablesState?> = mutableMapOf()
override fun equals(other: Any?): Boolean {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Зачем кастомные equals/hashcode? Ни один тест не упал после их удаления

if (this === other) return true
if (javaClass != other?.javaClass) return false

other as SerializedVariablesState

if (type != other.type) return false
if (value != other.value) return false
if (isContainer != other.isContainer) return false

return true
}

override fun hashCode(): Int {
var result = type.hashCode()
result = 31 * result + (value?.hashCode() ?: 0)
result = 31 * result + isContainer.hashCode()
return result
}
}

@Serializable
class SerializationReply(
val cell_id: Int = 1,
val descriptorsState: Map<String, SerializedVariablesState> = emptyMap(),
val comm_id: String = ""
)

@Serializable
class EvaluatedSnippetMetadata(
val newClasspath: Classpath = emptyList(),
val compiledData: SerializedCompiledScriptsData = SerializedCompiledScriptsData.EMPTY,
val newImports: List<String> = emptyList(),
val evaluatedVariablesState: Map<String, String?> = mutableMapOf()
val evaluatedVariablesState: Map<String, SerializedVariablesState> = emptyMap()
) {
companion object {
val EMPTY = EvaluatedSnippetMetadata()
Expand Down
8 changes: 8 additions & 0 deletions src/main/kotlin/org/jetbrains/kotlinx/jupyter/apiImpl.kt
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import org.jetbrains.kotlinx.jupyter.api.RenderersProcessor
import org.jetbrains.kotlinx.jupyter.api.ResultsAccessor
import org.jetbrains.kotlinx.jupyter.api.VariableState
import org.jetbrains.kotlinx.jupyter.api.libraries.LibraryResolutionRequest
import org.jetbrains.kotlinx.jupyter.repl.InternalEvaluator
import org.jetbrains.kotlinx.jupyter.repl.impl.SharedReplContext

class DisplayResultWrapper private constructor(
Expand Down Expand Up @@ -133,7 +134,9 @@ class NotebookImpl(

private val history = arrayListOf<CodeCellImpl>()
private var mainCellCreated = false
private val _unchangedVariables: MutableSet<String> = mutableSetOf()

val unchangedVariables: Set<String> get() = _unchangedVariables
val displays = DisplayContainerImpl()

override fun getAllDisplays(): List<DisplayResultWithCell> {
Expand All @@ -149,6 +152,11 @@ class NotebookImpl(
override val jreInfo: JREInfoProvider
get() = JavaRuntime

fun updateVariablesState(evaluator: InternalEvaluator) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like a very internal method. We most likely don't want to expose it to user

_unchangedVariables.clear()
_unchangedVariables.addAll(evaluator.getUnchangedVariables())
}

fun variablesReportAsHTML(): String {
return generateHTMLVarsReport(variablesState)
}
Expand Down
23 changes: 22 additions & 1 deletion src/main/kotlin/org/jetbrains/kotlinx/jupyter/message_types.kt
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import kotlinx.serialization.json.decodeFromJsonElement
import kotlinx.serialization.json.encodeToJsonElement
import kotlinx.serialization.json.jsonObject
import kotlinx.serialization.serializer
import org.jetbrains.kotlinx.jupyter.compiler.util.SerializedVariablesState
import org.jetbrains.kotlinx.jupyter.config.LanguageInfo
import org.jetbrains.kotlinx.jupyter.exceptions.ReplException
import kotlin.reflect.KClass
Expand Down Expand Up @@ -87,7 +88,11 @@ enum class MessageType(val contentClass: KClass<out MessageContent>) {
COMM_CLOSE(CommClose::class),

LIST_ERRORS_REQUEST(ListErrorsRequest::class),
LIST_ERRORS_REPLY(ListErrorsReply::class);
LIST_ERRORS_REPLY(ListErrorsReply::class),

// from Serialization_Request
VARIABLES_VIEW_REQUEST(SerializationRequest::class),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут видимо не допереименовано

VARIABLES_VIEW_REPLY(SerializationReply::class);

val type: String
get() = name.lowercase()
Expand Down Expand Up @@ -552,6 +557,22 @@ class ListErrorsReply(
val errors: List<ScriptDiagnostic>
) : MessageContent()

@Serializable
class SerializationRequest(
val cellId: Int,
val descriptorsState: Map<String, SerializedVariablesState>,
val topLevelDescriptorName: String = "",
val pathToDescriptor: List<String> = emptyList(),
val commId: String = ""
) : MessageContent()

@Serializable
class SerializationReply(
val cell_id: Int = 1,
val descriptorsState: Map<String, SerializedVariablesState> = emptyMap(),
val comm_id: String = ""
) : MessageContent()

@Serializable(MessageDataSerializer::class)
data class MessageData(
val header: MessageHeader? = null,
Expand Down
43 changes: 41 additions & 2 deletions src/main/kotlin/org/jetbrains/kotlinx/jupyter/protocol.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ package org.jetbrains.kotlinx.jupyter

import ch.qos.logback.classic.Level
import kotlinx.serialization.json.Json
import kotlinx.serialization.json.JsonElement
import kotlinx.serialization.json.JsonNull
import kotlinx.serialization.json.JsonObject
import kotlinx.serialization.json.encodeToJsonElement
import kotlinx.serialization.json.jsonObject
import org.jetbrains.annotations.TestOnly
import org.jetbrains.kotlinx.jupyter.LoggingManagement.disableLogging
import org.jetbrains.kotlinx.jupyter.LoggingManagement.mainLoggerLevel
Expand Down Expand Up @@ -82,7 +85,6 @@ class OkResponseWithMessage(
)
)
}

socket.send(
makeReplyMessage(
requestMsg,
Expand All @@ -92,7 +94,7 @@ class OkResponseWithMessage(
"engine" to Json.encodeToJsonElement(requestMsg.data.header?.session),
"status" to Json.encodeToJsonElement("ok"),
"started" to Json.encodeToJsonElement(startedTime),
"eval_metadata" to Json.encodeToJsonElement(metadata),
"eval_metadata" to Json.encodeToJsonElement(metadata.convertToNullIfEmpty()),
),
content = ExecuteReply(
MessageStatus.OK,
Expand Down Expand Up @@ -307,6 +309,27 @@ fun JupyterConnection.Socket.shellMessagesHandler(msg: Message, repl: ReplForJup
is CommInfoRequest -> {
sendWrapped(msg, makeReplyMessage(msg, MessageType.COMM_INFO_REPLY, content = CommInfoReply(mapOf())))
}
is CommOpen -> {
if (!content.targetName.equals("kotlin_serialization", ignoreCase = true)) {
send(makeReplyMessage(msg, MessageType.NONE))
return
}
log.debug("Message type in CommOpen: $msg, ${msg.type}")
val data = content.data ?: return sendWrapped(msg, makeReplyMessage(msg, MessageType.VARIABLES_VIEW_REPLY))
if (data.isEmpty()) return sendWrapped(msg, makeReplyMessage(msg, MessageType.VARIABLES_VIEW_REPLY))
log.debug("Message data: $data")
val messageContent = getVariablesDescriptorsFromJson(data)
connection.launchJob {
repl.serializeVariables(
messageContent.topLevelDescriptorName,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Выглядит так, что распаковка-запаковка делается без особого смысла. Можно передать просто messageContent

messageContent.descriptorsState,
content.commId,
messageContent.pathToDescriptor
) { result ->
sendWrapped(msg, makeReplyMessage(msg, MessageType.COMM_MSG, content = result))
}
}
}
is CompleteRequest -> {
connection.launchJob {
repl.complete(content.code, content.cursorPos) { result ->
Expand All @@ -321,6 +344,17 @@ fun JupyterConnection.Socket.shellMessagesHandler(msg: Message, repl: ReplForJup
}
}
}
is SerializationRequest -> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are SerializationRequest's are still used? I thought we switched to comm-s

connection.launchJob {
if (content.topLevelDescriptorName.isNotEmpty()) {
repl.serializeVariables(content.topLevelDescriptorName, content.descriptorsState, commID = content.commId, content.pathToDescriptor) { result ->
sendWrapped(msg, makeReplyMessage(msg, MessageType.VARIABLES_VIEW_REPLY, content = result))
}
} else {
sendWrapped(msg, makeReplyMessage(msg, MessageType.VARIABLES_VIEW_REPLY, content = null))
}
}
}
is IsCompleteRequest -> {
// We are in console mode, so switch off all the loggers
if (mainLoggerLevel() != Level.OFF) disableLogging()
Expand Down Expand Up @@ -513,3 +547,8 @@ fun JupyterConnection.evalWithIO(repl: ReplForJupyter, srcMessage: Message, body
KernelStreams.setStreams(false, out, err)
}
}

fun EvaluatedSnippetMetadata?.convertToNullIfEmpty(): JsonElement? {
val jsonNode = Json.encodeToJsonElement(this)
return if (jsonNode is JsonNull || jsonNode?.jsonObject.isEmpty()) null else jsonNode
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

return Json.encodeToJsonElement(this).takeIf { it !is JsonNull && it.jsonObject.isNotEmpty() }

}
Loading