From e714a5a2a8a90e1730cc715229113e0d6eccdbfa Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Mon, 8 Dec 2014 16:37:07 -0500 Subject: [PATCH] INT-3572: Improve Accept Once Filter Performance JIRA: https://jira.spring.io/browse/INT-3572 Previously, a blocking queue was used to hold seen files; this was used to enable FIFO when a max capacity is set. When used with no capacity, a queue is not needed; also the `contains` operation on a queue requires a linear search which does not scale well for a large number of files. Use a `HashSet` instead to significantly improve the `contains` performance. Only maintain a queue if a max capacity is set. Fix `AcceptOnceFileListFilterTests` to be compatible with Java < 8 --- .../filters/AcceptOnceFileListFilter.java | 37 ++++++++++++------ .../AcceptOnceFileListFilterTests.java | 39 +++++++++++++++++++ 2 files changed, 64 insertions(+), 12 deletions(-) diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/filters/AcceptOnceFileListFilter.java b/spring-integration-file/src/main/java/org/springframework/integration/file/filters/AcceptOnceFileListFilter.java index 8da68d584e..1456145553 100644 --- a/spring-integration-file/src/main/java/org/springframework/integration/file/filters/AcceptOnceFileListFilter.java +++ b/spring-integration-file/src/main/java/org/springframework/integration/file/filters/AcceptOnceFileListFilter.java @@ -16,8 +16,10 @@ package org.springframework.integration.file.filters; +import java.util.HashSet; import java.util.List; import java.util.Queue; +import java.util.Set; import java.util.concurrent.LinkedBlockingQueue; /** @@ -36,6 +38,8 @@ public class AcceptOnceFileListFilter extends AbstractFileListFilter imple private final Queue seen; + private final Set seenSet = new HashSet(); + private final Object monitor = new Object(); @@ -54,20 +58,24 @@ public class AcceptOnceFileListFilter extends AbstractFileListFilter imple * Creates an AcceptOnceFileListFilter based on an unbounded queue. */ public AcceptOnceFileListFilter() { - this.seen = new LinkedBlockingQueue(); + this.seen = null; } @Override public boolean accept(F file) { synchronized (this.monitor) { - if (this.seen.contains(file)) { + if (this.seenSet.contains(file)) { return false; } - if (!this.seen.offer(file)) { - this.seen.poll(); - this.seen.add(file); + if (this.seen != null) { + if (!this.seen.offer(file)) { + F removed = this.seen.poll(); + this.seenSet.remove(removed); + this.seen.add(file); + } } + this.seenSet.add(file); return true; } } @@ -78,13 +86,18 @@ public class AcceptOnceFileListFilter extends AbstractFileListFilter imple */ @Override public void rollback(F file, List files) { - boolean rollingBack = false; - for (F fileToRollback : files) { - if (fileToRollback.equals(file)) { - rollingBack = true; - } - if (rollingBack) { - this.seen.remove(fileToRollback); + synchronized (this.monitor) { + boolean rollingBack = false; + for (F fileToRollback : files) { + if (fileToRollback.equals(file)) { + rollingBack = true; + } + if (rollingBack) { + this.seenSet.remove(fileToRollback); + if (this.seen != null) { + this.seen.remove(fileToRollback); + } + } } } } diff --git a/spring-integration-file/src/test/java/org/springframework/integration/file/filters/AcceptOnceFileListFilterTests.java b/spring-integration-file/src/test/java/org/springframework/integration/file/filters/AcceptOnceFileListFilterTests.java index abb2497296..2a36e87fd9 100644 --- a/spring-integration-file/src/test/java/org/springframework/integration/file/filters/AcceptOnceFileListFilterTests.java +++ b/spring-integration-file/src/test/java/org/springframework/integration/file/filters/AcceptOnceFileListFilterTests.java @@ -16,14 +16,23 @@ package org.springframework.integration.file.filters; +import static org.hamcrest.Matchers.contains; +import static org.hamcrest.Matchers.containsInAnyOrder; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import java.util.Arrays; import java.util.List; +import java.util.Queue; +import java.util.Set; import org.junit.Test; +import org.springframework.integration.test.util.TestUtils; +import org.springframework.util.StopWatch; + /** * @author Gary Russell * @since 4.0.4 @@ -31,6 +40,36 @@ import org.junit.Test; */ public class AcceptOnceFileListFilterTests { + @Test + // This test used to take 34 seconds to run; now 25 milliseconds. + public void testPerformance_INT3572() { + StopWatch watch = new StopWatch(); + watch.start(); + AcceptOnceFileListFilter filter = new AcceptOnceFileListFilter(); + for (int i = 0; i < 100000; i++) { + filter.accept("" + i); + } + watch.stop(); + assertTrue(watch.getTotalTimeMillis() < 5000); + } + + @Test + @SuppressWarnings("unchecked") + public void testCapacity() { + AcceptOnceFileListFilter filter = new AcceptOnceFileListFilter(2); + assertTrue(filter.accept("foo")); + assertTrue(filter.accept("bar")); + assertFalse(filter.accept("foo")); + assertTrue(filter.accept("baz")); + assertTrue(filter.accept("foo")); + Queue seen = TestUtils.getPropertyValue(filter, "seen", Queue.class); + assertEquals(2, seen.size()); + Set seenSet = TestUtils.getPropertyValue(filter, "seenSet", Set.class); + assertEquals(2, seenSet.size()); + assertThat(seen, contains("baz", "foo")); + assertThat(seenSet, containsInAnyOrder("foo", "baz")); + } + @Test public void testRollback() { AcceptOnceFileListFilter filter = new AcceptOnceFileListFilter();