Decrease Light classes code generation in multithreaded cases by guarding KotlinClassInnerStuffCache, LightClassDataHolder.ForClass cache values calculation with lock

compiler/light-classes/src/org/jetbrains/kotlin/asJava/builder/LightClassDataProvider.kt

@@ -56,13 +56,10 @@ class LightClassDataProviderForClassOrObject(
     }
 
     override fun compute(): CachedValueProvider.Result<LightClassDataHolder.ForClass>? {
+        val trackerService = KotlinModificationTrackerService.getInstance(classOrObject.project)
         return CachedValueProvider.Result.create(
-                computeLightClassData(),
+            computeLightClassData(),
-                if (classOrObject.isLocal()) {
+            if (classOrObject.isLocal) trackerService.modificationTracker else trackerService.outOfBlockModificationTracker
-                    KotlinModificationTrackerService.getInstance(classOrObject.project).modificationTracker
-                } else {
-                    KotlinModificationTrackerService.getInstance(classOrObject.project).outOfBlockModificationTracker
-                }
         )
     }
 

compiler/light-classes/src/org/jetbrains/kotlin/asJava/classes/KotlinClassInnerStuffCache.kt

@@ -9,6 +9,7 @@ import com.intellij.openapi.diagnostic.Logger
 import com.intellij.openapi.util.SimpleModificationTracker
 import com.intellij.psi.*
 import com.intellij.psi.augment.PsiAugmentProvider
+import com.intellij.psi.impl.PsiCachedValueImpl
 import com.intellij.psi.impl.PsiClassImplUtil
 import com.intellij.psi.impl.PsiImplUtil
 import com.intellij.psi.impl.light.LightMethod
@@ -17,7 +18,6 @@ import com.intellij.psi.scope.ElementClassHint
 import com.intellij.psi.scope.NameHint
 import com.intellij.psi.scope.PsiScopeProcessor
 import com.intellij.psi.util.CachedValueProvider
-import com.intellij.psi.util.CachedValuesManager
 import com.intellij.util.ArrayUtil
 import com.intellij.util.containers.ContainerUtil
 import gnu.trove.THashMap
@@ -26,38 +26,72 @@ class KotlinClassInnerStuffCache(val myClass: PsiExtensibleClass, externalDepend
     private val myTracker = SimpleModificationTracker()
     private val dependencies: List<Any> = externalDependencies + myTracker
 
-    // This method is modified
+    fun <T : Any> get(initializer: () -> T) = object : Lazy<T> {
-    private fun <T> makeProvider(value: () -> T): CachedValueProvider.Result<T> {
+        // Note: holder is used as initialization monitor as well
-        return CachedValueProvider.Result.create(value(), dependencies)
+        private val holder = lazyPub {
+            PsiCachedValueImpl(PsiManager.getInstance(myClass.project),
+                               CachedValueProvider<T> {
+                                   val v = initializer()
+                                   CachedValueProvider.Result.create(v, dependencies)
+                               })
+        }
+
+        private fun computeValue(): T = holder.value.value ?: error("holder has not null in initializer")
+
+        override val value: T
+            get() {
+                return if (holder.value.hasUpToDateValue()) {
+                    computeValue()
+                } else {
+                    // the idea behind this locking approach:
+                    // Thread T1 starts to calculate value for A it acquires lock for A
+                    //
+                    // Assumption 1: Lets say A calculation requires another value e.g. B to be calculated
+                    // Assumption 2: Thread T2 wants to calculate value for B
+
+                    // to avoid dead-lock case we mark thread as doing calculation and acquire lock only once per thread
+                    // as a trade-off to prevent dependent value could be calculated several time
+                    // due to CAS (within putUserDataIfAbsent etc) the same instance of calculated value will be used
+
+                    if (!initIsRunning.get()) {
+                        synchronized(holder) {
+                            initIsRunning.set(true)
+                            try {
+                                computeValue()
+                            } finally {
+                                initIsRunning.set(false)
+                            }
+                        }
+                    } else {
+                        computeValue()
+                    }
+                }
+            }
+
+        override fun isInitialized() = holder.isInitialized()
     }
 
-    private fun <T : Any> get(myClass: PsiExtensibleClass, provider: CachedValueProvider<T>) = lazyPub {
+    private val _getConstructors: Array<PsiMethod> by get { PsiImplUtil.getConstructors(myClass) }
-        CachedValuesManager.getCachedValue(myClass, provider)
-    }
-
-    private val _getConstructors: Array<PsiMethod> by get(
-        myClass,
-        CachedValueProvider { makeProvider { PsiImplUtil.getConstructors(myClass) } })
 
     val constructors: Array<PsiMethod>
         get() = _getConstructors
 
-    private val _getFields: Array<PsiField> by get(myClass, CachedValueProvider { makeProvider { this.getAllFields() } })
+    private val _getFields: Array<PsiField> by get { this.getAllFields() }
 
     val fields: Array<PsiField>
         get() = _getFields
 
-    private val _getMethods: Array<PsiMethod> by get(myClass, CachedValueProvider { makeProvider { this.getAllMethods() } })
+    private val _getMethods: Array<PsiMethod> by get { this.getAllMethods() }
 
     val methods: Array<PsiMethod>
         get() = _getMethods
 
-    private val _getAllInnerClasses: Array<PsiClass> by get(myClass, CachedValueProvider { makeProvider { this.getAllInnerClasses() } })
+    private val _getAllInnerClasses: Array<PsiClass> by get { this.getAllInnerClasses() }
 
     val innerClasses: Array<out PsiClass>
         get() = _getAllInnerClasses
 
-    private val _getFieldsMap: Map<String, PsiField> by get(myClass, CachedValueProvider { makeProvider { this.getFieldsMap() } })
+    private val _getFieldsMap: Map<String, PsiField> by get { this.getFieldsMap() }
 
     fun findFieldByName(name: String, checkBases: Boolean): PsiField? {
         return if (checkBases) {
@@ -67,7 +101,7 @@ class KotlinClassInnerStuffCache(val myClass: PsiExtensibleClass, externalDepend
         }
     }
 
-    private val _getMethodsMap: Map<String, Array<PsiMethod>> by get(myClass, CachedValueProvider { makeProvider { this.getMethodsMap() } })
+    private val _getMethodsMap: Map<String, Array<PsiMethod>> by get { this.getMethodsMap() }
 
     fun findMethodsByName(name: String, checkBases: Boolean): Array<PsiMethod> {
         return if (checkBases) {
@@ -77,8 +111,7 @@ class KotlinClassInnerStuffCache(val myClass: PsiExtensibleClass, externalDepend
         }
     }
 
-    private val _getInnerClassesMap: Map<String, PsiClass> by get(myClass,
+    private val _getInnerClassesMap: Map<String, PsiClass> by get { this.getInnerClassesMap() }
-                                                                  CachedValueProvider { makeProvider { this.getInnerClassesMap() } })
 
     fun findInnerClassByName(name: String, checkBases: Boolean): PsiClass? {
         return if (checkBases) {
@@ -88,11 +121,11 @@ class KotlinClassInnerStuffCache(val myClass: PsiExtensibleClass, externalDepend
         }
     }
 
-    private val _makeValuesMethod: PsiMethod by get(myClass, CachedValueProvider { makeProvider { this.makeValuesMethod() } })
+    private val _makeValuesMethod: PsiMethod by get { this.makeValuesMethod() }
 
     fun getValuesMethod(): PsiMethod? = if (myClass.isEnum && myClass.name != null) _makeValuesMethod else null
 
-    private val _makeValueOfMethod: PsiMethod by get(myClass, CachedValueProvider { makeProvider { this.makeValueOfMethod() } })
+    private val _makeValueOfMethod: PsiMethod by get { this.makeValueOfMethod() }
 
     fun getValueOfMethod(): PsiMethod? = if (myClass.isEnum && myClass.name != null) _makeValueOfMethod else null
 
@@ -195,6 +228,9 @@ class KotlinClassInnerStuffCache(val myClass: PsiExtensibleClass, externalDepend
         private const val VALUES_METHOD = "values"
         private const val VALUE_OF_METHOD = "valueOf"
 
+        @JvmStatic
+        private val initIsRunning: ThreadLocal<Boolean> = ThreadLocal.withInitial { false }
+
         // Copy of PsiClassImplUtil.processDeclarationsInEnum for own cache class
         @JvmStatic
         fun processDeclarationsInEnum(

compiler/light-classes/src/org/jetbrains/kotlin/asJava/classes/KtLightClassForSourceDeclaration.kt

@@ -105,7 +105,8 @@ abstract class KtLightClassForSourceDeclaration(
     override val lightClassData: LightClassData
         get() = findLightClassData()
 
-    protected open fun findLightClassData() = getLightClassDataHolder().findDataForClassOrObject(classOrObject)
+    protected open fun findLightClassData() = getLightClassDataHolder().
+        findDataForClassOrObject(classOrObject)
 
     private fun getJavaFileStub(): PsiJavaFileStub = getLightClassDataHolder().javaFileStub
 
@@ -312,6 +313,10 @@ abstract class KtLightClassForSourceDeclaration(
 
     companion object {
         private val JAVA_API_STUB = Key.create<CachedValue<LightClassDataHolder.ForClass>>("JAVA_API_STUB")
+        private val JAVA_API_STUB_LOCK = Key.create<Any>("JAVA_API_STUB_LOCK")
+
+        @JvmStatic
+        private val javaApiStubInitIsRunning: ThreadLocal<Boolean> = ThreadLocal.withInitial { false }
 
         private val jetTokenToPsiModifier = listOf(
             PUBLIC_KEYWORD to PsiModifier.PUBLIC,
@@ -371,14 +376,47 @@ abstract class KtLightClassForSourceDeclaration(
         }
 
         private fun getLightClassCachedValue(classOrObject: KtClassOrObject): CachedValue<LightClassDataHolder.ForClass> {
-            var value =
+            val outerClassValue = getOutermostClassOrObject(classOrObject).getUserData(JAVA_API_STUB)
-                getOutermostClassOrObject(classOrObject).getUserData(JAVA_API_STUB) // stub computed for outer class can be used for inner/nested
+            outerClassValue?.let {
-                    ?: classOrObject.getUserData(JAVA_API_STUB)
+                // stub computed for outer class can be used for inner/nested
-            if (value == null) {
+                return it
-                value = CachedValuesManager.getManager(classOrObject.project).createCachedValue(
+            }
+            // the idea behind this locking approach:
+            // Thread T1 starts to calculate value for A it acquires lock for A
+            //
+            // Assumption 1: Lets say A calculation requires another value e.g. B to be calculated
+            // Assumption 2: Thread T2 wants to calculate value for B
+
+            // to avoid dead-lock case we mark thread as doing calculation and acquire lock only once per thread
+            // as a trade-off to prevent dependent value could be calculated several time
+            // due to CAS (within putUserDataIfAbsent etc) the same instance of calculated value will be used
+            val value: CachedValue<LightClassDataHolder.ForClass> = if (!javaApiStubInitIsRunning.get()) {
+                classOrObject.getUserData(JAVA_API_STUB) ?: run {
+                    val lock = classOrObject.putUserDataIfAbsent(JAVA_API_STUB_LOCK, Object())
+                    synchronized(lock) {
+                        try {
+                            javaApiStubInitIsRunning.set(true)
+                            computeLightClassCachedValue(classOrObject)
+                        } finally {
+                            javaApiStubInitIsRunning.set(false)
+                        }
+                    }
+                }
+            } else {
+                computeLightClassCachedValue(classOrObject)
+            }
+            return value
+        }
+
+        private fun computeLightClassCachedValue(
+            classOrObject: KtClassOrObject
+        ): CachedValue<LightClassDataHolder.ForClass> {
+            val value = classOrObject.getUserData(JAVA_API_STUB) ?: run {
+                val manager = CachedValuesManager.getManager(classOrObject.project)
+                val cachedValue = manager.createCachedValue(
                     LightClassDataProviderForClassOrObject(classOrObject), false
                 )
-                value = classOrObject.putUserDataIfAbsent(JAVA_API_STUB, value)
+                classOrObject.putUserDataIfAbsent(JAVA_API_STUB, cachedValue)
             }
             return value
         }

idea/idea-analysis/src/org/jetbrains/kotlin/idea/caches/lightClasses/LazyLightClassDataHolder.kt

@@ -43,7 +43,9 @@ sealed class LazyLightClassDataHolder(
             cache.computeIfAbsent(lazyLightClassDataHolder, diagnostics)
     }
 
-    private val exactResultLazyValue = lazyPub { builder(exactContextProvider()).stub }
+    private val _builderExactContextProvider: LightClassBuilderResult by lazyPub { builder(exactContextProvider()) }
+
+    private val exactResultLazyValue = lazyPub { _builderExactContextProvider.stub }
 
     private val lazyInexactStub by lazyPub {
         dummyContextProvider?.let { provider -> provider()?.let { context -> builder.invoke(context).stub } }
@@ -56,10 +58,9 @@ sealed class LazyLightClassDataHolder(
 
     override val extraDiagnostics: Diagnostics
         get() = diagnosticsHolderProvider().getOrCompute(this) {
-            builder(exactContextProvider()).diagnostics
+            // Force lazy diagnostics computation because otherwise a lot of memory is retained by computation.
-                // Force lazy diagnostics computation because otherwise a lot of memory is retained by computation.
+            // NB: Laziness here is not crucial anyway since somebody already has requested diagnostics and we hope one will use them
-                // NB: Laziness here is not crucial anyway since somebody already has requested diagnostics and we hope one will use them
+            _builderExactContextProvider.diagnostics.takeUnless { it.isEmpty() } ?: Diagnostics.EMPTY
-                .takeUnless { it.isEmpty() } ?: Diagnostics.EMPTY
         }
 
     // for facade or defaultImpls