Repository: qpid-broker-j Updated Branches: refs/heads/master db971ea2b -> 4ac5997f4
QPID-7830: [Broker-J] Simplify string caching functionality Project: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/repo Commit: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/commit/57bda83d Tree: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/tree/57bda83d Diff: http://git-wip-us.apache.org/repos/asf/qpid-broker-j/diff/57bda83d Branch: refs/heads/master Commit: 57bda83de7fbe6291aea507b5c1098fda5cf130d Parents: db971ea Author: Alex Rudyy <[email protected]> Authored: Fri May 25 15:40:57 2018 +0100 Committer: Alex Rudyy <[email protected]> Committed: Fri May 25 15:40:57 2018 +0100 ---------------------------------------------------------------------- .../server/protocol/v0_8/AMQShortString.java | 17 +++--- .../protocol/v0_8/AMQShortStringTest.java | 23 ++++---- .../server/protocol/v0_8/MessageMetaData.java | 9 ---- .../transport/BasicContentHeaderProperties.java | 24 --------- .../v0_8/transport/BasicPublishBody.java | 8 --- .../v1_0/codec/StringTypeConstructor.java | 29 +++++++++- .../type/messaging/ApplicationProperties.java | 5 +- .../messaging/NonEncodingRetainingSection.java | 30 ----------- .../v1_0/type/messaging/Properties.java | 4 +- .../v1_0/codec/StringTypeConstructorTest.java | 57 ++++++++++++++++++++ .../NonEncodingRetainingSectionTest.java | 54 ------------------- 11 files changed, 110 insertions(+), 150 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/57bda83d/broker-core/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQShortString.java ---------------------------------------------------------------------- diff --git a/broker-core/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQShortString.java b/broker-core/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQShortString.java index a01c02f..59f96e5 100644 --- a/broker-core/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQShortString.java +++ b/broker-core/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQShortString.java @@ -98,8 +98,14 @@ public final class AMQShortString implements Comparable<AMQShortString> byte[] data = new byte[length]; buffer.get(data); - final AMQShortString cached = getShortStringCache().getIfPresent(ByteBuffer.wrap(data)); - return cached != null ? cached : new AMQShortString(data); + ByteBuffer stringBuffer = ByteBuffer.wrap(data); + AMQShortString cached = getShortStringCache().getIfPresent(stringBuffer); + if (cached == null) + { + cached = new AMQShortString(data); + getShortStringCache().put(stringBuffer, cached); + } + return cached; } } @@ -297,11 +303,6 @@ public final class AMQShortString implements Comparable<AMQShortString> return false; } - public void intern() - { - getShortStringCache().put(ByteBuffer.wrap(_data), this); - } - public static AMQShortString validValueOf(Object obj) { return valueOf(obj,true,true); @@ -365,7 +366,7 @@ public final class AMQShortString implements Comparable<AMQShortString> } /** Unit testing only */ - static void setCache(final Cache<ByteBuffer, AMQShortString> cache) + static void setShortStringCache(final Cache<ByteBuffer, AMQShortString> cache) { CACHE.set(cache); } http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/57bda83d/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQShortStringTest.java ---------------------------------------------------------------------- diff --git a/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQShortStringTest.java b/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQShortStringTest.java index 38f5ac2..4c72060 100644 --- a/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQShortStringTest.java +++ b/broker-core/src/test/java/org/apache/qpid/server/protocol/v0_8/AMQShortStringTest.java @@ -34,6 +34,7 @@ import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; import org.junit.Test; +import org.apache.qpid.server.bytebuffer.QpidByteBuffer; import org.apache.qpid.test.utils.UnitTestBase; public class AMQShortStringTest extends UnitTestBase @@ -157,28 +158,30 @@ public class AMQShortStringTest extends UnitTestBase } @Test - public void testInterning() + public void testCaching() { Cache<ByteBuffer, AMQShortString> original = AMQShortString.getShortStringCache(); Cache<ByteBuffer, AMQShortString> cache = CacheBuilder.newBuilder().maximumSize(1).build(); - AMQShortString.setCache(cache); - + AMQShortString.setShortStringCache(cache); try { - AMQShortString str1 = AMQShortString.createAMQShortString("hello"); - str1.intern(); - AMQShortString str2 = AMQShortString.createAMQShortString("hello"); - AMQShortString str3 = AMQShortString.createAMQShortString("hello".getBytes(StandardCharsets.UTF_8)); + AMQShortString string = AMQShortString.createAMQShortString("hello"); + QpidByteBuffer qpidByteBuffer = QpidByteBuffer.allocate(2 * (string.length() + 1)); + string.writeToBuffer(qpidByteBuffer); + string.writeToBuffer(qpidByteBuffer); + + qpidByteBuffer.flip(); + + AMQShortString str1 = AMQShortString.readAMQShortString(qpidByteBuffer); + AMQShortString str2 = AMQShortString.readAMQShortString(qpidByteBuffer); assertEquals(str1, str2); - assertEquals(str1, str3); assertSame(str1, str2); - assertSame(str1, str3); } finally { cache.cleanUp(); - AMQShortString.setCache(original); + AMQShortString.setShortStringCache(original); } } http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/57bda83d/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/MessageMetaData.java ---------------------------------------------------------------------- diff --git a/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/MessageMetaData.java b/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/MessageMetaData.java index 2ad8511..1defedb 100644 --- a/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/MessageMetaData.java +++ b/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/MessageMetaData.java @@ -164,16 +164,7 @@ public class MessageMetaData implements StorableMessageMetaData ContentHeaderBody chb = ContentHeaderBody.createFromBuffer(buf, size); final AMQShortString exchange = AMQShortString.readAMQShortString(buf); - if (exchange != null) - { - exchange.intern(); - } final AMQShortString routingKey = AMQShortString.readAMQShortString(buf); - if (routingKey != null) - { - routingKey.intern(); - } - final byte flags = buf.get(); long arrivalTime = buf.getLong(); http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/57bda83d/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicContentHeaderProperties.java ---------------------------------------------------------------------- diff --git a/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicContentHeaderProperties.java b/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicContentHeaderProperties.java index 52183fc..43c26f1 100644 --- a/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicContentHeaderProperties.java +++ b/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicContentHeaderProperties.java @@ -374,19 +374,11 @@ public class BasicContentHeaderProperties if ((_propertyFlags & (CONTENT_TYPE_MASK)) != 0) { _contentType = AMQShortString.readAMQShortString(buffer); - if (_contentType != null) - { - _contentType.intern(); - } } if ((_propertyFlags & ENCODING_MASK) != 0) { _encoding = AMQShortString.readAMQShortString(buffer); - if (_encoding != null) - { - _encoding.intern(); - } } if ((_propertyFlags & HEADERS_MASK) != 0) @@ -418,10 +410,6 @@ public class BasicContentHeaderProperties if ((_propertyFlags & REPLY_TO_MASK) != 0) { _replyTo = AMQShortString.readAMQShortString(buffer); - if (_replyTo != null) - { - _replyTo.intern(); - } } if ((_propertyFlags & EXPIRATION_MASK) != 0) @@ -447,28 +435,16 @@ public class BasicContentHeaderProperties if ((_propertyFlags & USER_ID_MASK) != 0) { _userId = AMQShortString.readAMQShortString(buffer); - if (_userId != null) - { - _userId.intern(); - } } if ((_propertyFlags & APPLICATION_ID_MASK) != 0) { _appId = AMQShortString.readAMQShortString(buffer); - if (_appId != null) - { - _appId.intern(); - } } if ((_propertyFlags & CLUSTER_ID_MASK) != 0) { _clusterId = AMQShortString.readAMQShortString(buffer); - if (_clusterId != null) - { - _clusterId.intern(); - } } } http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/57bda83d/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicPublishBody.java ---------------------------------------------------------------------- diff --git a/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicPublishBody.java b/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicPublishBody.java index a8243ea..9821383 100644 --- a/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicPublishBody.java +++ b/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/transport/BasicPublishBody.java @@ -152,15 +152,7 @@ public class BasicPublishBody extends AMQMethodBodyImpl implements EncodableAMQD int ticket = buffer.getUnsignedShort(); AMQShortString exchange = AMQShortString.readAMQShortString(buffer); - if (exchange != null) - { - exchange.intern(); - } AMQShortString routingKey = AMQShortString.readAMQShortString(buffer); - if (routingKey != null) - { - routingKey.intern(); - } byte bitfield = buffer.get(); boolean mandatory = (bitfield & 0x01) != 0; http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/57bda83d/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/codec/StringTypeConstructor.java ---------------------------------------------------------------------- diff --git a/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/codec/StringTypeConstructor.java b/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/codec/StringTypeConstructor.java index 946dc4a..ffa708a 100644 --- a/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/codec/StringTypeConstructor.java +++ b/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/codec/StringTypeConstructor.java @@ -22,13 +22,22 @@ package org.apache.qpid.server.protocol.v1_0.codec; import static java.nio.charset.StandardCharsets.UTF_8; +import java.nio.ByteBuffer; + +import com.google.common.cache.Cache; + import org.apache.qpid.server.bytebuffer.QpidByteBuffer; import org.apache.qpid.server.protocol.v1_0.type.AmqpErrorException; import org.apache.qpid.server.protocol.v1_0.type.transport.AmqpError; +import org.apache.qpid.server.virtualhost.CacheFactory; +import org.apache.qpid.server.virtualhost.NullCache; public class StringTypeConstructor extends VariableWidthTypeConstructor<String> { + private static final NullCache<ByteBuffer, String> NULL_CACHE = new NullCache<>(); + private static final ThreadLocal<Cache<ByteBuffer, String>> CACHE = + ThreadLocal.withInitial(() -> CacheFactory.getCache("stringCache", NULL_CACHE)); public static StringTypeConstructor getInstance(int i) { @@ -67,6 +76,24 @@ public class StringTypeConstructor extends VariableWidthTypeConstructor<String> byte[] data = new byte[size]; in.get(data); - return new String(data, UTF_8); + + ByteBuffer buffer = ByteBuffer.wrap(data); + String cached = getCache().getIfPresent(buffer); + if (cached == null) + { + cached = new String(data, UTF_8); + getCache().put(buffer, cached); + } + return cached; + } + + static Cache<ByteBuffer, String> getCache() + { + return CACHE.get(); + } + + static void setCache(final Cache<ByteBuffer, String> cache) + { + CACHE.set(cache); } } http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/57bda83d/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/type/messaging/ApplicationProperties.java ---------------------------------------------------------------------- diff --git a/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/type/messaging/ApplicationProperties.java b/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/type/messaging/ApplicationProperties.java index f44f960..b2d3954 100644 --- a/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/type/messaging/ApplicationProperties.java +++ b/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/type/messaging/ApplicationProperties.java @@ -42,7 +42,6 @@ public class ApplicationProperties implements NonEncodingRetainingSection<Map<St { throw new IllegalArgumentException("Value must not be null"); } - LinkedHashMap<String,Object> copy = new LinkedHashMap<>(); for(Map.Entry<String,Object> entry: value.entrySet()) { if (entry.getKey() == null) @@ -53,10 +52,8 @@ public class ApplicationProperties implements NonEncodingRetainingSection<Map<St { throw new IllegalArgumentException("Application properties do not allow non-primitive values"); } - - copy.put(NonEncodingRetainingSection.getCached(entry.getKey()), entry.getValue()); } - _value = copy; + _value = value; } @Override http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/57bda83d/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/type/messaging/NonEncodingRetainingSection.java ---------------------------------------------------------------------- diff --git a/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/type/messaging/NonEncodingRetainingSection.java b/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/type/messaging/NonEncodingRetainingSection.java index b6385c2..fc9a43d 100644 --- a/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/type/messaging/NonEncodingRetainingSection.java +++ b/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/type/messaging/NonEncodingRetainingSection.java @@ -20,39 +20,9 @@ */ package org.apache.qpid.server.protocol.v1_0.type.messaging; -import com.google.common.cache.Cache; - import org.apache.qpid.server.protocol.v1_0.type.Section; -import org.apache.qpid.server.virtualhost.CacheFactory; -import org.apache.qpid.server.virtualhost.NullCache; public interface NonEncodingRetainingSection<T> extends Section<T> { - NullCache<String, String> NULL_CACHE = new NullCache<>(); - ThreadLocal<Cache<String, String>> CACHE = - ThreadLocal.withInitial(() -> CacheFactory.getCache("stringCache", NULL_CACHE)); - - static Cache<String, String> getStringCache() - { - return CACHE.get(); - } - - /** Unit testing only */ - static void setCache(final Cache<String, String> cache) - { - CACHE.set(cache); - } - - static String getCached(final String value) - { - String cached = getStringCache().getIfPresent(value); - if (cached == null) - { - cached = value; - getStringCache().put(cached, cached); - } - return cached; - } - EncodingRetainingSection<T> createEncodingRetainingSection(); } http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/57bda83d/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/type/messaging/Properties.java ---------------------------------------------------------------------- diff --git a/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/type/messaging/Properties.java b/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/type/messaging/Properties.java index 2fcc9ca..9c10cee 100644 --- a/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/type/messaging/Properties.java +++ b/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/type/messaging/Properties.java @@ -98,7 +98,7 @@ public class Properties implements NonEncodingRetainingSection<Properties> public void setTo(String to) { - _to = (to == null ? null : NonEncodingRetainingSection.getCached(to)); + _to = to; } public String getSubject() @@ -108,7 +108,7 @@ public class Properties implements NonEncodingRetainingSection<Properties> public void setSubject(String subject) { - _subject = (subject == null ? null : NonEncodingRetainingSection.getCached(subject)); + _subject = subject; } public String getReplyTo() http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/57bda83d/broker-plugins/amqp-1-0-protocol/src/test/java/org/apache/qpid/server/protocol/v1_0/codec/StringTypeConstructorTest.java ---------------------------------------------------------------------- diff --git a/broker-plugins/amqp-1-0-protocol/src/test/java/org/apache/qpid/server/protocol/v1_0/codec/StringTypeConstructorTest.java b/broker-plugins/amqp-1-0-protocol/src/test/java/org/apache/qpid/server/protocol/v1_0/codec/StringTypeConstructorTest.java new file mode 100644 index 0000000..6e87a44 --- /dev/null +++ b/broker-plugins/amqp-1-0-protocol/src/test/java/org/apache/qpid/server/protocol/v1_0/codec/StringTypeConstructorTest.java @@ -0,0 +1,57 @@ +/* + * + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + * + */ +package org.apache.qpid.server.protocol.v1_0.codec; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertSame; + +import java.nio.ByteBuffer; + +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; +import org.junit.Test; + +import org.apache.qpid.server.bytebuffer.QpidByteBuffer; + +public class StringTypeConstructorTest +{ + + @Test + public void construct() throws Exception + { + StringTypeConstructor constructor = StringTypeConstructor.getInstance(1); + Cache<ByteBuffer, String> original = StringTypeConstructor.getCache(); + Cache<ByteBuffer, String> cache = CacheBuilder.newBuilder().maximumSize(2).build(); + StringTypeConstructor.setCache(cache); + try + { + String string1 = constructor.construct(QpidByteBuffer.wrap(new byte[]{4, 't', 'e', 's', 't'}), null); + String string2 = constructor.construct(QpidByteBuffer.wrap(new byte[]{4, 't', 'e', 's', 't'}), null); + assertEquals(string1, string2); + assertSame(string1, string2); + } + finally + { + cache.cleanUp(); + StringTypeConstructor.setCache(original); + } + } +} http://git-wip-us.apache.org/repos/asf/qpid-broker-j/blob/57bda83d/broker-plugins/amqp-1-0-protocol/src/test/java/org/apache/qpid/server/protocol/v1_0/type/messaging/NonEncodingRetainingSectionTest.java ---------------------------------------------------------------------- diff --git a/broker-plugins/amqp-1-0-protocol/src/test/java/org/apache/qpid/server/protocol/v1_0/type/messaging/NonEncodingRetainingSectionTest.java b/broker-plugins/amqp-1-0-protocol/src/test/java/org/apache/qpid/server/protocol/v1_0/type/messaging/NonEncodingRetainingSectionTest.java deleted file mode 100644 index a56745a..0000000 --- a/broker-plugins/amqp-1-0-protocol/src/test/java/org/apache/qpid/server/protocol/v1_0/type/messaging/NonEncodingRetainingSectionTest.java +++ /dev/null @@ -1,54 +0,0 @@ -/* - * - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - * - */ -package org.apache.qpid.server.protocol.v1_0.type.messaging; - -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertSame; - -import com.google.common.cache.Cache; -import com.google.common.cache.CacheBuilder; -import org.junit.Test; - -public class NonEncodingRetainingSectionTest -{ - - @Test - public void getCached() - { - Cache<String, String> original = NonEncodingRetainingSection.getStringCache(); - Cache<String, String> cache = CacheBuilder.newBuilder().maximumSize(1).build(); - NonEncodingRetainingSection.setCache(cache); - - try - { - String string1 = NonEncodingRetainingSection.getCached(new String(new char[]{'t', 'e', 's', 't'})); - String string2 = NonEncodingRetainingSection.getCached(new String(new char[]{'t', 'e', 's', 't'})); - - assertEquals(string1, string2); - assertSame(string1, string2); - } - finally - { - cache.cleanUp(); - NonEncodingRetainingSection.setCache(original); - } - } -} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
