camel-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From davscl...@apache.org
Subject [2/4] camel git commit: CAMEL-8262: Optimize CaseInsensitiveMap to use TreeMap instead of custom HashMap with 2x maps. This reduces memory overhead and lower casing all keys costs cpu time. As the number of entries in the map is low then the TreeMap stru
Date Thu, 22 Jan 2015 14:55:33 GMT
CAMEL-8262: Optimize CaseInsensitiveMap to use TreeMap instead of custom HashMap with 2x maps.
This reduces memory overhead and lower casing all keys costs cpu time. As the number of entries
in the map is low then the TreeMap structure should be fast enough


Project: http://git-wip-us.apache.org/repos/asf/camel/repo
Commit: http://git-wip-us.apache.org/repos/asf/camel/commit/5ae518b7
Tree: http://git-wip-us.apache.org/repos/asf/camel/tree/5ae518b7
Diff: http://git-wip-us.apache.org/repos/asf/camel/diff/5ae518b7

Branch: refs/heads/master
Commit: 5ae518b7daf2c1213678b9f385632bb72f5fc9a5
Parents: 1cf8f30
Author: Claus Ibsen <davsclaus@apache.org>
Authored: Thu Jan 22 13:39:47 2015 +0100
Committer: Claus Ibsen <davsclaus@apache.org>
Committed: Thu Jan 22 13:41:52 2015 +0100

----------------------------------------------------------------------
 .../org/apache/camel/impl/DefaultMessage.java   |  11 +-
 .../apache/camel/util/CaseInsensitiveMap.java   | 167 +------------------
 .../camel/impl/DefaultMessageHeaderTest.java    |  17 --
 .../camel/util/CaseInsensitiveMapTest.java      |   8 +-
 .../camel/component/cometd/CometdBinding.java   |   7 +-
 5 files changed, 28 insertions(+), 182 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/camel/blob/5ae518b7/camel-core/src/main/java/org/apache/camel/impl/DefaultMessage.java
----------------------------------------------------------------------
diff --git a/camel-core/src/main/java/org/apache/camel/impl/DefaultMessage.java b/camel-core/src/main/java/org/apache/camel/impl/DefaultMessage.java
index ebc8c25..24ba1ec 100644
--- a/camel-core/src/main/java/org/apache/camel/impl/DefaultMessage.java
+++ b/camel-core/src/main/java/org/apache/camel/impl/DefaultMessage.java
@@ -16,6 +16,7 @@
  */
 package org.apache.camel.impl;
 
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Set;
@@ -141,6 +142,9 @@ public class DefaultMessage extends MessageSupport {
         }
 
         boolean matches = false;
+        // must use a set to store the keys to remove as we cannot walk using entrySet and
remove at the same time
+        // due concurrent modification error
+        Set<String> toRemove = new HashSet<String>();
         for (Map.Entry<String, Object> entry : headers.entrySet()) {
             String key = entry.getKey();
             if (EndpointHelper.matchPattern(key, pattern)) {
@@ -148,10 +152,13 @@ public class DefaultMessage extends MessageSupport {
                     continue;
                 }
                 matches = true;
-                headers.remove(entry.getKey());
+                toRemove.add(entry.getKey());
             }
-
         }
+        for (String key : toRemove) {
+            headers.remove(key);
+        }
+
         return matches;
     }
 

http://git-wip-us.apache.org/repos/asf/camel/blob/5ae518b7/camel-core/src/main/java/org/apache/camel/util/CaseInsensitiveMap.java
----------------------------------------------------------------------
diff --git a/camel-core/src/main/java/org/apache/camel/util/CaseInsensitiveMap.java b/camel-core/src/main/java/org/apache/camel/util/CaseInsensitiveMap.java
index 0ed2245..ef6749c 100644
--- a/camel-core/src/main/java/org/apache/camel/util/CaseInsensitiveMap.java
+++ b/camel-core/src/main/java/org/apache/camel/util/CaseInsensitiveMap.java
@@ -16,182 +16,31 @@
  */
 package org.apache.camel.util;
 
-import java.util.AbstractSet;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.Locale;
 import java.util.Map;
-import java.util.Set;
+import java.util.TreeMap;
 
 /**
- * A map that uses case insensitive keys, but preserves the original keys in the keySet.
+ * A map that uses case insensitive keys, but preserves the original key cases.
  * <p/>
- * This map allows you to do lookup using case insensitive keys so you can retrieve the value
without worrying about
- * whether some transport protocol affects the keys such as Http and Mail protocols can do.
- * <p/>
- * When copying from this map to a regular Map such as {@link java.util.HashMap} then the
original keys are
- * copied over and you get the old behavior back using a regular Map with case sensitive
keys.
+ * The map is based on {@link TreeMap} and therefore uses O(n) for lookup and not O(1) as
a {@link java.util.HashMap} does.
  * <p/>
  * This map is <b>not</b> designed to be thread safe as concurrent access to
it is not supposed to be performed
  * by the Camel routing engine.
  *
  * @version 
  */
-public class CaseInsensitiveMap extends HashMap<String, Object> {
-    private static final long serialVersionUID = -8538318195477618308L;
+public class CaseInsensitiveMap extends TreeMap<String, Object> {
 
-    // holds a map of lower case key -> original key
-    private Map<String, String> originalKeys = new HashMap<String, String>();
-    // holds a snapshot view of current entry set
-    private transient Set<Map.Entry<String, Object>> entrySetView;
+    private static final long serialVersionUID = -8538318195477618308L;
 
     public CaseInsensitiveMap() {
+        super(String.CASE_INSENSITIVE_ORDER);
     }
 
     public CaseInsensitiveMap(Map<? extends String, ?> map) {
+        // must use the insensitive order
+        super(String.CASE_INSENSITIVE_ORDER);
         putAll(map);
     }
 
-    public CaseInsensitiveMap(int initialCapacity, float loadFactor) {
-        super(initialCapacity, loadFactor);
-        originalKeys = new HashMap<String, String>(initialCapacity, loadFactor);
-    }
-
-    public CaseInsensitiveMap(int initialCapacity) {
-        super(initialCapacity);
-        originalKeys = new HashMap<String, String>(initialCapacity);
-    }
-
-    @Override
-    public Object get(Object key) {
-        String s = assembleKey(key);
-        Object answer = super.get(s);
-        if (answer == null) {
-            // fallback to lookup by original key
-            String originalKey = originalKeys.get(s);
-            answer = super.get(originalKey);
-        }
-        return answer;
-    }
-
-    @Override
-    public synchronized Object put(String key, Object value) {
-        // invalidate views as we mutate
-        entrySetView = null;
-        String s = assembleKey(key);
-        originalKeys.put(s, key);
-        return super.put(s, value);
-    }
-
-    @Override
-    public synchronized void putAll(Map<? extends String, ?> map) {
-        entrySetView = null;
-        if (map != null && !map.isEmpty()) {
-            for (Map.Entry<? extends String, ?> entry : map.entrySet()) {
-                String key = entry.getKey();
-                Object value = entry.getValue();
-                String s = assembleKey(key);
-                originalKeys.put(s, key);
-                super.put(s, value);
-            }
-        }
-    }
-
-    @Override
-    public synchronized Object remove(Object key) {
-        if (key == null) {
-            return null;
-        }
-
-        // invalidate views as we mutate
-        entrySetView = null;
-        String s = assembleKey(key);
-        originalKeys.remove(s);
-        return super.remove(s);
-    }
-
-    @Override
-    public synchronized void clear() {
-        // invalidate views as we mutate
-        entrySetView = null;
-        originalKeys.clear();
-        super.clear();
-    }
-
-    @Override
-    public boolean containsKey(Object key) {
-        if (key == null) {
-            return false;
-        }
-
-        String s = assembleKey(key);
-        return super.containsKey(s);
-    }
-
-    private static String assembleKey(Object key) {
-        return key.toString().toLowerCase(Locale.ENGLISH);
-    }
-
-    @Override
-    public synchronized Set<Map.Entry<String, Object>> entrySet() {
-        if (entrySetView == null) {
-            // build the key set using the original keys so we retain their case
-            // when for example we copy values to another map
-            entrySetView = new HashSet<Map.Entry<String, Object>>(this.size());
-            for (final Map.Entry<String, Object> entry : super.entrySet()) {
-                Map.Entry<String, Object> view = new Map.Entry<String, Object>()
{
-                    public String getKey() {
-                        String s = entry.getKey();
-                        // use the original key so we can preserve it
-                        return originalKeys.get(s);
-                    }
-
-                    public Object getValue() {
-                        return entry.getValue();
-                    }
-
-                    public Object setValue(Object o) {
-                        return entry.setValue(o);
-                    }
-                };
-                entrySetView.add(view);
-            }
-        }
-
-        return entrySetView;
-    }
-
-    @Override
-    public Set<String> keySet() {
-        return new CaseInsensitiveKeySet();
-    }
-
-    /**
-     * To use the original keys but support checking if a key exist using case insensitive.
-     */
-    private final class CaseInsensitiveKeySet extends AbstractSet<String> {
-
-        public Iterator<String> iterator() {
-            // use the original case keys, which is stored in as values
-            return originalKeys.values().iterator();
-        }
-
-        public int size() {
-            return CaseInsensitiveMap.this.size();
-        }
-
-        public boolean contains(Object o) {
-            return CaseInsensitiveMap.this.containsKey(o);
-        }
-
-        public boolean remove(Object o) {
-            return CaseInsensitiveMap.this.remove(o) != null;
-        }
-
-        public void clear() {
-            CaseInsensitiveMap.this.clear();
-        }
-    }
-
 }
\ No newline at end of file

http://git-wip-us.apache.org/repos/asf/camel/blob/5ae518b7/camel-core/src/test/java/org/apache/camel/impl/DefaultMessageHeaderTest.java
----------------------------------------------------------------------
diff --git a/camel-core/src/test/java/org/apache/camel/impl/DefaultMessageHeaderTest.java
b/camel-core/src/test/java/org/apache/camel/impl/DefaultMessageHeaderTest.java
index 0f31be9..ff66227 100644
--- a/camel-core/src/test/java/org/apache/camel/impl/DefaultMessageHeaderTest.java
+++ b/camel-core/src/test/java/org/apache/camel/impl/DefaultMessageHeaderTest.java
@@ -128,23 +128,6 @@ public class DefaultMessageHeaderTest extends TestCase {
         assertTrue(msg.getHeaders().isEmpty());
     }
 
-    public void testRemoveHeaderWithNullArg() {
-        Message msg = new DefaultMessage();
-        assertNull(msg.getHeader("foo"));
-
-        msg.setHeader("tick", "bla");
-        msg.setHeader("tack", "blaa");
-        msg.setHeader("tock", "blaaa");
-
-        assertEquals("bla", msg.getHeader("tick"));
-        assertEquals("blaa", msg.getHeader("tack"));
-        assertEquals("blaaa", msg.getHeader("tock"));
-
-        msg.removeHeader(null);
-
-        assertFalse(msg.getHeaders().isEmpty());
-    }
-
     public void testRemoveHeaderWithNullValue() {
         Message msg = new DefaultMessage();
         assertNull(msg.getHeader("foo"));

http://git-wip-us.apache.org/repos/asf/camel/blob/5ae518b7/camel-core/src/test/java/org/apache/camel/util/CaseInsensitiveMapTest.java
----------------------------------------------------------------------
diff --git a/camel-core/src/test/java/org/apache/camel/util/CaseInsensitiveMapTest.java b/camel-core/src/test/java/org/apache/camel/util/CaseInsensitiveMapTest.java
index 17d78bd..8bfd5d8 100644
--- a/camel-core/src/test/java/org/apache/camel/util/CaseInsensitiveMapTest.java
+++ b/camel-core/src/test/java/org/apache/camel/util/CaseInsensitiveMapTest.java
@@ -344,7 +344,9 @@ public class CaseInsensitiveMapTest extends TestCase {
 
         Map<String, Object> other = new HashMap<String, Object>(map);
         assertEquals(false, other.containsKey("foo"));
-        assertEquals(true, other.containsKey("FOO"));
+        assertEquals(false, other.containsKey("FOO"));
+        // CaseInsensitiveMap preserves the original keys, which would be the 1st key we
put
+        assertEquals(true, other.containsKey("Foo"));
         assertEquals(1, other.size());
     }
 
@@ -462,7 +464,9 @@ public class CaseInsensitiveMapTest extends TestCase {
                     foo.put("cake", "cheese");
 
                     // copy foo to map as map is a shared resource
-                    map.putAll(foo);
+                    synchronized (map) {
+                        map.putAll(foo);
+                    }
 
                     latch.countDown();
                 }

http://git-wip-us.apache.org/repos/asf/camel/blob/5ae518b7/components/camel-cometd/src/main/java/org/apache/camel/component/cometd/CometdBinding.java
----------------------------------------------------------------------
diff --git a/components/camel-cometd/src/main/java/org/apache/camel/component/cometd/CometdBinding.java
b/components/camel-cometd/src/main/java/org/apache/camel/component/cometd/CometdBinding.java
index eaa53d4..38f6c09 100644
--- a/components/camel-cometd/src/main/java/org/apache/camel/component/cometd/CometdBinding.java
+++ b/components/camel-cometd/src/main/java/org/apache/camel/component/cometd/CometdBinding.java
@@ -78,10 +78,13 @@ public class CometdBinding {
 
         Message message = new DefaultMessage();
         message.setBody(data);
-        message.setHeaders(getHeadersFromMessage(cometdMessage));
+        Map headers = getHeadersFromMessage(cometdMessage);
+        if (headers != null) {
+            message.setHeaders(headers);
+        }
         message.setHeader(COMETD_CLIENT_ID_HEADER_NAME, remote.getId());
 
-        if (cometdMessage.get(COMETD_SUBSCRIPTION_HEADER_NAME) != null) {
+        if (cometdMessage != null && cometdMessage.get(COMETD_SUBSCRIPTION_HEADER_NAME)
!= null) {
             message.setHeader(COMETD_SUBSCRIPTION_HEADER_NAME, cometdMessage.get(COMETD_SUBSCRIPTION_HEADER_NAME));
         }
         


Mime
View raw message