Bug 576333 - [performance] optimize EclipseContext.localValues
EclipseContext.isSetLocally is a hotspot during open java editor.
Avoid synchronizedMap by using a ConcurrentHashMap for
EclipseContext.localValues.
Since ConcurrentHashMap does not allow null values it is wrapped. Null
values are substituted by a neutral value.
Speeds up EclipseContext.localValues by factor ~ 4.
Change-Id: Ie79da1a9cfc9b4d7b8bce8a4d590cbc5329183ec
Signed-off-by: Joerg Kubitz <jkubitz-eclipse@gmx.de>
Reviewed-on: https://git.eclipse.org/r/c/platform/eclipse.platform.runtime/+/185955
Tested-by: Platform Bot <platform-bot@eclipse.org>
diff --git a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ConcurrentNeutralValueMap.java b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ConcurrentNeutralValueMap.java
new file mode 100644
index 0000000..f588fac
--- /dev/null
+++ b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/ConcurrentNeutralValueMap.java
@@ -0,0 +1,221 @@
+/*******************************************************************************
+ * Copyright (c) 2021 Joerg Kubitz.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ * Joerg Kubitz - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.e4.core.internal.contexts;
+
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.function.BiConsumer;
+
+/**
+ * A wrapper around ConcurrentHashMap which allows null values.
+ *
+ * For null values a neutral Element is internally used.
+ *
+ * Faster then a synchronized Map.
+ */
+public class ConcurrentNeutralValueMap<K, V> {// implements subset of Map<K, V>
+ final ConcurrentHashMap<K, V> delegate = new ConcurrentHashMap<>();
+ /** a value that is used for null elements **/
+ final private V neutralValue;
+
+ /**
+ * @param neutralValue a Element that does not equal any other Value
+ * @see #neutralObject()
+ */
+ public ConcurrentNeutralValueMap(V neutralValue) {
+ this.neutralValue = neutralValue;
+ }
+
+ private V wrapValue(V value) {
+ return value == null ? neutralValue : value;
+ }
+
+ private V unwrapValue(V v) {
+ return v == neutralValue ? null : v;
+ }
+
+ public V get(Object key) {
+ return unwrapValue(delegate.get(key));
+ }
+
+ private static final class NullValue {
+ @Override
+ public String toString() {
+ return "null"; //$NON-NLS-1$
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ return false;
+ }
+
+ @Override
+ public int hashCode() {
+ return 0x55555555;
+ }
+ }
+
+ /** returns a neutral Element that does not equals any other **/
+ public static Object neutralObject() {
+ return new NullValue();
+ }
+
+ /*
+ * Method overloads from java.lang.Object:
+ */
+
+ @Override
+ public int hashCode() {
+ return delegate.hashCode();
+ }
+
+ @Override
+ public boolean equals(Object obj) {
+ return delegate.equals(obj);
+ }
+
+ @Override
+ public String toString() {
+ return delegate.toString();
+ }
+
+ /*
+ * Method overloads from java.util.Map:
+ */
+
+ /**
+ * Like {@link java.util.Map#put} but we do not return a value.
+ *
+ * @see java.util.Map#put(Object, Object)
+ * @see ConcurrentHashMap#put(Object, Object)
+ * @see #putAndGetOld(Object, Object)
+ **/
+ public void put(K key, V value) {
+ delegate.put(key, wrapValue(value));
+ }
+
+ /**
+ * Like {@link java.util.Map#put} but we do not return a wrapped value.
+ *
+ * @see java.util.Map#put(Object, Object)
+ * @see Value
+ **/
+ public Value<V> putAndGetOld(K key, V value) {
+ return new Wrapped(delegate.put(key, wrapValue(value)));
+ }
+
+ /**
+ * @see java.util.Map#remove(Object)
+ * @see ConcurrentHashMap#remove(Object)
+ **/
+ public V remove(Object key) {
+ return unwrapValue(delegate.remove(key));
+ }
+
+ /**
+ * @see java.util.Map#size()
+ **/
+ public int size() {
+ return delegate.size();
+ }
+
+ /**
+ * @see java.util.Map#isEmpty()
+ **/
+ public boolean isEmpty() {
+ return delegate.isEmpty();
+ }
+
+ /**
+ * @see java.util.Map#clear()
+ **/
+ public void clear() {
+ delegate.clear();
+ }
+
+ /**
+ * Use is discouraged as the outcome might change concurrently
+ *
+ * @see java.util.Map#containsKey(Object)
+ * @see ConcurrentHashMap#containsKey(Object)
+ **/
+ public boolean containsKey(Object key) {
+ return delegate.containsKey(key);
+ }
+
+ /*
+ * Some helpful methods from ConcurrentHashMap which do not return V:
+ */
+
+ /** @see ConcurrentHashMap#forEach(BiConsumer) **/
+ public void forEach(BiConsumer<? super K, ? super V> action) {
+ delegate.forEach( //
+ (k, v) -> action.accept(k, unwrapValue(v) //
+ ));
+ }
+
+ /*
+ * Some methods from ConcurrentHashMap which return V. returning null is
+ * discouraged because it is not clear if that was a value or not:
+ */
+
+ /**
+ * Like {@link ConcurrentHashMap#putIfAbsent(Object, Object)} but we do not
+ * return anything.
+ *
+ * @see ConcurrentHashMap#putIfAbsent(Object, Object)
+ **/
+ public void putIfAbsent(K key, V value) {
+ delegate.putIfAbsent(key, wrapValue(value));
+ }
+
+ public interface Value<V> {
+ /**
+ * @return the result of map.contains(key)
+ */
+ boolean isPresent();
+
+ /**
+ * @return the result of map.get(key)
+ */
+ V unwraped();
+ }
+
+ private final class Wrapped implements Value<V> {
+ V wrappedValue;
+
+ private Wrapped(V wrappedValue) {
+ this.wrappedValue = wrappedValue;
+ }
+
+ @Override
+ public boolean isPresent() {
+ return wrappedValue != null;
+ }
+
+ @Override
+ public V unwraped() {
+ return unwrapValue(wrappedValue);
+ }
+ }
+
+ /**
+ * @return a value that is wrapped into a Value Object.
+ * @see Value#isPresent()
+ * @see Value#unwraped()
+ */
+ public Value<V> getValue(K key) {
+ return new Wrapped(delegate.get(key));
+ }
+
+}
diff --git a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java
index 5a0f644..6b92681 100644
--- a/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java
+++ b/bundles/org.eclipse.e4.core.contexts/src/org/eclipse/e4/core/internal/contexts/EclipseContext.java
@@ -33,6 +33,7 @@
import org.eclipse.e4.core.contexts.IEclipseContext;
import org.eclipse.e4.core.contexts.RunAndTrack;
import org.eclipse.e4.core.di.IInjector;
+import org.eclipse.e4.core.internal.contexts.ConcurrentNeutralValueMap.Value;
import org.eclipse.e4.core.internal.contexts.osgi.ContextDebugHelper;
import org.osgi.service.event.Event;
import org.osgi.service.event.EventAdmin;
@@ -91,7 +92,8 @@
private WeakGroupedListenerList weakListeners = new WeakGroupedListenerList();
private Map<String, ValueComputation> localValueComputations = Collections.synchronizedMap(new HashMap<>());
- final protected Map<String, Object> localValues = Collections.synchronizedMap(new HashMap<>());
+ final protected ConcurrentNeutralValueMap<String, Object> localValues = // null values allowed
+ new ConcurrentNeutralValueMap<>(ConcurrentNeutralValueMap.neutralObject());
private Set<String> modifiable;
@@ -256,8 +258,9 @@
Object result = null;
// 1. try for local value
- if (localValues.containsKey(name)) {
- result = localValues.get(name);
+ Value<Object> value = localValues.getValue(name);
+ if (value.isPresent()) {
+ result = value.unwraped();
if (result == null)
return null;
} else
@@ -320,9 +323,10 @@
}
protected boolean isLocalEquals(String name, Object newValue) {
- if (!localValues.containsKey(name))
+ Value<Object> value = localValues.getValue(name);
+ if (!value.isPresent())
return false;
- return (localValues.get(name) == newValue);
+ return (value.unwraped() == newValue);
}
private boolean isSetLocally(String name) {
@@ -371,8 +375,9 @@
setParent((IEclipseContext) value);
return;
}
- boolean containsKey = localValues.containsKey(name);
- Object oldValue = localValues.put(name, value);
+ Value<Object> old = localValues.putAndGetOld(name, value);
+ boolean containsKey = old.isPresent();
+ Object oldValue = old.unwraped();
if (!containsKey || oldValue != value) {
Set<Scheduled> scheduled = new LinkedHashSet<>();
invalidate(name, ContextChangeEvent.ADDED, oldValue, value, scheduled);
@@ -407,7 +412,7 @@
String tmp = "Variable " + name + " is not modifiable in the context " + this; //$NON-NLS-1$ //$NON-NLS-2$
throw new IllegalArgumentException(tmp);
}
- Object oldValue = localValues.put(name, value);
+ Object oldValue = localValues.putAndGetOld(name, value).unwraped();
if (oldValue != value)
invalidate(name, ContextChangeEvent.ADDED, oldValue, value, scheduled);
return true;
@@ -470,9 +475,7 @@
if (modifiable == null)
modifiable = new HashSet<>(3);
modifiable.add(name);
- if (localValues.containsKey(name))
- return;
- localValues.put(name, null);
+ localValues.putIfAbsent(name, null);
}
private boolean checkModifiable(String name) {
@@ -709,23 +712,22 @@
// This method is for debug only, do not use externally
public Map<String, Object> localData() {
Map<String, Object> result = new HashMap<>(localValues.size());
- for (Map.Entry<String, Object> entry : localValues.entrySet()) {
- if (entry.getValue() instanceof IContextFunction) {
- continue;
+ localValues.forEach((k, v) -> {
+ if (!(v instanceof IContextFunction)) {
+ result.put(k, v);
}
- result.put(entry.getKey(), entry.getValue());
- }
+ });
return result;
}
// This method is for debug only, do not use externally
public Map<String, Object> localContextFunction() {
Map<String, Object> result = new HashMap<>(localValues.size());
- for (Map.Entry<String, Object> entry : localValues.entrySet()) {
- if (entry.getValue() instanceof IContextFunction) {
- result.put(entry.getKey(), entry.getValue());
+ localValues.forEach((k, v) -> {
+ if (v instanceof IContextFunction) {
+ result.put(k, v);
}
- }
+ });
return result;
}
diff --git a/tests/org.eclipse.e4.core.tests/META-INF/MANIFEST.MF b/tests/org.eclipse.e4.core.tests/META-INF/MANIFEST.MF
index 3aa9bf4..c9f8b95 100644
--- a/tests/org.eclipse.e4.core.tests/META-INF/MANIFEST.MF
+++ b/tests/org.eclipse.e4.core.tests/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@
Bundle-ManifestVersion: 2
Bundle-Name: E4 Core Tests
Bundle-SymbolicName: org.eclipse.e4.core.tests
-Bundle-Version: 1.2.200.qualifier
+Bundle-Version: 1.2.300.qualifier
Bundle-Vendor: Eclipse.org
Bundle-Activator: org.eclipse.e4.core.internal.tests.CoreTestsActivator
Require-Bundle: org.eclipse.osgi;bundle-version="3.6.0",
diff --git a/tests/org.eclipse.e4.core.tests/pom.xml b/tests/org.eclipse.e4.core.tests/pom.xml
index 7ec41c2..cc0712d 100644
--- a/tests/org.eclipse.e4.core.tests/pom.xml
+++ b/tests/org.eclipse.e4.core.tests/pom.xml
@@ -19,7 +19,7 @@
</parent>
<groupId>org.eclipse.e4</groupId>
<artifactId>org.eclipse.e4.core.tests</artifactId>
- <version>1.2.200-SNAPSHOT</version>
+ <version>1.2.300-SNAPSHOT</version>
<packaging>eclipse-test-plugin</packaging>
<properties>
diff --git a/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/contexts/NeutralValueTest.java b/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/contexts/NeutralValueTest.java
new file mode 100644
index 0000000..dccb2c3
--- /dev/null
+++ b/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/internal/tests/contexts/NeutralValueTest.java
@@ -0,0 +1,144 @@
+/*******************************************************************************
+ * Copyright (c) 2021 Joerg Kubitz.
+ *
+ * This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License 2.0
+ * which accompanies this distribution, and is available at
+ * https://www.eclipse.org/legal/epl-2.0/
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ *
+ * Contributors:
+ * Joerg Kubitz - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.e4.core.internal.tests.contexts;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertTrue;
+
+import java.util.HashSet;
+import java.util.Set;
+
+import org.eclipse.e4.core.internal.contexts.ConcurrentNeutralValueMap;
+import org.eclipse.e4.core.internal.contexts.ConcurrentNeutralValueMap.Value;
+import org.junit.Test;
+
+public class NeutralValueTest {
+
+ @Test
+ public void testNeutralObject() {
+ assertFalse(new Object().equals(ConcurrentNeutralValueMap.neutralObject()));
+ assertFalse(ConcurrentNeutralValueMap.neutralObject().equals(new Object()));
+ assertNotEquals(ConcurrentNeutralValueMap.neutralObject(), ConcurrentNeutralValueMap.neutralObject());
+ assertEquals("" + null, ConcurrentNeutralValueMap.neutralObject().toString());
+ }
+
+ @Test
+ public void testConcurrentNeutralValueMap() {
+ ConcurrentNeutralValueMap<String, Double> map = new ConcurrentNeutralValueMap<>(Double.NaN);
+ map.put("nix", null); // modify
+ map.put("2", 2.0); // modify
+ map.put("3", 3.0); // modify
+ map.put("4", 4.0); // modify
+ map.put("garnix", null); // modify
+ assertTrue(map.containsKey("garnix"));
+ map.remove("garnix");
+
+ assertTrue(map.containsKey("nix"));
+ assertTrue(map.containsKey("2"));
+ assertFalse(map.containsKey("1"));
+ assertFalse(map.containsKey("garnix"));
+
+ assertEquals(4, map.size());
+
+ assertFalse(map.isEmpty());
+
+ assertEquals(null, map.get("nix"));
+ assertEquals(null, map.get("1"));
+ assertEquals(Double.valueOf(2.0), map.get("2"));
+ assertEquals(Double.valueOf(3.0), map.get("3"));
+ assertEquals(Double.valueOf(4.0), map.get("4"));
+
+ Set<String> keys = new HashSet<>();
+ Set<Double> values = new HashSet<>();
+ map.forEach((k, v) -> keys.add(k));
+ map.forEach((k, v) -> values.add(v));
+ assertEquals(Set.of("nix", "2", "3", "4"), keys);
+ assertTrue(values.contains(null));
+ assertTrue(values.contains(2.0));
+
+ assertTrue(map.getValue("nix").isPresent());
+ assertFalse(map.getValue("1").isPresent());
+ assertTrue(map.getValue("2").isPresent());
+ assertTrue(map.getValue("3").isPresent());
+ assertTrue(map.getValue("4").isPresent());
+
+ {
+ Value<Double> v = map.getValue("nix");
+ assertTrue(v.isPresent());
+ assertEquals(null, v.unwraped());
+ }
+ {
+ Value<Double> v = map.getValue("1");
+ assertFalse(v.isPresent());
+ assertEquals(null, v.unwraped());
+ }
+ {
+ Value<Double> v = map.getValue("2");
+ assertTrue(v.isPresent());
+ assertEquals(Double.valueOf(2.0), v.unwraped());
+ }
+
+ {
+ Value<Double> v = map.putAndGetOld("5", 5555.0); // modify
+ assertFalse(v.isPresent());
+ assertEquals(null, v.unwraped());
+ assertEquals(Double.valueOf(5555.0), map.get("5"));
+ }
+ {
+ Value<Double> v = map.putAndGetOld("5", 5.0); // modify
+ assertTrue(v.isPresent());
+ assertEquals(Double.valueOf(5555.0), v.unwraped());
+ assertEquals(Double.valueOf(5.0), map.get("5"));
+ }
+ map.putIfAbsent("5", 5555.0); // modify
+ assertEquals(Double.valueOf(5.0), map.get("5"));
+ map.remove("5"); // modify
+ assertFalse(map.containsKey("5"));
+
+ {
+ Value<Double> v = map.putAndGetOld("five", null); // modify
+ assertFalse(v.isPresent());
+ assertEquals(null, v.unwraped());
+ assertEquals(null, map.get("five"));
+ }
+ {
+ Value<Double> v = map.putAndGetOld("five", 5.0); // modify
+ assertTrue(v.isPresent());
+ assertEquals(null, v.unwraped());
+ assertEquals(Double.valueOf(5.0), map.get("five"));
+ }
+ {
+ map.putIfAbsent("five", null); // modify
+ Value<Double> v = map.getValue("five");
+ assertTrue(v.isPresent());
+ assertEquals(Double.valueOf(5.0), v.unwraped());
+ }
+ map.remove("five"); // modify
+ {
+ map.putIfAbsent("five", null); // modify
+ Value<Double> v = map.getValue("five");
+ assertTrue(v.isPresent());
+ assertEquals(null, v.unwraped());
+ }
+ map.remove("five"); // modify
+ assertFalse(map.containsKey("five"));
+
+ map.clear(); // modify
+ assertEquals(0, map.size());
+ assertTrue(map.isEmpty());
+ }
+
+}
diff --git a/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/tests/CoreTestSuite.java b/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/tests/CoreTestSuite.java
index 1fe12e8..69a8f39 100644
--- a/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/tests/CoreTestSuite.java
+++ b/tests/org.eclipse.e4.core.tests/src/org/eclipse/e4/core/tests/CoreTestSuite.java
@@ -22,6 +22,7 @@
import org.eclipse.e4.core.internal.tests.contexts.ContextDynamicTest;
import org.eclipse.e4.core.internal.tests.contexts.DependenciesLeakTest;
import org.eclipse.e4.core.internal.tests.contexts.EclipseContextTest;
+import org.eclipse.e4.core.internal.tests.contexts.NeutralValueTest;
import org.eclipse.e4.core.internal.tests.contexts.ReparentingTest;
import org.eclipse.e4.core.internal.tests.contexts.RunAndTrackTest;
import org.eclipse.e4.core.internal.tests.contexts.inject.ActivationInjectionTest;
@@ -102,6 +103,7 @@
DisposeClassLinkTest.class,
InjectStaticContextTest.class,
ActivationTest.class,
+ NeutralValueTest.class,
// Contexts injection
AnnotationsInjectionTest.class,
