From 8b6e5ebc0cfccaad14323e47337ddac47c8347aa Mon Sep 17 00:00:00 2001 From: Alexey Kudravtsev Date: Fri, 13 Sep 2024 22:42:58 +0200 Subject: [PATCH] optimization: save memory on duplicate maps by making them copy-on-write GitOrigin-RevId: d3ba1f05221a7760d8357da9a02282016706b695 --- .../template/impl/TemplateContext.java | 76 ++++++++----------- .../template/impl/TemplateManagerImpl.java | 2 +- 2 files changed, 34 insertions(+), 44 deletions(-) diff --git a/platform/analysis-impl/src/com/intellij/codeInsight/template/impl/TemplateContext.java b/platform/analysis-impl/src/com/intellij/codeInsight/template/impl/TemplateContext.java index 56efafcb286a..f23afb339ae0 100644 --- a/platform/analysis-impl/src/com/intellij/codeInsight/template/impl/TemplateContext.java +++ b/platform/analysis-impl/src/com/intellij/codeInsight/template/impl/TemplateContext.java @@ -8,7 +8,6 @@ import com.intellij.codeInsight.template.TemplateContextType; import com.intellij.util.JdomKt; import kotlin.Lazy; import kotlin.LazyKt; -import kotlin.jvm.functions.Function0; import org.jdom.Element; import org.jetbrains.annotations.*; @@ -18,11 +17,12 @@ import java.util.List; import java.util.Map; public final class TemplateContext { - private final Map myContextStates = new HashMap<>(); + private volatile Map myContextStates = Map.of(); + @NotNull public TemplateContext createCopy() { TemplateContext cloneResult = new TemplateContext(); - cloneResult.myContextStates.putAll(myContextStates); + cloneResult.myContextStates = myContextStates; return cloneResult; } @@ -36,7 +36,7 @@ public final class TemplateContext { @NotNull TemplateContext thisContext, @NotNull TemplateContext defaultContext) { for (LiveTemplateContext value : allContexts.getLiveTemplateContexts()) { - if (isEnabled(allContexts, thisContext, value) != isEnabled(allContexts, defaultContext, value)) { + if (thisContext.isEnabled(allContexts, value) != defaultContext.isEnabled(allContexts, value)) { return value; } } @@ -53,29 +53,23 @@ public final class TemplateContext { return null; } - public boolean isEnabled(@NotNull TemplateContextType contextType) { + public synchronized boolean isEnabled(@NotNull TemplateContextType contextType) { LiveTemplateContextsSnapshot allContexts = LiveTemplateContextService.getInstance().getSnapshot(); - synchronized (myContextStates) { - return isEnabledNoSync(this, allContexts, contextType.getContextId()); - } + return isEnabledNoSync(allContexts, contextType.getContextId()); } - private static boolean isEnabled(@NotNull LiveTemplateContextsSnapshot allContexts, - @NotNull TemplateContext thisContext, + private synchronized boolean isEnabled(@NotNull LiveTemplateContextsSnapshot allContexts, @NotNull LiveTemplateContext contextType) { - synchronized (thisContext.myContextStates) { - return isEnabledNoSync(thisContext, allContexts, contextType.getContextId()); - } + return isEnabledNoSync(allContexts, contextType.getContextId()); } - private static boolean isEnabledNoSync(@NotNull TemplateContext thisContext, - @NotNull LiveTemplateContextsSnapshot allContexts, - @NotNull String contextTypeId) { - Boolean storedValue = thisContext.getOwnValueNoSync(contextTypeId); + private boolean isEnabledNoSync(@NotNull LiveTemplateContextsSnapshot allContexts, + @NotNull String contextTypeId) { + Boolean storedValue = getOwnValueNoSync(contextTypeId); if (storedValue == null) { LiveTemplateContext liveTemplateContext = allContexts.getLiveTemplateContext(contextTypeId); String baseContextTypeId = liveTemplateContext != null ? liveTemplateContext.getBaseContextId() : null; - return baseContextTypeId != null && isEnabledNoSync(thisContext, allContexts, baseContextTypeId); + return baseContextTypeId != null && !baseContextTypeId.equals(contextTypeId) && isEnabledNoSync(allContexts, baseContextTypeId); } return storedValue.booleanValue(); } @@ -84,44 +78,43 @@ public final class TemplateContext { return getOwnValue(contextType.getContextId()); } - private @Nullable Boolean getOwnValue(String contextTypeId) { - synchronized (myContextStates) { - return getOwnValueNoSync(contextTypeId); - } + private synchronized @Nullable Boolean getOwnValue(String contextTypeId) { + return getOwnValueNoSync(contextTypeId); } private @Nullable Boolean getOwnValueNoSync(String contextTypeId) { return myContextStates.get(contextTypeId); } - public void setEnabled(TemplateContextType contextType, boolean value) { - synchronized (myContextStates) { - myContextStates.put(contextType.getContextId(), value); - } + public synchronized void setEnabled(@NotNull TemplateContextType contextType, boolean value) { + Map map = new HashMap<>(myContextStates); + map.put(contextType.getContextId(), value); + myContextStates = Map.copyOf(map); } // used during initialization => no sync @VisibleForTesting - public void setDefaultContext(@NotNull TemplateContext defContext) { - Map copy = new HashMap<>(myContextStates); - myContextStates.clear(); - myContextStates.putAll(defContext.myContextStates); - myContextStates.putAll(copy); + public synchronized void setDefaultContext(@NotNull TemplateContext defContext) { + Map copy = new HashMap<>(defContext.myContextStates); + copy.putAll(myContextStates); + myContextStates = Map.copyOf(copy); } // used during initialization => no sync @ApiStatus.Internal public void readTemplateContext(@NotNull Element element, @NotNull LiveTemplateContextService ltContextService) { + Map result = new HashMap<>(); Map internMap = ltContextService.getInternalIds(); for (Element option : element.getChildren("option")) { String name = option.getAttributeValue("name"); String value = option.getAttributeValue("value"); if (name != null && value != null) { - myContextStates.put(internMap.getOrDefault(name, name), Boolean.parseBoolean(value)); + result.put(internMap.getOrDefault(name, name), Boolean.parseBoolean(value)); } } - - myContextStates.putAll(makeInheritanceExplicit(this, ltContextService.getSnapshot())); + myContextStates = result; // makeInheritanceExplicit needs this + result.putAll(makeInheritanceExplicit(this, ltContextService.getSnapshot())); + myContextStates = Map.copyOf(result); } @VisibleForTesting @@ -144,7 +137,7 @@ public final class TemplateContext { } private boolean isDisabledByInheritance(TemplateContext thisContext, LiveTemplateContextsSnapshot allContexts, LiveTemplateContext type) { - if (!thisContext.hasOwnValue(type) && !isEnabled(allContexts, thisContext, type)) { + if (!thisContext.hasOwnValue(type) && !thisContext.isEnabled(allContexts, type)) { LiveTemplateContext context = type; while (context != null) { if (hasOwnValue(context)) { @@ -194,15 +187,12 @@ public final class TemplateContext { } public static @NotNull Lazy> getIdToType() { - return LazyKt.lazy(new Function0<>() { - @Override - public Map invoke() { - Map idToType = new HashMap<>(); - for (LiveTemplateContext type : LiveTemplateContextService.getInstance().getLiveTemplateContexts()) { - idToType.put(type.getContextId(), type.getTemplateContextType()); - } - return idToType; + return LazyKt.lazy(() -> { + Map idToType = new HashMap<>(); + for (LiveTemplateContext type : LiveTemplateContextService.getInstance().getLiveTemplateContexts()) { + idToType.put(type.getContextId(), type.getTemplateContextType()); } + return idToType; }); } diff --git a/platform/lang-impl/src/com/intellij/codeInsight/template/impl/TemplateManagerImpl.java b/platform/lang-impl/src/com/intellij/codeInsight/template/impl/TemplateManagerImpl.java index 0bd9f9f91392..f88ab4928dec 100644 --- a/platform/lang-impl/src/com/intellij/codeInsight/template/impl/TemplateManagerImpl.java +++ b/platform/lang-impl/src/com/intellij/codeInsight/template/impl/TemplateManagerImpl.java @@ -204,7 +204,7 @@ public final class TemplateManagerImpl extends TemplateManager implements Dispos return !Character.isJavaIdentifierPart(c); } - private static void addToMap(@NotNull Map map, @NotNull Collection keys, U value) { + private static void addToMap(@NotNull Map map, @NotNull Collection keys, U value) { for (T key : keys) { map.put(key, value); }