From 6dd0901ca02ca4efa9a8218e5cd00843a01a1df2 Mon Sep 17 00:00:00 2001 From: Sergey Patrikeev Date: Wed, 6 May 2020 15:42:35 +0300 Subject: [PATCH] Revert "Fix return value of EmptyInputDataDiffBuilder and MapInputDataDiffBuilder." This reverts commit 8308aaac GitOrigin-RevId: b8eef48ffb606aa1f3f202a896132323812fd210 --- .../impl/CollectionInputDataDiffBuilder.java | 60 +++++++++ .../impl/EmptyInputDataDiffBuilder.java | 64 ++------- .../impl/MapInputDataDiffBuilder.java | 123 +++++++++++++----- .../KeyCollectionForwardIndexAccessor.java | 46 ++----- 4 files changed, 178 insertions(+), 115 deletions(-) create mode 100644 platform/util/src/com/intellij/util/indexing/impl/CollectionInputDataDiffBuilder.java diff --git a/platform/util/src/com/intellij/util/indexing/impl/CollectionInputDataDiffBuilder.java b/platform/util/src/com/intellij/util/indexing/impl/CollectionInputDataDiffBuilder.java new file mode 100644 index 000000000000..978294205a32 --- /dev/null +++ b/platform/util/src/com/intellij/util/indexing/impl/CollectionInputDataDiffBuilder.java @@ -0,0 +1,60 @@ +/* + * Copyright 2000-2016 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. + */ +package com.intellij.util.indexing.impl; + +import com.intellij.util.indexing.StorageException; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +import java.util.Collection; +import java.util.Collections; +import java.util.Map; + +public class CollectionInputDataDiffBuilder extends DirectInputDataDiffBuilder { + @NotNull + private final Collection mySeq; + + public CollectionInputDataDiffBuilder(int inputId, @Nullable Collection seq) { + super(inputId); + mySeq = seq == null ? Collections.emptySet() : seq; + } + + @Override + public boolean differentiate(@NotNull Map newData, + @NotNull KeyValueUpdateProcessor addProcessor, + @NotNull KeyValueUpdateProcessor updateProcessor, + @NotNull RemovedKeyProcessor removeProcessor) throws StorageException { + return differentiateWithKeySeq(mySeq, newData, myInputId, addProcessor, removeProcessor); + } + + @NotNull + @Override + public Collection getKeys() { + return mySeq; + } + + static boolean differentiateWithKeySeq(@NotNull Collection currentData, + @NotNull Map newData, + int inputId, + @NotNull KeyValueUpdateProcessor addProcessor, + @NotNull RemovedKeyProcessor removeProcessor) throws StorageException { + for (Key key : currentData) { + removeProcessor.process(key, inputId); + } + EmptyInputDataDiffBuilder.processKeys(newData, addProcessor, inputId); + return true; + } +} diff --git a/platform/util/src/com/intellij/util/indexing/impl/EmptyInputDataDiffBuilder.java b/platform/util/src/com/intellij/util/indexing/impl/EmptyInputDataDiffBuilder.java index 526521114d98..6042027a07e6 100644 --- a/platform/util/src/com/intellij/util/indexing/impl/EmptyInputDataDiffBuilder.java +++ b/platform/util/src/com/intellij/util/indexing/impl/EmptyInputDataDiffBuilder.java @@ -15,7 +15,6 @@ */ package com.intellij.util.indexing.impl; -import com.intellij.openapi.util.Ref; import com.intellij.util.indexing.StorageException; import gnu.trove.THashMap; import gnu.trove.TObjectObjectProcedure; @@ -25,7 +24,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Map; -public final class EmptyInputDataDiffBuilder extends DirectInputDataDiffBuilder { +public class EmptyInputDataDiffBuilder extends DirectInputDataDiffBuilder { public EmptyInputDataDiffBuilder(int inputId) { super(inputId); } @@ -37,25 +36,23 @@ public final class EmptyInputDataDiffBuilder extends DirectInputData @Override public boolean differentiate(@NotNull Map newData, - @NotNull final KeyValueUpdateProcessor addProcessor, - @NotNull KeyValueUpdateProcessor updateProcessor, - @NotNull RemovedKeyProcessor removeProcessor) throws StorageException { - return processAllKeyValuesAsAdded(myInputId, newData, addProcessor); + @NotNull final KeyValueUpdateProcessor addProcessor, + @NotNull KeyValueUpdateProcessor updateProcessor, + @NotNull RemovedKeyProcessor removeProcessor) throws StorageException { + return processKeys(newData, addProcessor, myInputId); } - public static boolean processAllKeyValuesAsAdded(int inputId, - @NotNull Map newData, - @NotNull final KeyValueUpdateProcessor addProcessor) + static boolean processKeys(@NotNull Map currentData, + @NotNull final KeyValueUpdateProcessor processor, + final int inputId) throws StorageException { - Ref anyAdded = Ref.create(false); - if (newData instanceof THashMap) { + if (currentData instanceof THashMap) { final StorageException[] exception = new StorageException[]{null}; - ((THashMap)newData).forEachEntry(new TObjectObjectProcedure() { + ((THashMap)currentData).forEachEntry(new TObjectObjectProcedure() { @Override public boolean execute(Key k, Value v) { try { - addProcessor.process(k, v, inputId); - anyAdded.set(true); + processor.process(k, v, inputId); } catch (StorageException e) { exception[0] = e; @@ -69,44 +66,11 @@ public final class EmptyInputDataDiffBuilder extends DirectInputData } } else { - for (Map.Entry entry : newData.entrySet()) { - addProcessor.process(entry.getKey(), entry.getValue(), inputId); - anyAdded.set(true); + for (Map.Entry entry : currentData.entrySet()) { + processor.process(entry.getKey(), entry.getValue(), inputId); } } - return anyAdded.get(); - } - - public static boolean processAllKeyValuesAsRemoved(int inputId, - @NotNull Map newData, - @NotNull RemovedKeyProcessor removedProcessor) - throws StorageException { - Ref anyRemoved = Ref.create(false); - if (newData instanceof THashMap) { - final StorageException[] exception = new StorageException[]{null}; - ((THashMap)newData).forEachEntry(new TObjectObjectProcedure() { - @Override - public boolean execute(Key k, Value v) { - try { - removedProcessor.process(k, inputId); - anyRemoved.set(true); - } - catch (StorageException e) { - exception[0] = e; - return false; - } - return true; - } - }); - if (exception[0] != null) throw exception[0]; - } - else { - for (Key key : newData.keySet()) { - removedProcessor.process(key, inputId); - anyRemoved.set(true); - } - } - return anyRemoved.get(); + return true; } } diff --git a/platform/util/src/com/intellij/util/indexing/impl/MapInputDataDiffBuilder.java b/platform/util/src/com/intellij/util/indexing/impl/MapInputDataDiffBuilder.java index bdf239558d9c..ec39a6d3080e 100644 --- a/platform/util/src/com/intellij/util/indexing/impl/MapInputDataDiffBuilder.java +++ b/platform/util/src/com/intellij/util/indexing/impl/MapInputDataDiffBuilder.java @@ -15,16 +15,23 @@ */ package com.intellij.util.indexing.impl; +import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.util.Comparing; +import com.intellij.util.SystemProperties; import com.intellij.util.indexing.StorageException; +import gnu.trove.THashMap; +import gnu.trove.TObjectObjectProcedure; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.Collection; import java.util.Collections; import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; + +public class MapInputDataDiffBuilder extends DirectInputDataDiffBuilder { + private static final boolean ourDiffUpdateEnabled = SystemProperties.getBooleanProperty("idea.disable.diff.index.update", true); -public final class MapInputDataDiffBuilder extends DirectInputDataDiffBuilder { @NotNull private final Map myMap; @@ -35,44 +42,91 @@ public final class MapInputDataDiffBuilder extends DirectInputDataDi @Override public boolean differentiate(@NotNull Map newData, - @NotNull KeyValueUpdateProcessor addProcessor, - @NotNull KeyValueUpdateProcessor updateProcessor, - @NotNull RemovedKeyProcessor removeProcessor) throws StorageException { - if (myMap.isEmpty()) { - return EmptyInputDataDiffBuilder.processAllKeyValuesAsAdded(myInputId, newData, addProcessor); - } - if (newData.isEmpty()) { - return EmptyInputDataDiffBuilder.processAllKeyValuesAsRemoved(myInputId, newData, removeProcessor); - } + @NotNull KeyValueUpdateProcessor addProcessor, + @NotNull KeyValueUpdateProcessor updateProcessor, + @NotNull RemovedKeyProcessor removeProcessor) throws StorageException { + if (ourDiffUpdateEnabled) { + if (myMap.isEmpty()) { + EmptyInputDataDiffBuilder.processKeys(newData, addProcessor, myInputId); + } + else if (newData.isEmpty()) { + processAllKeysAsDeleted(removeProcessor); + } + else { + int added = 0; + int removed = 0; - int added = 0; - int removed = 0; - int updated = 0; - - for (Map.Entry e : myMap.entrySet()) { - Key key = e.getKey(); - Value oldValue = e.getValue(); - Value newValue = newData.get(key); - if (!Comparing.equal(oldValue, newValue) || (newValue == null && !newData.containsKey(key))) { - if (newData.containsKey(key)) { - updateProcessor.process(key, newValue, myInputId); - updated++; + for (Map.Entry e: myMap.entrySet()) { + final Key key = e.getKey(); + final Value newValue = newData.get(key); + if (!Comparing.equal(e.getValue(), newValue) || (newValue == null && !newData.containsKey(key))) { + if (!newData.containsKey(key)) { + removeProcessor.process(key, myInputId); + } else { + updateProcessor.process(key, newValue, myInputId); + added++; + } + removed++; + } } - else { - removeProcessor.process(key, myInputId); - removed++; + + for (Map.Entry e : newData.entrySet()) { + final Key key = e.getKey(); + if (!myMap.containsKey(key)) { + addProcessor.process(key, e.getValue(), myInputId); + added++; + } + } + + incrementalAdditions.addAndGet(added); + incrementalRemovals.addAndGet(removed); + int totalRequests = requests.incrementAndGet(); + totalRemovals.addAndGet(myMap.size()); + totalAdditions.addAndGet(newData.size()); + + if ((totalRequests & 0xFFFF) == 0 && DebugAssertions.DEBUG) { + Logger.getInstance(getClass()).info("Incremental index diff update:" + requests + + ", removals:" + totalRemovals + "->" + incrementalRemovals + + ", additions:" + totalAdditions + "->" + incrementalAdditions + + ", no op changes:" + noopModifications + ); + } + + if (added == 0 && removed == 0) { + noopModifications.incrementAndGet(); + return false; } } } + else { + CollectionInputDataDiffBuilder.differentiateWithKeySeq(myMap.keySet(), newData, myInputId, addProcessor, removeProcessor); + } + return true; + } - for (Map.Entry e : newData.entrySet()) { - final Key newKey = e.getKey(); - if (!myMap.containsKey(newKey)) { - addProcessor.process(newKey, e.getValue(), myInputId); - added++; + private void processAllKeysAsDeleted(final RemovedKeyProcessor removeProcessor) throws StorageException { + if (myMap instanceof THashMap) { + final StorageException[] exception = new StorageException[]{null}; + ((THashMap)myMap).forEachEntry(new TObjectObjectProcedure() { + @Override + public boolean execute(Key k, Value v) { + try { + removeProcessor.process(k, myInputId); + } + catch (StorageException e) { + exception[0] = e; + return false; + } + return true; + } + }); + if (exception[0] != null) throw exception[0]; + } + else { + for (Key key : myMap.keySet()) { + removeProcessor.process(key, myInputId); } } - return added != 0 || removed != 0 || updated != 0; } @NotNull @@ -80,4 +134,11 @@ public final class MapInputDataDiffBuilder extends DirectInputDataDi public Collection getKeys() { return myMap.keySet(); } + + private static final AtomicInteger requests = new AtomicInteger(); + private static final AtomicInteger totalRemovals = new AtomicInteger(); + private static final AtomicInteger totalAdditions = new AtomicInteger(); + private static final AtomicInteger incrementalRemovals = new AtomicInteger(); + private static final AtomicInteger incrementalAdditions = new AtomicInteger(); + private static final AtomicInteger noopModifications = new AtomicInteger(); } diff --git a/platform/util/src/com/intellij/util/indexing/impl/forward/KeyCollectionForwardIndexAccessor.java b/platform/util/src/com/intellij/util/indexing/impl/forward/KeyCollectionForwardIndexAccessor.java index 92c1e54471a2..aabf647d22cf 100644 --- a/platform/util/src/com/intellij/util/indexing/impl/forward/KeyCollectionForwardIndexAccessor.java +++ b/platform/util/src/com/intellij/util/indexing/impl/forward/KeyCollectionForwardIndexAccessor.java @@ -2,16 +2,18 @@ package com.intellij.util.indexing.impl.forward; import com.intellij.util.indexing.IndexExtension; -import com.intellij.util.indexing.StorageException; -import com.intellij.util.indexing.impl.*; +import com.intellij.util.indexing.IndexId; +import com.intellij.util.indexing.impl.CollectionInputDataDiffBuilder; +import com.intellij.util.indexing.impl.InputData; +import com.intellij.util.indexing.impl.InputDataDiffBuilder; +import com.intellij.util.indexing.impl.InputIndexDataExternalizer; import com.intellij.util.io.DataExternalizer; +import com.intellij.util.io.KeyDescriptor; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.Collection; -import java.util.Collections; -import java.util.Map; import java.util.Set; @ApiStatus.Experimental @@ -21,12 +23,16 @@ public class KeyCollectionForwardIndexAccessor extends AbstractForwa } public KeyCollectionForwardIndexAccessor(@NotNull IndexExtension extension) { - this(new InputIndexDataExternalizer<>(extension.getKeyDescriptor(), extension.getName())); + this(extension.getKeyDescriptor(), extension.getName()); + } + + public KeyCollectionForwardIndexAccessor(@NotNull KeyDescriptor externalizer, @NotNull IndexId indexId) { + super(new InputIndexDataExternalizer<>(externalizer, indexId)); } @Override protected InputDataDiffBuilder createDiffBuilder(int inputId, @Nullable Collection keys) { - return new KeyCollectionInputDataDiffBuilder<>(inputId, keys); + return new CollectionInputDataDiffBuilder<>(inputId, keys); } @Nullable @@ -40,32 +46,4 @@ public class KeyCollectionForwardIndexAccessor extends AbstractForwa protected int getBufferInitialSize(@NotNull Collection keys) { return 4 * keys.size(); } - - // Marks all keys as removed and then all new key-values as added. Does not try to find keys that haven't changed. - private static final class KeyCollectionInputDataDiffBuilder extends DirectInputDataDiffBuilder { - @NotNull - private final Collection myKeys; - - KeyCollectionInputDataDiffBuilder(int inputId, @Nullable Collection keys) { - super(inputId); - myKeys = keys == null ? Collections.emptySet() : keys; - } - - @Override - public boolean differentiate(@NotNull Map newData, - @NotNull KeyValueUpdateProcessor addProcessor, - @NotNull KeyValueUpdateProcessor updateProcessor, - @NotNull RemovedKeyProcessor removeProcessor) throws StorageException { - for (Key key : myKeys) { - removeProcessor.process(key, myInputId); - } - return !myKeys.isEmpty() || EmptyInputDataDiffBuilder.processAllKeyValuesAsAdded(myInputId, newData, addProcessor); - } - - @NotNull - @Override - public Collection getKeys() { - return myKeys; - } - } }