From 766681c0e7aeb2ec93446a42e83d7619b8c3209d Mon Sep 17 00:00:00 2001
From: Dominic Fox <40790090+distributedleetravis@users.noreply.github.com>
Date: Mon, 26 Nov 2018 13:48:32 +0000
Subject: [PATCH] Avoid constructing a lookup/builder if the type is already
 cached (#4294)

---
 .../internal/model/LocalTypeInformation.kt    |  5 +-
 .../internal/model/LocalTypeModel.kt          | 51 ++++++++++++++-----
 2 files changed, 41 insertions(+), 15 deletions(-)

diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeInformation.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeInformation.kt
index c7473623d4..9c815fe84f 100644
--- a/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeInformation.kt
+++ b/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeInformation.kt
@@ -54,11 +54,12 @@ sealed class LocalTypeInformation {
          * types beginning the with provided [Type] and construct a complete set of [LocalTypeInformation] for that type.
          *
          * @param type The [Type] to obtain [LocalTypeInformation] for.
+         * @param typeIdentifier The [TypeIdentifier] for the [Type] to obtain [LocalTypeInformation] for.
          * @param lookup The [LocalTypeLookup] to use to find previously-constructed [LocalTypeInformation].
          */
-        fun forType(type: Type, lookup: LocalTypeLookup): LocalTypeInformation {
+        fun forType(type: Type, typeIdentifier: TypeIdentifier, lookup: LocalTypeLookup): LocalTypeInformation {
             val builder =  LocalTypeInformationBuilder(lookup)
-            val result = builder.build(type, TypeIdentifier.forGenericType(type))
+            val result = builder.build(type, typeIdentifier)
 
             // Patch every cyclic reference with a `follow` property pointing to the type information it refers to.
             builder.cycles.forEach { cycle ->
diff --git a/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeModel.kt b/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeModel.kt
index 9acdacb71e..45bdc8794f 100644
--- a/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeModel.kt
+++ b/serialization/src/main/kotlin/net/corda/serialization/internal/model/LocalTypeModel.kt
@@ -41,12 +41,6 @@ interface LocalTypeModel {
      * @param type The [Type] to get [LocalTypeInformation] for.
      */
     fun inspect(type: Type): LocalTypeInformation
-
-    /**
-     * Get [LocalTypeInformation] directly from the registry by [TypeIdentifier], returning null if no type information
-     * is registered for that identifier.
-     */
-    operator fun get(typeIdentifier: TypeIdentifier): LocalTypeInformation?
 }
 
 /**
@@ -55,20 +49,51 @@ interface LocalTypeModel {
  *
  * @param typeModelConfiguration Configuration controlling the behaviour of the [LocalTypeModel]'s type inspection.
  */
-class ConfigurableLocalTypeModel(private val typeModelConfiguration: LocalTypeModelConfiguration): LocalTypeModel, LocalTypeLookup {
+class ConfigurableLocalTypeModel(private val typeModelConfiguration: LocalTypeModelConfiguration): LocalTypeModel {
 
     private val typeInformationCache = DefaultCacheProvider.createCache<TypeIdentifier, LocalTypeInformation>()
 
-    override fun isExcluded(type: Type): Boolean = typeModelConfiguration.isExcluded(type)
+    /**
+     * We need to provide the [TypeInformationBuilder] with a temporary local cache, so that it doesn't leak
+     * [LocalTypeInformation] with unpatched cycles into the global cache where other threads can access them
+     * before we've patched the cycles up.
+     */
+    private class BuilderLookup(
+            private val globalCache: MutableMap<TypeIdentifier, LocalTypeInformation>,
+            private val typeModelConfiguration: LocalTypeModelConfiguration) : LocalTypeLookup {
 
-    override fun inspect(type: Type): LocalTypeInformation = LocalTypeInformation.forType(type, this)
+        private val localCache: MutableMap<TypeIdentifier, LocalTypeInformation> = mutableMapOf()
 
-    override fun findOrBuild(type: Type, typeIdentifier: TypeIdentifier, builder: (Boolean) -> LocalTypeInformation): LocalTypeInformation =
-            this[typeIdentifier] ?: builder(typeModelConfiguration.isOpaque(type)).apply {
-                typeInformationCache.putIfAbsent(typeIdentifier, this)
+        /**
+         * Read from the global cache (which contains only cycle-resolved type information), falling through
+         * to the local cache if the type isn't there yet.
+         */
+        override fun findOrBuild(type: Type, typeIdentifier: TypeIdentifier, builder: (Boolean) -> LocalTypeInformation): LocalTypeInformation =
+                globalCache[typeIdentifier] ?:
+                localCache.getOrPut(typeIdentifier) { builder(typeModelConfiguration.isOpaque(type)) }
+
+        override fun isExcluded(type: Type): Boolean = typeModelConfiguration.isExcluded(type)
+
+        /**
+         * Merge the local cache back into the global cache, once we've finished traversal (and patched all cycles).
+         */
+        fun merge() {
+            localCache.forEach { (identifier, information) ->
+                globalCache.putIfAbsent(identifier, information)
             }
+        }
+    }
 
-    override operator fun get(typeIdentifier: TypeIdentifier): LocalTypeInformation? = typeInformationCache[typeIdentifier]
+    override fun inspect(type: Type): LocalTypeInformation {
+        val typeIdentifier = TypeIdentifier.forGenericType(type)
+
+        return typeInformationCache.getOrPut(typeIdentifier) {
+            val lookup = BuilderLookup(typeInformationCache, typeModelConfiguration)
+            val result = LocalTypeInformation.forType(type, typeIdentifier, lookup)
+            lookup.merge()
+            result
+        }
+    }
 }
 
 /**