From 5d35a022c91a40fd5960100444c4520df5416b7e Mon Sep 17 00:00:00 2001 From: Rajith Muditha Attapattu Date: Thu, 23 May 2013 22:27:03 +0000 Subject: QPID-4873 Commiting patch by Helen Kwong. git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk/qpid@1485878 13f79535-47bb-0310-9956-ffa450edef68 --- .../client/messaging/address/AddressHelper.java | 10 ++- .../apache/qpid/client/messaging/address/Link.java | 5 +- .../apache/qpid/client/messaging/address/Node.java | 13 +-- .../org/apache/qpid/client/AMQDestinationTest.java | 95 ++++++++++++++++++++++ .../qpid/configuration/ClientProperties.java | 2 + .../apache/qpid/messaging/util/AddressParser.java | 46 ++++++++++- .../org/apache/qpid/messaging/util/Parser.java | 14 +++- 7 files changed, 165 insertions(+), 20 deletions(-) (limited to 'java') 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 getBindings(Map props) { - List bindings = new ArrayList(); List bindingList = (props == null) ? Collections.EMPTY_LIST : (List) props.get(X_BINDINGS); - if (bindingList != null) + if (bindingList != null && !bindingList.isEmpty()) { + List bindings = new ArrayList(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 _bindings = new ArrayList(); + private List _bindings = Collections.emptyList(); private SubscriptionQueue _subscriptionQueue; public Reliability getReliability() @@ -206,7 +205,7 @@ public class Link public static class Subscription { - private Map args = new HashMap(); + private Map args = Collections.emptyMap(); private boolean exclusive = false; public Map 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 _bindings = new ArrayList(); + private List _bindings = Collections.emptyList(); private Map _declareArgs = new HashMap(); protected Node(String name) @@ -112,10 +111,6 @@ public class Node _bindings = bindings; } - public void addBinding(Binding binding) { - this._bindings.add(binding); - } - public Map 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 optionsA = new HashMap(); + 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> optionsMaps = + Collections.synchronizedMap( + new LinkedHashMap>(MAX_CACHED_ENTRIES +1,1.1f,true) + { + @Override + protected boolean removeEldestEntry(Map.Entry> eldest) + { + return size() > MAX_CACHED_ENTRIES; + } + + }); + public static List lex(String input) { return LEXER.lex(input); @@ -267,11 +289,27 @@ public class AddressParser extends Parser subject = null; } - Map options; + Map 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 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 eat_until(Token.Type ... types) { - List result = new ArrayList(); + List result = new ArrayList(); while (!matches(types)) { result.add(eat()); @@ -82,4 +82,16 @@ class Parser return result; } + /** + * Returns the remaining list of tokens, without eating them + */ + List remainder() + { + List result = new ArrayList(); + for (int i = idx; i < tokens.size(); i++) + { + result.add(tokens.get(i)); + } + return result; + } } -- cgit v1.2.1