You are viewing a plain text version of this content. The canonical link for it is here.
Posted to server-dev@james.apache.org by bt...@apache.org on 2020/07/23 07:53:18 UTC
[james-project] 07/18: JAMES-3177 make use of a persistent
datastructure to avoid most locking in UidMsnConverter
This is an automated email from the ASF dual-hosted git repository.
btellier pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git
commit db67bdfc5623a09b88c4c8a9763a35174a420e86
Author: Matthieu Baechler <ma...@apache.org>
AuthorDate: Tue Jun 16 17:42:30 2020 +0200
JAMES-3177 make use of a persistent datastructure to avoid most locking in UidMsnConverter
Leveraging vavr here helps a lot but the caller is not ready yet to handle
an immutable UidMsnConverter.
Next step is to ensure locking when calling mutation method on UidMsnConverter
and removing locks
---
protocols/imap/pom.xml | 4 ++
.../james/imap/processor/base/UidMsnConverter.java | 83 ++++++++--------------
.../imap/processor/base/UidMsnConverterTest.java | 23 +++---
3 files changed, 47 insertions(+), 63 deletions(-)
diff --git a/protocols/imap/pom.xml b/protocols/imap/pom.xml
index b6930eb..a8dc191 100644
--- a/protocols/imap/pom.xml
+++ b/protocols/imap/pom.xml
@@ -91,6 +91,10 @@
<artifactId>javax.mail</artifactId>
</dependency>
<dependency>
+ <groupId>io.vavr</groupId>
+ <artifactId>vavr</artifactId>
+ </dependency>
+ <dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
</dependency>
diff --git a/protocols/imap/src/main/java/org/apache/james/imap/processor/base/UidMsnConverter.java b/protocols/imap/src/main/java/org/apache/james/imap/processor/base/UidMsnConverter.java
index fc7b114..1850db5 100644
--- a/protocols/imap/src/main/java/org/apache/james/imap/processor/base/UidMsnConverter.java
+++ b/protocols/imap/src/main/java/org/apache/james/imap/processor/base/UidMsnConverter.java
@@ -19,97 +19,70 @@
package org.apache.james.imap.processor.base;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
import java.util.Optional;
-import java.util.TreeSet;
+import java.util.function.Function;
import org.apache.james.mailbox.MessageUid;
import org.apache.james.mailbox.NullableMessageSequenceNumber;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.collect.Lists;
-public class UidMsnConverter {
+import io.vavr.collection.TreeSet;
- public static final int FIRST_MSN = 1;
+public class UidMsnConverter {
- @VisibleForTesting final ArrayList<MessageUid> uids;
+ @VisibleForTesting TreeSet<MessageUid> uids;
public UidMsnConverter() {
- this.uids = Lists.newArrayList();
+ this.uids = TreeSet.empty();
}
- public synchronized void addAll(List<MessageUid> addedUids) {
- TreeSet<MessageUid> tmp = new TreeSet<>();
- tmp.addAll(uids);
- tmp.addAll(addedUids);
- uids.clear();
- uids.addAll(tmp);
+ public synchronized void addAll(java.util.List<MessageUid> addedUids) {
+ uids = uids.addAll(addedUids);
}
- public synchronized NullableMessageSequenceNumber getMsn(MessageUid uid) {
- int position = Collections.binarySearch(uids, uid);
- if (position < 0) {
- return NullableMessageSequenceNumber.noMessage();
- }
- return NullableMessageSequenceNumber.of(position + 1);
+ public NullableMessageSequenceNumber getMsn(MessageUid uid) {
+ return uids
+ .zipWithIndex()
+ .toMap(Function.identity())
+ .get(uid)
+ .map(position -> NullableMessageSequenceNumber.of(position + 1))
+ .getOrElse(NullableMessageSequenceNumber.noMessage());
}
- public synchronized Optional<MessageUid> getUid(int msn) {
- if (msn <= uids.size() && msn > 0) {
- return Optional.of(uids.get(msn - 1));
- }
- return Optional.empty();
+ public Optional<MessageUid> getUid(int msn) {
+ return uids
+ .drop(msn - 1)
+ .headOption()
+ .toJavaOptional();
}
- public synchronized Optional<MessageUid> getLastUid() {
- if (uids.isEmpty()) {
- return Optional.empty();
- }
- return getUid(getLastMsn());
+ public Optional<MessageUid> getLastUid() {
+ return uids.lastOption().toJavaOptional();
}
- public synchronized Optional<MessageUid> getFirstUid() {
- return getUid(FIRST_MSN);
+ public Optional<MessageUid> getFirstUid() {
+ return uids.headOption().toJavaOptional();
}
- public synchronized int getNumMessage() {
+ public int getNumMessage() {
return uids.size();
}
public synchronized void remove(MessageUid uid) {
- uids.remove(uid);
+ uids = uids.remove(uid);
}
- public synchronized boolean isEmpty() {
+ public boolean isEmpty() {
return uids.isEmpty();
}
public synchronized void clear() {
- uids.clear();
+ uids = TreeSet.empty();
}
public synchronized void addUid(MessageUid uid) {
- if (uids.contains(uid)) {
- return;
- }
- if (isLastUid(uid)) {
- uids.add(uid);
- } else {
- uids.add(uid);
- Collections.sort(uids);
- }
+ uids = uids.add(uid);
}
- private boolean isLastUid(MessageUid uid) {
- Optional<MessageUid> lastUid = getLastUid();
- return !lastUid.isPresent() ||
- lastUid.get().compareTo(uid) < 0;
- }
-
- private int getLastMsn() {
- return getNumMessage();
- }
}
diff --git a/protocols/imap/src/test/java/org/apache/james/imap/processor/base/UidMsnConverterTest.java b/protocols/imap/src/test/java/org/apache/james/imap/processor/base/UidMsnConverterTest.java
index 438db40..78b8b3f 100644
--- a/protocols/imap/src/test/java/org/apache/james/imap/processor/base/UidMsnConverterTest.java
+++ b/protocols/imap/src/test/java/org/apache/james/imap/processor/base/UidMsnConverterTest.java
@@ -33,6 +33,8 @@ import org.junit.Test;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
+import io.vavr.Tuple;
+
public class UidMsnConverterTest {
private UidMsnConverter testee;
private MessageUid messageUid1;
@@ -62,11 +64,20 @@ public class UidMsnConverterTest {
}
@Test
- public void getUidShouldTheCorrespondingUidIfItExist() {
+ public void getUidShouldReturnEmptyIfOutOfRange() {
testee.addUid(messageUid1);
+ testee.addUid(messageUid2);
- assertThat(testee.getUid(1))
- .contains(messageUid1);
+ assertThat(testee.getUid(50))
+ .isEmpty();
+ }
+
+ @Test
+ public void getUidShouldReturnTheCorrespondingUidIfItExist() {
+ testee.addAll(ImmutableList.of(messageUid1, messageUid2));
+
+ assertThat(testee.getUid(2))
+ .contains(messageUid2);
}
@Test
@@ -447,11 +458,7 @@ public class UidMsnConverterTest {
}
private Map<Integer, MessageUid> mapTesteeInternalDataToMsnByUid() {
- ImmutableMap.Builder<Integer, MessageUid> result = ImmutableMap.builder();
- for (int i = 0; i < testee.uids.size(); i++) {
- result.put(i + 1, testee.uids.get(i));
- }
- return result.build();
+ return testee.uids.zipWithIndex().toMap(input -> Tuple.of(input._2 + 1, input._1)).toJavaMap();
}
}
\ No newline at end of file
---------------------------------------------------------------------
To unsubscribe, e-mail: server-dev-unsubscribe@james.apache.org
For additional commands, e-mail: server-dev-help@james.apache.org