Repository: camel Updated Branches: refs/heads/master ae10823c3 -> ac0e8e8de
CAMEL-11330: Optimize DefaultExchange to use plain HashMap for exchange properties as ConcurrentMap should not be needed. Project: http://git-wip-us.apache.org/repos/asf/camel/repo Commit: http://git-wip-us.apache.org/repos/asf/camel/commit/ac0e8e8d Tree: http://git-wip-us.apache.org/repos/asf/camel/tree/ac0e8e8d Diff: http://git-wip-us.apache.org/repos/asf/camel/diff/ac0e8e8d Branch: refs/heads/master Commit: ac0e8e8dea42411885adf88892c4c5d85b7190b6 Parents: 9bba604 Author: Claus Ibsen <davscl...@apache.org> Authored: Thu May 25 22:05:13 2017 +0200 Committer: Claus Ibsen <davscl...@apache.org> Committed: Fri May 26 09:53:17 2017 +0200 ---------------------------------------------------------------------- .../org/apache/camel/impl/DefaultExchange.java | 25 +++-- .../apache/camel/impl/DefaultExchangeTest.java | 14 +++ .../processor/Camel715ThreadProcessorTest.java | 101 +++++++++++++++++++ 3 files changed, 132 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/camel/blob/ac0e8e8d/camel-core/src/main/java/org/apache/camel/impl/DefaultExchange.java ---------------------------------------------------------------------- diff --git a/camel-core/src/main/java/org/apache/camel/impl/DefaultExchange.java b/camel-core/src/main/java/org/apache/camel/impl/DefaultExchange.java index 6db0770..97b10a2 100644 --- a/camel-core/src/main/java/org/apache/camel/impl/DefaultExchange.java +++ b/camel-core/src/main/java/org/apache/camel/impl/DefaultExchange.java @@ -18,10 +18,11 @@ package org.apache.camel.impl; import java.util.ArrayList; import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; +import java.util.Set; import org.apache.camel.CamelContext; import org.apache.camel.Endpoint; @@ -237,18 +238,28 @@ public final class DefaultExchange implements Exchange { return false; } + // store keys to be removed as we cannot loop and remove at the same time in implementations such as HashMap + Set<String> toBeRemoved = new HashSet<>(); boolean matches = false; - for (Map.Entry<String, Object> entry : properties.entrySet()) { - String key = entry.getKey(); + for (String key : properties.keySet()) { if (EndpointHelper.matchPattern(key, pattern)) { if (excludePatterns != null && isExcludePatternMatch(key, excludePatterns)) { continue; } matches = true; - properties.remove(entry.getKey()); + toBeRemoved.add(key); } + } + if (!toBeRemoved.isEmpty()) { + if (toBeRemoved.size() == properties.size()) { + // special optimization when all should be removed + properties.clear(); + } else { + toBeRemoved.forEach(k -> properties.remove(k)); + } } + return matches; } @@ -520,13 +531,11 @@ public final class DefaultExchange implements Exchange { } protected Map<String, Object> createProperties() { - // TODO: likely not needed, we can use a HashMap - return new ConcurrentHashMap<>(); + return new HashMap<>(); } protected Map<String, Object> createProperties(Map<String, Object> properties) { - // TODO: likely not needed, we can use a HashMap - return new ConcurrentHashMap<>(properties); + return new HashMap<>(properties); } private static boolean isExcludePatternMatch(String key, String... excludePatterns) { http://git-wip-us.apache.org/repos/asf/camel/blob/ac0e8e8d/camel-core/src/test/java/org/apache/camel/impl/DefaultExchangeTest.java ---------------------------------------------------------------------- diff --git a/camel-core/src/test/java/org/apache/camel/impl/DefaultExchangeTest.java b/camel-core/src/test/java/org/apache/camel/impl/DefaultExchangeTest.java index ee14ca0..610c37d 100644 --- a/camel-core/src/test/java/org/apache/camel/impl/DefaultExchangeTest.java +++ b/camel-core/src/test/java/org/apache/camel/impl/DefaultExchangeTest.java @@ -147,6 +147,20 @@ public class DefaultExchangeTest extends ExchangeTestSupport { assertEquals("Africa", exchange.getProperty("zone", String.class)); } + public void testRemoveAllProperties() throws Exception { + exchange.removeProperty("foobar"); + assertFalse(exchange.hasProperties()); + + exchange.setProperty("fruit", "apple"); + exchange.setProperty("fruit1", "banana"); + exchange.setProperty("zone", "Africa"); + assertTrue(exchange.hasProperties()); + + exchange.removeProperties("*"); + assertFalse(exchange.hasProperties()); + assertEquals(exchange.getProperties().size(), 0); + } + public void testRemovePropertiesWithExclusion() throws Exception { exchange.removeProperty("foobar"); assertFalse(exchange.hasProperties()); http://git-wip-us.apache.org/repos/asf/camel/blob/ac0e8e8d/camel-core/src/test/java/org/apache/camel/processor/Camel715ThreadProcessorTest.java ---------------------------------------------------------------------- diff --git a/camel-core/src/test/java/org/apache/camel/processor/Camel715ThreadProcessorTest.java b/camel-core/src/test/java/org/apache/camel/processor/Camel715ThreadProcessorTest.java new file mode 100644 index 0000000..d103f3b --- /dev/null +++ b/camel-core/src/test/java/org/apache/camel/processor/Camel715ThreadProcessorTest.java @@ -0,0 +1,101 @@ +/** + * 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.camel.processor; + +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; + +import junit.framework.TestCase; +import org.apache.camel.CamelContext; +import org.apache.camel.Endpoint; +import org.apache.camel.Exchange; +import org.apache.camel.Message; +import org.apache.camel.Processor; +import org.apache.camel.ProducerTemplate; +import org.apache.camel.builder.RouteBuilder; +import org.apache.camel.component.mock.MockEndpoint; +import org.apache.camel.impl.DefaultCamelContext; +import org.junit.Test; + +/** + * An old unit test from CAMEL-715 which reproduced a problem which we don't have anymore + * in Camel threads EIP and the routing engine. + */ +public class Camel715ThreadProcessorTest extends TestCase { + private static final int ITERS = 50000; + + class SendingProcessor implements Processor { + int iterationNumber; + + public SendingProcessor(int iter) { + iterationNumber = iter; + } + + public void process(Exchange exchange) throws Exception { + Message in = exchange.getIn(); + in.setBody("a"); + // may set the property here + exchange.setProperty("iterationNumber", iterationNumber); + } + } + + @Test + public void testThreadProcessor() throws Exception { + CamelContext context = new DefaultCamelContext(); + + final CountDownLatch latch = new CountDownLatch(ITERS); + + context.addRoutes(new RouteBuilder() { + + @Override + public void configure() throws Exception { + from("direct:a") + .threads(4) + .to("mock:input") + .process(new Processor() { + public void process(Exchange ex) throws Exception { + latch.countDown(); + } + }); + } + }); + + MockEndpoint mock = context.getEndpoint("mock:input", MockEndpoint.class); + mock.expectedMessageCount(ITERS); + + final ProducerTemplate template = context.createProducerTemplate(); + + final Endpoint e = context.getEndpoint("direct:a"); + context.start(); + + for (int i = 0; i < ITERS; i++) { + template.send(e, new SendingProcessor(i)); + } + + MockEndpoint.assertIsSatisfied(30, TimeUnit.SECONDS); + + latch.await(30, TimeUnit.SECONDS); + + for (int i = 0; i < ITERS; i++) { + Integer number = mock.getReceivedExchanges().get(i).getProperty("iterationNumber", Integer.class); + assertNotNull(number); + assertEquals(i, number.intValue()); + } + + context.stop(); + } +}