thread safe modification tracker (BaseState, ModuleRootManagerImpl, LibraryTableBase)
authorVladimir Krivosheev <vladimir.krivosheev@jetbrains.com>
Thu, 28 Dec 2017 12:02:28 +0000 (13:02 +0100)
committerVladimir Krivosheev <vladimir.krivosheev@jetbrains.com>
Thu, 28 Dec 2017 13:41:47 +0000 (14:41 +0100)
platform/configuration-store-impl/testSrc/ApplicationStoreTest.kt
platform/projectModel-api/src/com/intellij/configurationStore/properties/CollectionStoredProperty.kt
platform/projectModel-api/src/com/intellij/configurationStore/properties/FloatStoredProperty.kt
platform/projectModel-api/src/com/intellij/configurationStore/properties/IntStoredProperty.kt
platform/projectModel-api/src/com/intellij/configurationStore/properties/LongStoredProperty.kt
platform/projectModel-api/src/com/intellij/configurationStore/properties/ObjectStoredProperty.kt
platform/projectModel-api/src/com/intellij/configurationStore/properties/StringStoredProperty.kt
platform/projectModel-api/src/com/intellij/openapi/components/BaseState.kt
platform/projectModel-impl/src/com/intellij/openapi/roots/impl/ModuleRootManagerImpl.java
platform/projectModel-impl/src/com/intellij/openapi/roots/impl/libraries/LibraryTableBase.java

index d73cda703fba9ac440048b467931da4eacc7cc01..cc529fc7efb75afe222701918d305e66aadb57e9 100644 (file)
@@ -43,6 +43,7 @@ import java.io.ByteArrayInputStream
 import java.io.InputStream
 import java.nio.file.Path
 import java.nio.file.Paths
+import java.util.concurrent.atomic.AtomicLong
 import kotlin.properties.Delegates
 
 internal class ApplicationStoreTest {
@@ -162,14 +163,12 @@ internal class ApplicationStoreTest {
 
     val componentPath = configDir.resolve("a.xml")
     assertThat(componentPath).isRegularFile
-    val componentFile = componentPath
 
     // additional export path
     val additionalPath = configDir.resolve("foo")
     additionalPath.writeChild("bar.icls", "")
-    val additionalFile = additionalPath
     val exportedData = BufferExposingByteArrayOutputStream()
-    exportSettings(setOf(componentFile, additionalFile), exportedData, configPath)
+    exportSettings(setOf(componentPath, additionalPath), exportedData, configPath)
 
     val relativePaths = getPaths(ByteArrayInputStream(exportedData.internalBuffer, 0, exportedData.size()))
     assertThat(relativePaths).containsOnly("a.xml", "foo/", "foo/bar.icls", "IntelliJ IDEA Global Settings")
@@ -180,7 +179,8 @@ internal class ApplicationStoreTest {
     val componentKey = A::class.java.name
     picoContainer.registerComponent(InstanceComponentAdapter(componentKey, component))
     try {
-      assertThat(getExportableComponentsMap(false, false, storageManager, relativePaths)).containsOnly(componentFile.to(listOf(ExportableItem(componentFile, ""))), additionalFile.to(listOf(ExportableItem(additionalFile, " (schemes)"))))
+      assertThat(getExportableComponentsMap(false, false, storageManager, relativePaths)).containsOnly(
+        componentPath.to(listOf(ExportableItem(componentPath, ""))), additionalPath.to(listOf(ExportableItem(additionalPath, " (schemes)"))))
     }
     finally {
       picoContainer.unregisterComponent(componentKey)
@@ -252,12 +252,12 @@ internal class ApplicationStoreTest {
     open class A : PersistentStateComponent<TestState>, SimpleModificationTracker() {
       var options = TestState()
 
-      var stateCalledCount = 0
+      val stateCalledCount = AtomicLong(0)
       var lastGetStateStackTrace: String? = null
 
       override fun getState(): TestState {
         lastGetStateStackTrace = ExceptionUtil.currentStackTrace()
-        stateCalledCount++
+        stateCalledCount.incrementAndGet()
         return options
       }
 
@@ -270,23 +270,23 @@ internal class ApplicationStoreTest {
     componentStore.initComponent(component, false)
 
     assertThat(component.modificationCount).isEqualTo(0)
-    assertThat(component.stateCalledCount).isEqualTo(0)
+    assertThat(component.stateCalledCount.get()).isEqualTo(0)
 
     // test that store correctly set last modification count to component modification count on init
     component.lastGetStateStackTrace = null
     saveStore()
     @Suppress("USELESS_CAST")
     assertThat(component.lastGetStateStackTrace as String?).isNull()
-    assertThat(component.stateCalledCount).isEqualTo(0)
+    assertThat(component.stateCalledCount.get()).isEqualTo(0)
 
     // change modification count - store will be forced to check changes using serialization and A.getState will be called
     component.incModificationCount()
     saveStore()
-    assertThat(component.stateCalledCount).isEqualTo(1)
+    assertThat(component.stateCalledCount.get()).isEqualTo(1)
 
     // test that store correctly save last modification time and doesn't call our state on next save
     saveStore()
-    assertThat(component.stateCalledCount).isEqualTo(1)
+    assertThat(component.stateCalledCount.get()).isEqualTo(1)
 
     val componentFile = testAppConfig.resolve("a.xml")
     assertThat(componentFile).doesNotExist()
@@ -299,7 +299,7 @@ internal class ApplicationStoreTest {
 
     component.incModificationCount()
     saveStore()
-    assertThat(component.stateCalledCount).isEqualTo(2)
+    assertThat(component.stateCalledCount.get()).isEqualTo(2)
 
     assertThat(componentFile).hasContent("""
     <application>
@@ -314,16 +314,16 @@ internal class ApplicationStoreTest {
 
     @State(name = "TestPersistentStateComponentWithModificationTracker", storages = arrayOf(Storage("b.xml")))
     open class A : PersistentStateComponentWithModificationTracker<TestState> {
-      var modificationCount: Long = 0
+      var modificationCount = AtomicLong(0)
 
-      override fun getStateModificationCount() = modificationCount
+      override fun getStateModificationCount() = modificationCount.get()
 
       var options = TestState()
 
-      var stateCalledCount = 0
+      var stateCalledCount = AtomicLong(0)
 
       override fun getState(): TestState {
-        stateCalledCount++
+        stateCalledCount.incrementAndGet()
         return options
       }
 
@@ -332,28 +332,28 @@ internal class ApplicationStoreTest {
       }
 
       fun incModificationCount() {
-        modificationCount++
+        modificationCount.incrementAndGet()
       }
     }
 
     val component = A()
     componentStore.initComponent(component, false)
 
-    assertThat(component.modificationCount).isEqualTo(0)
-    assertThat(component.stateCalledCount).isEqualTo(0)
+    assertThat(component.modificationCount.get()).isEqualTo(0)
+    assertThat(component.stateCalledCount.get()).isEqualTo(0)
 
     // test that store correctly set last modification count to component modification count on init
     saveStore()
-    assertThat(component.stateCalledCount).isEqualTo(0)
+    assertThat(component.stateCalledCount.get()).isEqualTo(0)
 
     // change modification count - store will be forced to check changes using serialization and A.getState will be called
     component.incModificationCount()
     saveStore()
-    assertThat(component.stateCalledCount).isEqualTo(1)
+    assertThat(component.stateCalledCount.get()).isEqualTo(1)
 
     // test that store correctly save last modification time and doesn't call our state on next save
     saveStore()
-    assertThat(component.stateCalledCount).isEqualTo(1)
+    assertThat(component.stateCalledCount.get()).isEqualTo(1)
 
     val componentFile = testAppConfig.resolve("b.xml")
     assertThat(componentFile).doesNotExist()
@@ -366,7 +366,7 @@ internal class ApplicationStoreTest {
 
     component.incModificationCount()
     saveStore()
-    assertThat(component.stateCalledCount).isEqualTo(2)
+    assertThat(component.stateCalledCount.get()).isEqualTo(2)
 
     assertThat(componentFile).hasContent("""
     <application>
index b3c9e6f7e8c965f66a52340311c76377773b7029..4d51d54cbbabc7a4bb397fe05111db83fcb3bc17 100644 (file)
@@ -19,7 +19,7 @@ internal open class CollectionStoredProperty<E, C : MutableCollection<E>>(protec
 
   override fun setValue(thisRef: BaseState, property: KProperty<*>, @Suppress("PARAMETER_NAME_CHANGED_ON_OVERRIDE") newValue: C) {
     if (doSetValue(value, newValue)) {
-      thisRef.ownModificationCount++
+      thisRef.intIncrementModificationCount()
     }
   }
 
@@ -56,7 +56,7 @@ internal class MapStoredProperty<K: Any, V>(private val value: MutableMap<K, V>)
 
   override fun setValue(thisRef: BaseState, property: KProperty<*>, @Suppress("PARAMETER_NAME_CHANGED_ON_OVERRIDE") newValue: MutableMap<K, V>) {
     if (doSetValue(value, newValue)) {
-      thisRef.ownModificationCount++
+      thisRef.intIncrementModificationCount()
     }
   }
 
index 28494bb4f8d964ca7f5d28386ef16a0c7e9bb58a..a170a2e83c2a8a7bc8f001030743ce80db73c669 100644 (file)
@@ -16,7 +16,7 @@ internal class FloatStoredProperty(private val defaultValue: Float, private val
   override fun setValue(thisRef: BaseState, property: KProperty<*>, value: Float) {
     val newValue = valueNormalizer?.invoke(value) ?: value
     if (this.value != newValue) {
-      thisRef.ownModificationCount++
+      thisRef.intIncrementModificationCount()
       this.value = newValue
     }
   }
index 5721253e99e0fd7526700e6918d5598974890ca0..47ba90ae3adae0bb246268329f5738aa76db45c8 100644 (file)
@@ -16,7 +16,7 @@ internal class IntStoredProperty(private val defaultValue: Int, private val valu
   override fun setValue(thisRef: BaseState, property: KProperty<*>, value: Int) {
     val newValue = valueNormalizer?.invoke(value) ?: value
     if (this.value != newValue) {
-      thisRef.ownModificationCount++
+      thisRef.intIncrementModificationCount()
       this.value = newValue
     }
   }
index e03d5f201289daec6c05aacfc4d5c7cc8b6694a4..9c635d9566db4fe529b91ce9c1cb7d16ee8a27d2 100644 (file)
@@ -16,7 +16,7 @@ internal class LongStoredProperty(private val defaultValue: Long, private val va
   override fun setValue(thisRef: BaseState, property: KProperty<*>, value: Long) {
     val newValue = valueNormalizer?.invoke(value) ?: value
     if (this.value != newValue) {
-      thisRef.ownModificationCount++
+      thisRef.intIncrementModificationCount()
       this.value = newValue
     }
   }
index 9f6ea1c8fbe7be79eec90fdf65d1f15b7d7e19cd..0f8a4a8ea29967aa436925dce715f18790470613 100644 (file)
@@ -14,7 +14,7 @@ internal abstract class ObjectStateStoredPropertyBase<T>(protected var value: T)
 
   override fun setValue(thisRef: BaseState, property: KProperty<*>, @Suppress("PARAMETER_NAME_CHANGED_ON_OVERRIDE") newValue: T) {
     if (value != newValue) {
-      thisRef.ownModificationCount++
+      thisRef.intIncrementModificationCount()
       value = newValue
     }
   }
index 1ff733d8128cf20918007008a506dd822ced7329..b94798acb2049e756216bf15175f9fe733a5b98b 100644 (file)
@@ -16,7 +16,7 @@ internal class NormalizedStringStoredProperty(private val defaultValue: String?)
   override fun setValue(thisRef: BaseState, property: KProperty<*>, value: String?) {
     val newValue = if (value.isNullOrEmpty()) null else value
     if (this.value != newValue) {
-      thisRef.ownModificationCount++
+      thisRef.intIncrementModificationCount()
       this.value = newValue
     }
   }
index 13ecee88573d75d441e82665e729150e1db0125f..5c598d4e6adb351fde842c334fb60deaf3727cda 100644 (file)
@@ -13,16 +13,20 @@ import com.intellij.util.xmlb.SerializationFilter
 import com.intellij.util.xmlb.annotations.Transient
 import gnu.trove.THashMap
 import java.nio.charset.Charset
+import java.util.concurrent.atomic.AtomicLongFieldUpdater
 
 private val LOG = logger<BaseState>()
 
 abstract class BaseState : SerializationFilter, ModificationTracker {
+  companion object {
+    private val MOD_COUNT_UPDATER = AtomicLongFieldUpdater.newUpdater(BaseState::class.java, "ownModificationCount")
+  }
+
   private val properties: MutableList<StoredProperty> = SmartList()
 
   @Volatile
   @Transient
-  @JvmField
-  internal var ownModificationCount: Long = 0
+  private var ownModificationCount: Long = 0
 
   fun <T> property(): StoredPropertyBase<T?> {
     val result = ObjectStoredProperty<T?>(null)
@@ -144,7 +148,11 @@ abstract class BaseState : SerializationFilter, ModificationTracker {
   }
 
   protected fun incrementModificationCount() {
-    ownModificationCount++
+    intIncrementModificationCount()
+  }
+
+  internal fun intIncrementModificationCount() {
+    MOD_COUNT_UPDATER.incrementAndGet(this)
   }
 
   override fun accepts(accessor: Accessor, bean: Any): Boolean {
index 6f9ad6f7effd4ec7509dbe955a94b71432580527..d71cce3c51cae830b1507c800ebae746fe0602ef 100644 (file)
@@ -44,7 +44,7 @@ public class ModuleRootManagerImpl extends ModuleRootManager implements Disposab
   private final OrderRootsCache myOrderRootsCache;
   private final Map<RootModelImpl, Throwable> myModelCreations = new THashMap<>();
 
-  protected SimpleModificationTracker myModificationTracker;
+  protected final SimpleModificationTracker myModificationTracker = new SimpleModificationTracker();
 
   public ModuleRootManagerImpl(@NotNull Module module,
                                @NotNull ProjectRootManagerImpl projectRootManager,
index 22869e7f84a97dc327b7d97b3163d10f2748a05f..ad7a6e4756f8b714a563f1187ab626daabae9324 100644 (file)
@@ -1,17 +1,5 @@
 /*
- * Copyright 2000-2017 JetBrains s.r.o.
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
+ * Copyright 2000-2017 JetBrains s.r.o. Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE file.
  */
 
 package com.intellij.openapi.roots.impl.libraries;
@@ -26,10 +14,7 @@ import com.intellij.openapi.roots.ProjectModelExternalSource;
 import com.intellij.openapi.roots.libraries.Library;
 import com.intellij.openapi.roots.libraries.LibraryTable;
 import com.intellij.openapi.roots.libraries.PersistentLibraryKind;
-import com.intellij.openapi.util.Disposer;
-import com.intellij.openapi.util.InvalidDataException;
-import com.intellij.openapi.util.JDOMExternalizable;
-import com.intellij.openapi.util.WriteExternalException;
+import com.intellij.openapi.util.*;
 import com.intellij.openapi.util.registry.Registry;
 import com.intellij.openapi.util.text.StringUtil;
 import com.intellij.util.EventDispatcher;
@@ -48,7 +33,7 @@ public abstract class LibraryTableBase implements PersistentStateComponent<Eleme
   private LibraryModel myModel = new LibraryModel();
   private boolean myFirstLoad = true;
 
-  private volatile long myModificationCount;
+  private final SimpleModificationTracker myModificationTracker = new SimpleModificationTracker();
 
   @NotNull
   @Override
@@ -74,7 +59,7 @@ public abstract class LibraryTableBase implements PersistentStateComponent<Eleme
   }
 
   @Override
-  public void loadState(final Element element) {
+  public void loadState(@NotNull Element element) {
     if (myFirstLoad) {
       myModel.readExternal(element);
     }
@@ -90,7 +75,7 @@ public abstract class LibraryTableBase implements PersistentStateComponent<Eleme
   }
 
   public long getStateModificationCount() {
-    return myModificationCount;
+    return myModificationTracker.getModificationCount();
   }
 
   @Override
@@ -162,7 +147,7 @@ public abstract class LibraryTableBase implements PersistentStateComponent<Eleme
     if (Registry.is("store.track.module.root.manager.changes", false)) {
       LOG.error("library");
     }
-    myModificationCount++;
+    myModificationTracker.incModificationCount();
   }
 
   @NotNull