diff options
7 files changed, 165 insertions, 20 deletions
diff --git a/java/client/src/main/java/org/apache/qpid/client/messaging/address/AddressHelper.java b/java/client/src/main/java/org/apache/qpid/client/messaging/address/AddressHelper.java index 72fc74e19c..99154e820f 100644 --- a/java/client/src/main/java/org/apache/qpid/client/messaging/address/AddressHelper.java +++ b/java/client/src/main/java/org/apache/qpid/client/messaging/address/AddressHelper.java @@ -123,10 +123,10 @@ public class AddressHelper @SuppressWarnings("unchecked") public List<Binding> getBindings(Map props) { - List<Binding> bindings = new ArrayList<Binding>(); List<Map> bindingList = (props == null) ? Collections.EMPTY_LIST : (List<Map>) props.get(X_BINDINGS); - if (bindingList != null) + if (bindingList != null && !bindingList.isEmpty()) { + List<Binding> bindings = new ArrayList<Binding>(bindingList.size()); for (Map bindingMap : bindingList) { Binding binding = new Binding( @@ -138,8 +138,12 @@ public class AddressHelper .get(ARGUMENTS)); bindings.add(binding); } + return bindings; + } + else + { + return Collections.emptyList(); } - return bindings; } public Map getDeclareArgs(Map props) diff --git a/java/client/src/main/java/org/apache/qpid/client/messaging/address/Link.java b/java/client/src/main/java/org/apache/qpid/client/messaging/address/Link.java index 40a84ebd02..a614690f83 100644 --- a/java/client/src/main/java/org/apache/qpid/client/messaging/address/Link.java +++ b/java/client/src/main/java/org/apache/qpid/client/messaging/address/Link.java @@ -20,7 +20,6 @@ */ package org.apache.qpid.client.messaging.address; -import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.List; @@ -43,7 +42,7 @@ public class Link private int _producerCapacity = 0; private Subscription subscription; private Reliability reliability = Reliability.AT_LEAST_ONCE; - private List<Binding> _bindings = new ArrayList<Binding>(); + private List<Binding> _bindings = Collections.emptyList(); private SubscriptionQueue _subscriptionQueue; public Reliability getReliability() @@ -206,7 +205,7 @@ public class Link public static class Subscription { - private Map<String,Object> args = new HashMap<String,Object>(); + private Map<String,Object> args = Collections.emptyMap(); private boolean exclusive = false; public Map<String, Object> getArgs() diff --git a/java/client/src/main/java/org/apache/qpid/client/messaging/address/Node.java b/java/client/src/main/java/org/apache/qpid/client/messaging/address/Node.java index 005f98f344..cc8e11b91a 100644 --- a/java/client/src/main/java/org/apache/qpid/client/messaging/address/Node.java +++ b/java/client/src/main/java/org/apache/qpid/client/messaging/address/Node.java @@ -21,15 +21,14 @@ package org.apache.qpid.client.messaging.address; -import org.apache.qpid.client.AMQDestination; -import org.apache.qpid.client.AMQDestination.Binding; - -import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; +import org.apache.qpid.client.AMQDestination; +import org.apache.qpid.client.AMQDestination.Binding; + public class Node { private int _nodeType = AMQDestination.UNKNOWN_TYPE; @@ -39,7 +38,7 @@ public class Node private boolean _isExclusive; private String _alternateExchange; private String _exchangeType = "topic"; // used when node is an exchange instead of a queue. - private List<Binding> _bindings = new ArrayList<Binding>(); + private List<Binding> _bindings = Collections.emptyList(); private Map<String,Object> _declareArgs = new HashMap<String,Object>(); protected Node(String name) @@ -112,10 +111,6 @@ public class Node _bindings = bindings; } - public void addBinding(Binding binding) { - this._bindings.add(binding); - } - public Map<String,Object> getDeclareArgs() { return _declareArgs; diff --git a/java/client/src/test/java/org/apache/qpid/client/AMQDestinationTest.java b/java/client/src/test/java/org/apache/qpid/client/AMQDestinationTest.java index f46623ad3b..6433c9acb7 100644 --- a/java/client/src/test/java/org/apache/qpid/client/AMQDestinationTest.java +++ b/java/client/src/test/java/org/apache/qpid/client/AMQDestinationTest.java @@ -20,6 +20,10 @@ */ package org.apache.qpid.client; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; + import junit.framework.TestCase; public class AMQDestinationTest extends TestCase @@ -63,4 +67,95 @@ public class AMQDestinationTest extends TestCase assertTrue(dest7.hashCode() != dest8.hashCode()); assertTrue(dest6.hashCode() != dest9.hashCode()); } + + /** + * Tests that destinations created with the same options string will share the same address options map. + */ + public void testCacheAddressOptionsMaps() throws Exception + { + // Create destinations 1 and 3 with the same options string, and destinations 2 and 4 with a different one + String optionsStringA = "{create: always, node: {type: topic}}"; + String optionsStringB = "{}"; // empty options + AMQDestination dest1 = createDestinationWithOptions("testDest1", optionsStringA); + AMQDestination dest2 = createDestinationWithOptions("testDest2", optionsStringB); + AMQDestination dest3 = createDestinationWithOptions("testDest3", optionsStringA); + AMQDestination dest4 = createDestinationWithOptions("testDest4", optionsStringB); + + // Destinations 1 and 3 should refer to the same address options map + assertSame("Destinations 1 and 3 were created with the same options and should refer to the same options map.", + dest1.getAddress().getOptions(), dest3.getAddress().getOptions()); + // Destinations 2 and 4 should refer to the same address options map + assertSame("Destinations 2 and 4 were created with the same options and should refer to the same options map.", + dest2.getAddress().getOptions(), dest4.getAddress().getOptions()); + // Destinations 1 and 2 should have a different options map + assertNotSame("Destinations 1 and 2 should not have the same options map.", + dest1.getAddress().getOptions(), dest2.getAddress().getOptions()); + + // Verify the contents of the shared map are as expected + Map<String, Object> optionsA = new HashMap<String, Object>(); + optionsA.put("create", "always"); + optionsA.put("node", Collections.singletonMap("type", "topic")); + assertEquals("Contents of the shared address options map are not as expected.", + optionsA, dest1.getAddress().getOptions()); + assertEquals("Contents of the empty shared address options map are not as expected.", + Collections.emptyMap(), dest2.getAddress().getOptions()); + + // Verify that address options map is immutable + try + { + dest1.getAddress().getOptions().put("testKey", "testValue"); + fail("Should not be able able to modify an address's options map."); + } + catch (UnsupportedOperationException e) + { + // expected + } + } + + private AMQDestination createDestinationWithOptions(String destName, String optionsString) throws Exception + { + String addr = "ADDR:" + destName + "; " + optionsString; + return new AMQAnyDestination(addr); + } + + /** + * Tests that when a queue has no link subscription arguments and no link bindings, its Subscription + * arguments and its bindings list refer to constant empty collections. + */ + public void testEmptyLinkBindingsAndSubscriptionArgs() throws Exception + { + // no link properties + assertEmptyLinkBindingsAndSubscriptionArgs(new AMQAnyDestination("ADDR:testQueue")); + + // has link properties but no x-bindings; has link x-subscribes but no arguments + String xSubscribeAddr = "ADDR:testQueueWithXSubscribes; {link: {x-subscribes: {exclusive: true}}}"; + assertEmptyLinkBindingsAndSubscriptionArgs(new AMQAnyDestination(xSubscribeAddr)); + } + + private void assertEmptyLinkBindingsAndSubscriptionArgs(AMQDestination dest) { + assertEquals("Default link subscription arguments should be the constant Collections empty map.", + Collections.emptyMap(), dest.getLink().getSubscription().getArgs()); + assertSame("Defaultl link bindings should be the constant Collections empty list.", + Collections.emptyList(), dest.getLink().getBindings()); + } + + /** + * Tests that when a node has no bindings specified, its bindings list refers to a constant empty list, + * so that we are not consuming extra memory unnecessarily. + */ + public void testEmptyNodeBindings() throws Exception + { + // no node properties + assertEmptyNodeBindings(new AMQAnyDestination("ADDR:testDest1")); + // has node properties but no x-bindings + assertEmptyNodeBindings(new AMQAnyDestination("ADDR:testDest2; {node: {type: queue}}")); + assertEmptyNodeBindings(new AMQAnyDestination("ADDR:testDest3; {node: {type: topic}}")); + } + + private void assertEmptyNodeBindings(AMQDestination dest) + { + assertSame("Empty node bindings should refer to the constant Collections empty list.", + Collections.emptyList(), dest.getNode().getBindings()); + } + } diff --git a/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java b/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java index 7aa280ce02..a0140785f4 100644 --- a/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java +++ b/java/common/src/main/java/org/apache/qpid/configuration/ClientProperties.java @@ -205,6 +205,8 @@ public class ClientProperties public static final String QPID_DECLARE_EXCHANGES_PROP_NAME = "qpid.declare_exchanges"; public static final String VERIFY_QUEUE_ON_SEND = "qpid.verify_queue_on_send"; + public static final String QPID_MAX_CACHED_ADDR_OPTION_STRINGS = "qpid.max_cached_address_option_strings"; + public static final int DEFAULT_MAX_CACHED_ADDR_OPTION_STRINGS = 10; private ClientProperties() { diff --git a/java/common/src/main/java/org/apache/qpid/messaging/util/AddressParser.java b/java/common/src/main/java/org/apache/qpid/messaging/util/AddressParser.java index d1e10607ac..4e3782673c 100644 --- a/java/common/src/main/java/org/apache/qpid/messaging/util/AddressParser.java +++ b/java/common/src/main/java/org/apache/qpid/messaging/util/AddressParser.java @@ -20,12 +20,17 @@ */ package org.apache.qpid.messaging.util; -import org.apache.qpid.messaging.Address; - import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + +import org.apache.qpid.configuration.ClientProperties; +import org.apache.qpid.messaging.Address; /** @@ -58,6 +63,23 @@ public class AddressParser extends Parser private static Lexer LEXER = lxi.compile(); + private static final int MAX_CACHED_ENTRIES = Integer.getInteger(ClientProperties.QPID_MAX_CACHED_ADDR_OPTION_STRINGS, + ClientProperties.DEFAULT_MAX_CACHED_ADDR_OPTION_STRINGS); + + // stores address options maps for options strings that we have encountered; using a synchronizedMap wrapper + // in case multiple threads are parsing addresses. + private static Map<String, Map<Object, Object>> optionsMaps = + Collections.synchronizedMap( + new LinkedHashMap<String, Map<Object, Object>>(MAX_CACHED_ENTRIES +1,1.1f,true) + { + @Override + protected boolean removeEldestEntry(Map.Entry<String,Map<Object, Object>> eldest) + { + return size() > MAX_CACHED_ENTRIES; + } + + }); + public static List<Token> lex(String input) { return LEXER.lex(input); @@ -267,11 +289,27 @@ public class AddressParser extends Parser subject = null; } - Map options; + Map<Object, Object> options; if (matches(SEMI)) { eat(SEMI); - options = map(); + + // get the remaining string denoting the options and see if we've already encountered an address + // with the same options before + String optionsString = toks2str(remainder()); + Map<Object,Object> storedMap = optionsMaps.get(optionsString); + if (storedMap == null) + { + // if these are new options, construct a new map and store it in the encountered collection + options = Collections.unmodifiableMap(map()); + optionsMaps.put(optionsString, options); + } + else + { + // if we already have the map for these options, use the stored map + options = storedMap; + eat_until(EOF); + } } else { diff --git a/java/common/src/main/java/org/apache/qpid/messaging/util/Parser.java b/java/common/src/main/java/org/apache/qpid/messaging/util/Parser.java index 2e983f5165..3019326963 100644 --- a/java/common/src/main/java/org/apache/qpid/messaging/util/Parser.java +++ b/java/common/src/main/java/org/apache/qpid/messaging/util/Parser.java @@ -74,7 +74,7 @@ class Parser List<Token> eat_until(Token.Type ... types) { - List<Token> result = new ArrayList(); + List<Token> result = new ArrayList<Token>(); while (!matches(types)) { result.add(eat()); @@ -82,4 +82,16 @@ class Parser return result; } + /** + * Returns the remaining list of tokens, without eating them + */ + List<Token> remainder() + { + List<Token> result = new ArrayList<Token>(); + for (int i = idx; i < tokens.size(); i++) + { + result.add(tokens.get(i)); + } + return result; + } } |