Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorThomas Watson2019-09-05 16:24:58 +0000
committerThomas Watson2019-09-10 16:02:07 +0000
commitd2ee5fa2383547043bed51ebf81c2313cd939db5 (patch)
tree0284c010892340f623e764eaf3d383d6f2e9a914 /bundles
parent63a9e1075ab029c5090a3d50cf52b82f052c62f6 (diff)
downloadrt.equinox.framework-d2ee5fa2383547043bed51ebf81c2313cd939db5.tar.gz
rt.equinox.framework-d2ee5fa2383547043bed51ebf81c2313cd939db5.tar.xz
rt.equinox.framework-d2ee5fa2383547043bed51ebf81c2313cd939db5.zip
Bug 550645 - Performance improvements to CaseInsensitiveDictionaryMap
Improve hashCode calculation to avoid Character.toUpper/LowerCase calls for ASCII chars and make hashCode final for CaseInsensitiveKey Improve put operations by not interning the key on every put. It is now up to the client to intern if necessary. Updated ManifestElement to do that for bundle manifest keys while parsing the bundle manifest. Also improved performance for keys that are not already present in the Map by avoiding a call to remove first. With these optimizations put together we should see ~4x perforamance improvement in the usage of the CaseInsensitiveDictionaryMap by the framework. Change-Id: Ibe21780b2be95cecab994fa2ca2e817d7bab112c Signed-off-by: Thomas Watson <tjwatson@us.ibm.com>
Diffstat (limited to 'bundles')
-rw-r--r--bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/perf/CaseMapPerformanceTest.java96
-rw-r--r--bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/util/AllTests.java1
-rw-r--r--bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/util/TestCaseinsensitiveMap.java109
-rw-r--r--bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/framework/util/CaseInsensitiveDictionaryMap.java51
-rw-r--r--bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/util/ManifestElement.java3
5 files changed, 239 insertions, 21 deletions
diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/perf/CaseMapPerformanceTest.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/perf/CaseMapPerformanceTest.java
index 35d9bdb50..20bc21e79 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/perf/CaseMapPerformanceTest.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/perf/CaseMapPerformanceTest.java
@@ -13,6 +13,7 @@
*******************************************************************************/
package org.eclipse.osgi.tests.perf;
+import java.util.HashMap;
import java.util.Map;
import junit.framework.Test;
import junit.framework.TestSuite;
@@ -20,6 +21,8 @@ import org.eclipse.core.tests.harness.PerformanceTestRunner;
import org.eclipse.osgi.framework.util.CaseInsensitiveDictionaryMap;
import org.eclipse.osgi.framework.util.Headers;
import org.eclipse.osgi.tests.OSGiTest;
+import org.junit.Assert;
+import org.osgi.framework.Constants;
public class CaseMapPerformanceTest extends OSGiTest {
static final String[] KEYS;
@@ -89,9 +92,9 @@ public class CaseMapPerformanceTest extends OSGiTest {
doTestMap(headers, 10);
}
- public void testXCaseMap020() {
- final Map<String, Object> headers = new CaseInsensitiveDictionaryMap<String, Object>(20);
- doTestMap(headers, 10);
+ public void testXCaseMap034() {
+ final Map<String, Object> headers = new CaseInsensitiveDictionaryMap<String, Object>(34);
+ doTestMap(headers, 34);
}
public void testXCaseMap100() {
@@ -100,15 +103,37 @@ public class CaseMapPerformanceTest extends OSGiTest {
}
private void doTestMap(final Map<String, Object> map, final int numKeys) {
- fillMap(map, numKeys);
new PerformanceTestRunner() {
protected void test() {
+ fillMap(map, numKeys);
doMapGet(map, numKeys);
}
}.run(this, 10, 10000);
}
+ public void testCommonKeyMap() {
+ final Map<String, Object> map = new CaseInsensitiveDictionaryMap<>(34);
+ new PerformanceTestRunner() {
+ protected void test() {
+ fillCommonKeyMap(map);
+ doCommonKeyMapGet(map);
+ }
+
+ }.run(this, 10, 10000);
+ }
+
+ public void testCommonHashMap() {
+ final Map<String, Object> map = new HashMap<>(34);
+ new PerformanceTestRunner() {
+ protected void test() {
+ fillCommonKeyMap(map);
+ doCommonKeyMapGet(map);
+ }
+
+ }.run(this, 10, 10000);
+ }
+
static void fillMap(Map<String, Object> map, int numKeys) {
map.clear();
for (int i = 0; i < numKeys; i++) {
@@ -116,10 +141,71 @@ public class CaseMapPerformanceTest extends OSGiTest {
}
}
+ static void fillCommonKeyMap(Map<String, Object> map) {
+ map.clear();
+ for (String key : COMMON_KEY_NAMES) {
+ map.put(key, VALUE);
+ }
+ }
+
static void doMapGet(Map<String, Object> map, int numKeys) {
for (int i = 0; i < numKeys; i++) {
- map.get(KEYS[i]);
+ Assert.assertEquals("Wrong value found.", VALUE, map.get(KEYS[i]));
+ }
+ }
+
+ static void doCommonKeyMapGet(Map<String, Object> map) {
+ for (String key : COMMON_KEY_NAMES) {
+ Assert.assertEquals("Wrong value found.", VALUE, map.get(key));
}
}
+ final static String[] COMMON_KEY_NAMES = new String[] {
+
+ // common core service property keys
+ Constants.OBJECTCLASS, //
+ Constants.SERVICE_BUNDLEID, //
+ Constants.SERVICE_CHANGECOUNT, //
+ Constants.SERVICE_DESCRIPTION, //
+ Constants.SERVICE_ID, //
+ Constants.SERVICE_PID, //
+ Constants.SERVICE_RANKING, //
+ Constants.SERVICE_SCOPE, //
+ Constants.SERVICE_VENDOR, //
+
+ // common SCR service property keys
+ "component.name", //$NON-NLS-1$
+ "component.id", //$NON-NLS-1$
+
+ // common meta-type property keys
+ "metatype.pid", //$NON-NLS-1$
+ "metatype.factory.pid", //$NON-NLS-1$
+
+ // event admin keys
+ "event.topics", //$NON-NLS-1$
+ "event.filter", //$NON-NLS-1$
+
+ // jmx keys
+ "jmx.objectname", //$NON-NLS-1$
+
+ // common bundle manifest headers
+ Constants.BUNDLE_ACTIVATIONPOLICY, //
+ Constants.BUNDLE_ACTIVATOR, //
+ Constants.BUNDLE_CLASSPATH, //
+ Constants.BUNDLE_LICENSE, //
+ Constants.BUNDLE_LOCALIZATION, //
+ Constants.BUNDLE_MANIFESTVERSION, //
+ Constants.BUNDLE_NAME, //
+ Constants.BUNDLE_NATIVECODE, //
+ Constants.BUNDLE_REQUIREDEXECUTIONENVIRONMENT, //
+ Constants.BUNDLE_SCM, //
+ Constants.BUNDLE_SYMBOLICNAME, //
+ Constants.BUNDLE_VENDOR, //
+ Constants.BUNDLE_VERSION, //
+ Constants.EXPORT_PACKAGE, //
+ Constants.FRAGMENT_HOST, //
+ Constants.IMPORT_PACKAGE, //
+ Constants.REQUIRE_BUNDLE, //
+ Constants.REQUIRE_CAPABILITY //
+ };
}
diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/util/AllTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/util/AllTests.java
index a08745c8e..d86b15c06 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/util/AllTests.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/util/AllTests.java
@@ -25,6 +25,7 @@ public class AllTests extends TestSuite {
}
public AllTests() {
+ addTest(new TestSuite(TestCaseinsensitiveMap.class));
addTest(new TestSuite(ObjectPoolTestCase.class));
addTest(new TestSuite(ManifestElementTestCase.class));
addTest(new TestSuite(NLSTestCase.class));
diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/util/TestCaseinsensitiveMap.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/util/TestCaseinsensitiveMap.java
new file mode 100644
index 000000000..dd8730942
--- /dev/null
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/util/TestCaseinsensitiveMap.java
@@ -0,0 +1,109 @@
+package org.eclipse.osgi.tests.util;
+
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+import org.eclipse.core.tests.harness.CoreTest;
+import org.eclipse.osgi.framework.util.CaseInsensitiveDictionaryMap;
+import org.osgi.framework.Constants;
+
+public class TestCaseinsensitiveMap extends CoreTest {
+ @SuppressWarnings("deprecation")
+ static String[] COMMON_KEY_NAMES = new String[] { //
+
+ // common core service property keys
+ Constants.OBJECTCLASS, //
+ Constants.SERVICE_BUNDLEID, //
+ Constants.SERVICE_CHANGECOUNT, //
+ Constants.SERVICE_DESCRIPTION, //
+ Constants.SERVICE_ID, //
+ Constants.SERVICE_PID, //
+ Constants.SERVICE_RANKING, //
+ Constants.SERVICE_SCOPE, //
+ Constants.SERVICE_VENDOR, //
+
+ // common SCR service property keys
+ "component.name", //$NON-NLS-1$
+ "component.id", //$NON-NLS-1$
+
+ // common meta-type property keys
+ "metatype.pid", //$NON-NLS-1$
+ "metatype.factory.pid", //$NON-NLS-1$
+
+ // common event admin keys
+ "event.topics", //$NON-NLS-1$
+ "event.filter", //$NON-NLS-1$
+
+ // jmx keys
+ "jmx.objectname", //$NON-NLS-1$
+
+ // common bundle manifest headers
+ Constants.BUNDLE_ACTIVATIONPOLICY, //
+ Constants.BUNDLE_ACTIVATOR, //
+ Constants.BUNDLE_CLASSPATH, //
+ Constants.BUNDLE_LICENSE, //
+ Constants.BUNDLE_LOCALIZATION, //
+ Constants.BUNDLE_MANIFESTVERSION, //
+ Constants.BUNDLE_NAME, //
+ Constants.BUNDLE_NATIVECODE, //
+ Constants.BUNDLE_REQUIREDEXECUTIONENVIRONMENT, //
+ Constants.BUNDLE_SCM, //
+ Constants.BUNDLE_SYMBOLICNAME, //
+ Constants.BUNDLE_VENDOR, //
+ Constants.BUNDLE_VERSION, //
+ Constants.EXPORT_PACKAGE, //
+ Constants.FRAGMENT_HOST, //
+ Constants.IMPORT_PACKAGE, //
+ Constants.REQUIRE_BUNDLE, //
+ Constants.REQUIRE_CAPABILITY //
+ };
+
+ String[] OTHER_KEY_NAMES = new String[] {"test.key0", //
+ "test.key1", //
+ "test.key2", //
+ "test.key3", //
+ "test.key4", //
+ "test.key5", //
+ "test.key6", //
+ "test.key7", //
+ "test.key8", //
+ "test.key9" //
+ };
+
+ private static final String VALUE1 = "test value1";
+ private static final String VALUE2 = "test value2";
+
+ public void testCommonKeys() {
+ testKeys(COMMON_KEY_NAMES);
+ }
+
+ public void testOtherKeys() {
+ testKeys(OTHER_KEY_NAMES);
+ }
+
+ private void testKeys(String[] keys) {
+ Map<String, Object> testMap = new CaseInsensitiveDictionaryMap<>();
+ // first put a value in for all common keys
+ for (String key : keys) {
+ testMap.put(key, VALUE1);
+ assertEquals("Wrong value found.", VALUE1, testMap.get(key));
+ }
+
+ Set<String> upperKeys = new HashSet<>();
+ for (String key : keys) {
+ // now upper case all keys
+ String upperKey = key.toUpperCase();
+ upperKeys.add(upperKey);
+ assertEquals("Wrong value found.", VALUE1, testMap.get(upperKey));
+ // replace with value2 using upper case
+ assertEquals("Wrong value found.", VALUE1, testMap.put(upperKey, VALUE2));
+ // both original key and upper case key should give same value
+ assertEquals("Wrong value found.", VALUE2, testMap.get(key));
+ assertEquals("Wrong value found.", VALUE2, testMap.get(upperKey));
+ }
+
+ Set<String> currentKeys = testMap.keySet();
+ assertEquals("Wrong number of keys.", upperKeys.size(), currentKeys.size());
+ assertTrue("Wrong keys found: " + currentKeys, upperKeys.containsAll(upperKeys));
+ }
+}
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/framework/util/CaseInsensitiveDictionaryMap.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/framework/util/CaseInsensitiveDictionaryMap.java
index 73086a5d3..7f013b276 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/framework/util/CaseInsensitiveDictionaryMap.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/framework/util/CaseInsensitiveDictionaryMap.java
@@ -16,7 +16,16 @@ package org.eclipse.osgi.framework.util;
import static java.util.Objects.requireNonNull;
-import java.util.*;
+import java.util.AbstractSet;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Dictionary;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
import org.eclipse.osgi.internal.messages.Msg;
import org.eclipse.osgi.util.NLS;
@@ -178,10 +187,14 @@ public class CaseInsensitiveDictionaryMap<K, V> extends Dictionary<K, V> impleme
public V put(K key, V value) {
requireNonNull(value);
if (key instanceof String) {
- Object wrappedKey = keyWrap(((String) key).intern());
- V previous = map.remove(wrappedKey); // remove so we put key into map
- map.put(wrappedKey, value);
- return previous;
+ Object wrappedKey = keyWrap(key);
+ V existing = map.put(wrappedKey, value);
+ if (existing != null) {
+ // must remove to replace key if case has changed
+ map.remove(wrappedKey);
+ map.put(wrappedKey, value);
+ }
+ return existing;
}
return map.put(requireNonNull(key), value);
}
@@ -388,25 +401,33 @@ public class CaseInsensitiveDictionaryMap<K, V> extends Dictionary<K, V> impleme
}
}
+ static int computeHashCode(String key) {
+ int h = 1;
+ for (char c : key.toCharArray()) {
+ if (c < 0x80) { // ASCII
+ if (c >= 'A' && c <= 'Z') {
+ c += 'a' - 'A'; // convert to ASCII lowercase
+ }
+ } else {
+ c = Character.toLowerCase(Character.toUpperCase(c));
+ }
+ h = 31 * h + c;
+ }
+ return h;
+ }
+
private static final class CaseInsensitiveKey {
final String key;
- private transient int hashCode;
+ final private int hashCode;
CaseInsensitiveKey(String key) {
this.key = key;
+ this.hashCode = computeHashCode(key);
}
@Override
public int hashCode() {
- int h = hashCode;
- if (h != 0) {
- return h;
- }
- h = 1;
- for (char c : key.toCharArray()) {
- h = 31 * h + Character.toLowerCase(Character.toUpperCase(c));
- }
- return hashCode = h;
+ return hashCode;
}
@Override
diff --git a/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/util/ManifestElement.java b/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/util/ManifestElement.java
index 51cd39f78..93af0cf18 100644
--- a/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/util/ManifestElement.java
+++ b/bundles/org.eclipse.osgi/supplement/src/org/eclipse/osgi/util/ManifestElement.java
@@ -524,7 +524,8 @@ public class ManifestElement {
}
String header = line.substring(0, colon).trim();
String value = line.substring(colon + 1).trim();
- headers.put(header, value);
+ // intern the header here because they likely have constants for them anyway
+ headers.put(header.intern(), value);
}
} finally {
try {

Back to the top