From 13dfa850c0f718e651e2bcb0b7cf883a8b6a4497 Mon Sep 17 00:00:00 2001 From: Ruslan Cheremin Date: Thu, 22 Aug 2024 15:28:28 +0200 Subject: [PATCH] [indexes][refactoring] combine add/remove/update processors into single `UpdatedEntryProcessor` + 3 different processors were used during diff(old,new) calculation, to report added/updated/removed entries -> actual processing of all 3 categories is quite similar, so I replace 3 processors with 1 `UpdatedEntryProcessor` with an additional `UpdateKind` enum arg. GitOrigin-RevId: 53b1121ee1cb0899c87de17a9148b0b51029dd45 --- .../stubs/StubCumulativeInputDiffBuilder.java | 22 ++++----- .../com/intellij/psi/stubs/StubIndexEx.java | 6 +-- .../SingleEntryIndexForwardIndexAccessor.java | 10 ++-- ...ileElementTypeStubModificationTracker.java | 2 +- platform/util/api-dump-unreviewed.txt | 14 ------ .../impl/EmptyInputDataDiffBuilder.java | 17 +++---- .../indexing/impl/InputDataDiffBuilder.java | 20 +------- .../impl/KeyValueUpdateProcessor.java | 22 --------- .../impl/MapInputDataDiffBuilder.java | 18 +++---- .../util/indexing/impl/MapReduceIndex.java | 48 +++++++------------ .../indexing/impl/RemovedKeyProcessor.java | 22 --------- .../util/indexing/impl/UpdateData.java | 16 ++----- .../indexing/impl/UpdatedEntryProcessor.java | 27 +++++++++++ .../impl/forward/ForwardIndexAccessor.java | 5 +- .../KeyCollectionForwardIndexAccessor.java | 10 ++-- .../IndexStorageLayoutProviderTestBase.java | 25 +++------- 16 files changed, 97 insertions(+), 187 deletions(-) delete mode 100644 platform/util/src/com/intellij/util/indexing/impl/KeyValueUpdateProcessor.java delete mode 100644 platform/util/src/com/intellij/util/indexing/impl/RemovedKeyProcessor.java create mode 100644 platform/util/src/com/intellij/util/indexing/impl/UpdatedEntryProcessor.java diff --git a/platform/indexing-impl/src/com/intellij/psi/stubs/StubCumulativeInputDiffBuilder.java b/platform/indexing-impl/src/com/intellij/psi/stubs/StubCumulativeInputDiffBuilder.java index 7fa80718358c..9cd10fce7a00 100644 --- a/platform/indexing-impl/src/com/intellij/psi/stubs/StubCumulativeInputDiffBuilder.java +++ b/platform/indexing-impl/src/com/intellij/psi/stubs/StubCumulativeInputDiffBuilder.java @@ -1,4 +1,4 @@ -// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.intellij.psi.stubs; import com.intellij.openapi.diagnostic.Logger; @@ -10,8 +10,7 @@ import com.intellij.util.indexing.FileBasedIndexEx; import com.intellij.util.indexing.StorageException; import com.intellij.util.indexing.impl.DirectInputDataDiffBuilder; import com.intellij.util.indexing.impl.IndexDebugProperties; -import com.intellij.util.indexing.impl.KeyValueUpdateProcessor; -import com.intellij.util.indexing.impl.RemovedKeyProcessor; +import com.intellij.util.indexing.impl.UpdatedEntryProcessor; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -28,20 +27,15 @@ final class StubCumulativeInputDiffBuilder extends DirectInputDataDiffBuilder newData, - @NotNull KeyValueUpdateProcessor addProcessor, - @NotNull KeyValueUpdateProcessor updateProcessor, - @NotNull RemovedKeyProcessor removeProcessor) throws StorageException - { - return differentiate(newData, addProcessor, updateProcessor, removeProcessor, false); + @NotNull UpdatedEntryProcessor changesProcessor) throws StorageException { + return differentiate(newData, changesProcessor, false); } /** * @param dryRun if true, won't update the stub indices */ public boolean differentiate(@NotNull Map newData, - @NotNull KeyValueUpdateProcessor addProcessor, - @NotNull KeyValueUpdateProcessor updateProcessor, - @NotNull RemovedKeyProcessor removeProcessor, + @NotNull UpdatedEntryProcessor changesProcessor, boolean dryRun) throws StorageException { if (FileBasedIndexEx.TRACE_STUB_INDEX_UPDATES) { LOG.info((dryRun ? "[dry run]" : "") + "differentiate: inputId=" + myInputId + @@ -69,9 +63,9 @@ final class StubCumulativeInputDiffBuilder extends DirectInputDataDiffBuilder { * @return false if there is no difference and true otherwise */ public abstract boolean differentiate(@NotNull Map newData, - @NotNull KeyValueUpdateProcessor addProcessor, - @NotNull KeyValueUpdateProcessor updateProcessor, - @NotNull RemovedKeyProcessor removeProcessor) throws StorageException; + @NotNull UpdatedEntryProcessor changesProcessor) throws StorageException; } diff --git a/platform/util/src/com/intellij/util/indexing/impl/KeyValueUpdateProcessor.java b/platform/util/src/com/intellij/util/indexing/impl/KeyValueUpdateProcessor.java deleted file mode 100644 index 400e26c73f9a..000000000000 --- a/platform/util/src/com/intellij/util/indexing/impl/KeyValueUpdateProcessor.java +++ /dev/null @@ -1,22 +0,0 @@ -/* - * 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; - -public interface KeyValueUpdateProcessor { - void process(Key key, Value value, int inputId) throws StorageException; -} 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 11331d883b48..eeecdad747f0 100644 --- a/platform/util/src/com/intellij/util/indexing/impl/MapInputDataDiffBuilder.java +++ b/platform/util/src/com/intellij/util/indexing/impl/MapInputDataDiffBuilder.java @@ -1,9 +1,10 @@ -// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.intellij.util.indexing.impl; import com.intellij.openapi.diagnostic.Logger; import com.intellij.openapi.util.Comparing; import com.intellij.util.indexing.StorageException; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -12,6 +13,7 @@ import java.util.Collections; import java.util.Map; import java.util.concurrent.atomic.AtomicInteger; +@ApiStatus.Internal public class MapInputDataDiffBuilder extends DirectInputDataDiffBuilder { private final @NotNull Map myMap; @@ -22,14 +24,12 @@ public class MapInputDataDiffBuilder extends DirectInputDataDiffBuil @Override public boolean differentiate(@NotNull Map newData, - @NotNull KeyValueUpdateProcessor addProcessor, - @NotNull KeyValueUpdateProcessor updateProcessor, - @NotNull RemovedKeyProcessor removeProcessor) throws StorageException { + @NotNull UpdatedEntryProcessor changesProcessor) throws StorageException { if (myMap.isEmpty()) { - return EmptyInputDataDiffBuilder.processAllKeyValuesAsAdded(myInputId, newData, addProcessor); + return EmptyInputDataDiffBuilder.processAllKeyValuesAsAdded(myInputId, newData, changesProcessor); } if (newData.isEmpty()) { - return EmptyInputDataDiffBuilder.processAllKeyValuesAsRemoved(myInputId, myMap, removeProcessor); + return EmptyInputDataDiffBuilder.processAllKeyValuesAsRemoved(myInputId, myMap, changesProcessor); } int added = 0; @@ -42,11 +42,11 @@ public class MapInputDataDiffBuilder extends DirectInputDataDiffBuil Value newValue = newData.get(key); if (!Comparing.equal(oldValue, newValue) || (newValue == null && !newData.containsKey(key))) { if (newData.containsKey(key)) { - updateProcessor.process(key, newValue, myInputId); + changesProcessor.updated(key, newValue, myInputId); updated++; } else { - removeProcessor.process(key, myInputId); + changesProcessor.removed(key, myInputId); removed++; } } @@ -55,7 +55,7 @@ public class MapInputDataDiffBuilder extends DirectInputDataDiffBuil for (Map.Entry e : newData.entrySet()) { final Key newKey = e.getKey(); if (!myMap.containsKey(newKey)) { - addProcessor.process(newKey, e.getValue(), myInputId); + changesProcessor.added(newKey, e.getValue(), myInputId); added++; } } diff --git a/platform/util/src/com/intellij/util/indexing/impl/MapReduceIndex.java b/platform/util/src/com/intellij/util/indexing/impl/MapReduceIndex.java index a5791c1e2f4e..99662b883413 100644 --- a/platform/util/src/com/intellij/util/indexing/impl/MapReduceIndex.java +++ b/platform/util/src/com/intellij/util/indexing/impl/MapReduceIndex.java @@ -313,16 +313,11 @@ public abstract class MapReduceIndex implements InvertedIndex inputId, indexId(), - (addedEntriesProcessor, updatedEntriesProcessor, removedEntriesProcessor) -> { + (changedEntriesProcessor) -> { try { InputDataDiffBuilder diffBuilder = getKeysDiffBuilder(inputId); Map newData = inputData.getKeyValues(); - return diffBuilder.differentiate( - newData, - addedEntriesProcessor, - updatedEntriesProcessor, - removedEntriesProcessor - ); + return diffBuilder.differentiate(newData, changedEntriesProcessor); } catch (IOException e) { throw new StorageException("Error while applying " + this, e); @@ -391,27 +386,21 @@ public abstract class MapReduceIndex implements InvertedIndex protected abstract void requestRebuild(@NotNull Throwable e); - private final RemovedKeyProcessor myRemovedKeyProcessor = new RemovedKeyProcessor() { + private final UpdatedEntryProcessor changedEntriesProcessor = new UpdatedEntryProcessor() { @Override - public void process(Key key, int inputId) throws StorageException { + public void process(@NotNull UpdateKind kind, Key key, Value value, int inputId) throws StorageException { incrementModificationStamp(); - myStorage.removeAllValues(key, inputId); - } - }; - - private final KeyValueUpdateProcessor myAddedKeyProcessor = new KeyValueUpdateProcessor() { - @Override - public void process(Key key, Value value, int inputId) throws StorageException { - incrementModificationStamp(); - myStorage.addValue(key, inputId, value); - } - }; - - private final KeyValueUpdateProcessor myUpdatedKeyProcessor = new KeyValueUpdateProcessor() { - @Override - public void process(Key key, Value value, int inputId) throws StorageException { - incrementModificationStamp(); - myStorage.updateValue(key, inputId, value); + switch (kind) { + case ADDED: + myStorage.addValue(key, inputId, value); + break; + case UPDATED: + myStorage.updateValue(key, inputId, value); + break; + case REMOVED: + myStorage.removeAllValues(key, inputId); + break; + } } }; @@ -425,11 +414,8 @@ public abstract class MapReduceIndex implements InvertedIndex IndexId oldIndexId = IndexDebugProperties.DEBUG_INDEX_ID.get(); try { IndexDebugProperties.DEBUG_INDEX_ID.set(myIndexId); - boolean hasDifference = updateData.iterateChanges( - myAddedKeyProcessor, - myUpdatedKeyProcessor, - myRemovedKeyProcessor - ); + boolean hasDifference = updateData.iterateChanges(changedEntriesProcessor); + //TODO RC: separate lock for forwardIndex? -- it seems it is used only in updates (?), i.e. only in one place, // so if it is out-of-sync with inverted index it is not a big deal since nobody able to see it? // ...but it is important to not allow same fileId data to be applied by different threads, because diff --git a/platform/util/src/com/intellij/util/indexing/impl/RemovedKeyProcessor.java b/platform/util/src/com/intellij/util/indexing/impl/RemovedKeyProcessor.java deleted file mode 100644 index be3be3b287bf..000000000000 --- a/platform/util/src/com/intellij/util/indexing/impl/RemovedKeyProcessor.java +++ /dev/null @@ -1,22 +0,0 @@ -/* - * 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; - -public interface RemovedKeyProcessor { - void process(Key key, int inputId) throws StorageException; -} diff --git a/platform/util/src/com/intellij/util/indexing/impl/UpdateData.java b/platform/util/src/com/intellij/util/indexing/impl/UpdateData.java index fd176ed6ede1..6e621d510366 100644 --- a/platform/util/src/com/intellij/util/indexing/impl/UpdateData.java +++ b/platform/util/src/com/intellij/util/indexing/impl/UpdateData.java @@ -15,7 +15,7 @@ import java.io.IOException; *

* The update as a whole is going to the forward index, while the changes are going to the inverted index. * So the {@link #updateForwardIndex()} is to update the forward index with the new data, and - * {@link #iterateChanges(KeyValueUpdateProcessor, KeyValueUpdateProcessor, RemovedKeyProcessor)} is to stream + * {@link #iterateChanges(UpdatedEntryProcessor)} is to stream * the changes against current data to apply on the inverted index. *

* The 2 sides are separated to allow indexes to skip forward index update, and/or to provide an optimized version @@ -47,14 +47,8 @@ public final class UpdateData { //MAYBE RC: move ChangesProducer to the upper level, and make UpdateData implement ChangesProducer, so this // method become .forEachChange()? /** @return true if new data is different from current data -- which means at least one processor _was_ called */ - boolean iterateChanges(@NotNull KeyValueUpdateProcessor addedEntriesProcessor, - @NotNull KeyValueUpdateProcessor updatedEntriesProcessor, - @NotNull RemovedKeyProcessor removedEntriesProcessor) throws StorageException { - return changesProducer.forEachChange( - addedEntriesProcessor, - updatedEntriesProcessor, - removedEntriesProcessor - ); + boolean iterateChanges(@NotNull UpdatedEntryProcessor changedEntriesProcessor) throws StorageException { + return changesProducer.forEachChange(changedEntriesProcessor); } void updateForwardIndex() throws IOException { @@ -90,8 +84,6 @@ public final class UpdateData { @ApiStatus.Internal @FunctionalInterface public interface ChangesProducer { - boolean forEachChange(@NotNull KeyValueUpdateProcessor addedEntriesProcessor, - @NotNull KeyValueUpdateProcessor updatedEntriesProcessor, - @NotNull RemovedKeyProcessor removedEntriesProcessor) throws StorageException; + boolean forEachChange(@NotNull UpdatedEntryProcessor changedEntriesProcessor) throws StorageException; } } diff --git a/platform/util/src/com/intellij/util/indexing/impl/UpdatedEntryProcessor.java b/platform/util/src/com/intellij/util/indexing/impl/UpdatedEntryProcessor.java new file mode 100644 index 000000000000..d802165b8cac --- /dev/null +++ b/platform/util/src/com/intellij/util/indexing/impl/UpdatedEntryProcessor.java @@ -0,0 +1,27 @@ +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +package com.intellij.util.indexing.impl; + +import com.intellij.util.indexing.StorageException; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; + +@ApiStatus.Internal +@FunctionalInterface +public interface UpdatedEntryProcessor { + + enum UpdateKind {ADDED, UPDATED, REMOVED} + + default void added(Key key, Value value, int inputId) throws StorageException { + process(UpdateKind.ADDED, key, value, inputId); + } + + default void updated(Key key, Value value, int inputId) throws StorageException { + process(UpdateKind.UPDATED, key, value, inputId); + } + + default void removed(Key key, int inputId) throws StorageException { + process(UpdateKind.REMOVED, key, /*value: */ null, inputId); + } + + void process(@NotNull UpdateKind kind, Key key, Value value, int inputId) throws StorageException; +} diff --git a/platform/util/src/com/intellij/util/indexing/impl/forward/ForwardIndexAccessor.java b/platform/util/src/com/intellij/util/indexing/impl/forward/ForwardIndexAccessor.java index c582866f4b23..0267a51939af 100644 --- a/platform/util/src/com/intellij/util/indexing/impl/forward/ForwardIndexAccessor.java +++ b/platform/util/src/com/intellij/util/indexing/impl/forward/ForwardIndexAccessor.java @@ -4,8 +4,7 @@ package com.intellij.util.indexing.impl.forward; import com.intellij.openapi.util.io.ByteArraySequence; import com.intellij.util.indexing.impl.InputData; import com.intellij.util.indexing.impl.InputDataDiffBuilder; -import com.intellij.util.indexing.impl.KeyValueUpdateProcessor; -import com.intellij.util.indexing.impl.RemovedKeyProcessor; +import com.intellij.util.indexing.impl.UpdatedEntryProcessor; import org.jetbrains.annotations.ApiStatus.Internal; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -19,7 +18,7 @@ public interface ForwardIndexAccessor { * Method deserializes sequence bytes back into a {@link InputData}, and creates a diff-builder from this input * (with id=inputId). * The diff-builder could be used later to calculate a diff between that inputData, and any other inputData - * (see {@link InputDataDiffBuilder#differentiate(Map, KeyValueUpdateProcessor, KeyValueUpdateProcessor, RemovedKeyProcessor)}) + * (see {@link InputDataDiffBuilder#differentiate(Map, UpdatedEntryProcessor)}) */ @NotNull InputDataDiffBuilder getDiffBuilder(int inputId, @Nullable ByteArraySequence sequence) throws IOException; 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 827f782c8cec..72a2a9d07e21 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 @@ -1,4 +1,4 @@ -// Copyright 2000-2023 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. +// Copyright 2000-2024 JetBrains s.r.o. and contributors. Use of this source code is governed by the Apache 2.0 license. package com.intellij.util.indexing.impl.forward; import com.intellij.util.indexing.IndexExtension; @@ -51,13 +51,11 @@ public final class KeyCollectionForwardIndexAccessor extends Abstrac @Override public boolean differentiate(@NotNull Map newData, - @NotNull KeyValueUpdateProcessor addProcessor, - @NotNull KeyValueUpdateProcessor updateProcessor, - @NotNull RemovedKeyProcessor removeProcessor) throws StorageException { + @NotNull UpdatedEntryProcessor changesProcessor) throws StorageException { for (Key key : myKeys) { - removeProcessor.process(key, myInputId); + changesProcessor.removed(key, myInputId); } - boolean anyAdded = EmptyInputDataDiffBuilder.processAllKeyValuesAsAdded(myInputId, newData, addProcessor); + boolean anyAdded = EmptyInputDataDiffBuilder.processAllKeyValuesAsAdded(myInputId, newData, changesProcessor); boolean anyRemoved = !myKeys.isEmpty(); return anyAdded || anyRemoved; } diff --git a/platform/util/testSrc/com/intellij/util/indexing/IndexStorageLayoutProviderTestBase.java b/platform/util/testSrc/com/intellij/util/indexing/IndexStorageLayoutProviderTestBase.java index 3f65ca9a823d..9345594cf1a7 100644 --- a/platform/util/testSrc/com/intellij/util/indexing/IndexStorageLayoutProviderTestBase.java +++ b/platform/util/testSrc/com/intellij/util/indexing/IndexStorageLayoutProviderTestBase.java @@ -285,14 +285,8 @@ public abstract class IndexStorageLayoutProviderTestBase { ByteArraySequence serializedData = forwardIndex.get(inputId); forwardIndexAccessor.getDiffBuilder(inputId, serializedData).differentiate( expectedInputData, - (k, v, _inputId) -> { - inconsistencies.add("add(" + k + ", " + v + ")[" + _inputId + "]"); - }, - (k, v, _inputId) -> { - inconsistencies.add("update(" + k + ", " + v + ")[" + _inputId + "]"); - }, - (k, _inputId) -> { - inconsistencies.add("remove(" + k + ")[" + _inputId + "]"); + (kind, k, v, _inputId) -> { + inconsistencies.add(kind + "(" + k + ", " + v + ")[" + _inputId + "]"); } ); } @@ -351,16 +345,11 @@ public abstract class IndexStorageLayoutProviderTestBase { inputId, serializedData ); - diffBuilder.differentiate(expectedInputData, - (k, v, _inputId) -> { - fail(); - }, - (k, v, _inputId) -> { - fail(); - }, - (k, _inputId) -> { - fail(); - }); + diffBuilder.differentiate( + expectedInputData, + (kind, key, value, _inputId) -> { + fail("Shouldn't be any difference, but: " + kind + "(" + key + ", " + value + ")[" + _inputId + "]"); + }); } assertTrue(