You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by bt...@apache.org on 2020/10/06 01:20:45 UTC

[james-project] branch master updated (11ea9fb -> e70905f)

This is an automated email from the ASF dual-hosted git repository.

btellier pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/james-project.git.


    from 11ea9fb  JAMES-3396 Document loop detection in webadmin documentation
     new 0d47412  Revert "JAMES-3177 make use of a persistent datastructure to avoid most locking in UidMsnConverter"
     new e70905f  JAMES-3402 Add performance tests for UidMsnConverter::getMSN

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 .../james/imap/processor/base/UidMsnConverter.java | 83 ++++++++++++++--------
 .../imap/processor/base/UidMsnConverterTest.java   | 30 ++++----
 2 files changed, 73 insertions(+), 40 deletions(-)


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[james-project] 01/02: Revert "JAMES-3177 make use of a persistent datastructure to avoid most locking in UidMsnConverter"

Posted by bt...@apache.org.
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 0d4741227b46ba2a8ed2839f34bd40188e734243
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Mon Oct 5 09:37:16 2020 +0200

    Revert "JAMES-3177 make use of a persistent datastructure to avoid most locking in UidMsnConverter"
    
    This reverts commit db67bdfc5623a09b88c4c8a9763a35174a420e86.
---
 .../james/imap/processor/base/UidMsnConverter.java | 83 ++++++++++++++--------
 .../imap/processor/base/UidMsnConverterTest.java   | 23 +++---
 2 files changed, 63 insertions(+), 43 deletions(-)

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 1850db5..fc7b114 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,70 +19,97 @@
 
 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.function.Function;
+import java.util.TreeSet;
 
 import org.apache.james.mailbox.MessageUid;
 import org.apache.james.mailbox.NullableMessageSequenceNumber;
 
 import com.google.common.annotations.VisibleForTesting;
-
-import io.vavr.collection.TreeSet;
+import com.google.common.collect.Lists;
 
 public class UidMsnConverter {
 
-    @VisibleForTesting TreeSet<MessageUid> uids;
+    public static final int FIRST_MSN = 1;
+
+    @VisibleForTesting final ArrayList<MessageUid> uids;
 
     public UidMsnConverter() {
-        this.uids = TreeSet.empty();
+        this.uids = Lists.newArrayList();
     }
 
-    public synchronized void addAll(java.util.List<MessageUid> addedUids) {
-        uids = uids.addAll(addedUids);
+    public synchronized void addAll(List<MessageUid> addedUids) {
+        TreeSet<MessageUid> tmp = new TreeSet<>();
+        tmp.addAll(uids);
+        tmp.addAll(addedUids);
+        uids.clear();
+        uids.addAll(tmp);
     }
 
-    public NullableMessageSequenceNumber getMsn(MessageUid uid) {
-        return uids
-            .zipWithIndex()
-            .toMap(Function.identity())
-            .get(uid)
-            .map(position -> NullableMessageSequenceNumber.of(position + 1))
-            .getOrElse(NullableMessageSequenceNumber.noMessage());
+    public synchronized NullableMessageSequenceNumber getMsn(MessageUid uid) {
+        int position = Collections.binarySearch(uids, uid);
+        if (position < 0) {
+            return NullableMessageSequenceNumber.noMessage();
+        }
+        return NullableMessageSequenceNumber.of(position + 1);
     }
 
-    public Optional<MessageUid> getUid(int msn) {
-        return uids
-            .drop(msn - 1)
-            .headOption()
-            .toJavaOptional();
+    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> getLastUid() {
-        return uids.lastOption().toJavaOptional();
+    public synchronized Optional<MessageUid> getLastUid() {
+        if (uids.isEmpty()) {
+            return Optional.empty();
+        }
+        return getUid(getLastMsn());
     }
 
-    public Optional<MessageUid> getFirstUid() {
-        return uids.headOption().toJavaOptional();
+    public synchronized Optional<MessageUid> getFirstUid() {
+        return getUid(FIRST_MSN);
     }
 
-    public int getNumMessage() {
+    public synchronized int getNumMessage() {
         return uids.size();
     }
 
     public synchronized void remove(MessageUid uid) {
-        uids = uids.remove(uid);
+        uids.remove(uid);
     }
 
-    public boolean isEmpty() {
+    public synchronized boolean isEmpty() {
         return uids.isEmpty();
     }
 
     public synchronized void clear() {
-        uids = TreeSet.empty();
+        uids.clear();
     }
 
     public synchronized void addUid(MessageUid uid) {
-        uids = uids.add(uid);
+        if (uids.contains(uid)) {
+            return;
+        }
+        if (isLastUid(uid)) {
+            uids.add(uid);
+        } else {
+            uids.add(uid);
+            Collections.sort(uids);
+        }
     }
 
+    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 78b8b3f..438db40 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,8 +33,6 @@ 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;
@@ -64,20 +62,11 @@ public class UidMsnConverterTest {
     }
 
     @Test
-    public void getUidShouldReturnEmptyIfOutOfRange() {
+    public void getUidShouldTheCorrespondingUidIfItExist() {
         testee.addUid(messageUid1);
-        testee.addUid(messageUid2);
-
-        assertThat(testee.getUid(50))
-            .isEmpty();
-    }
 
-    @Test
-    public void getUidShouldReturnTheCorrespondingUidIfItExist() {
-        testee.addAll(ImmutableList.of(messageUid1, messageUid2));
-
-        assertThat(testee.getUid(2))
-            .contains(messageUid2);
+        assertThat(testee.getUid(1))
+            .contains(messageUid1);
     }
 
     @Test
@@ -458,7 +447,11 @@ public class UidMsnConverterTest {
     }
 
     private Map<Integer, MessageUid> mapTesteeInternalDataToMsnByUid() {
-        return testee.uids.zipWithIndex().toMap(input -> Tuple.of(input._2 + 1, input._1)).toJavaMap();
+        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();
     }
 
 }
\ No newline at end of file


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org


[james-project] 02/02: JAMES-3402 Add performance tests for UidMsnConverter::getMSN

Posted by bt...@apache.org.
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 e70905f33a955dc542d880bb652cc40da6c28abb
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Mon Oct 5 15:34:44 2020 +0700

    JAMES-3402 Add performance tests for UidMsnConverter::getMSN
    
    This gets the MSN for each messages in a mailbox.
---
 .../james/imap/processor/base/UidMsnConverterTest.java      | 13 +++++++++++++
 1 file changed, 13 insertions(+)

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..243abf9 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
@@ -23,6 +23,8 @@ import static org.assertj.core.api.Assertions.assertThat;
 
 import java.time.Duration;
 import java.util.Map;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
 
 import org.apache.james.mailbox.MessageUid;
 import org.apache.james.mailbox.NullableMessageSequenceNumber;
@@ -62,6 +64,17 @@ public class UidMsnConverterTest {
     }
 
     @Test
+    public void loopingGetMSNShouldSucceedForAMillionItems() {
+        int count = 1000;
+        testee.addAll(IntStream.range(0, count)
+            .mapToObj(i -> MessageUid.of(i + 1))
+            .collect(Collectors.toList()));
+
+        IntStream.range(0, 1000000)
+            .forEach(i -> testee.getMsn(MessageUid.of(i + 1)));
+    }
+
+    @Test
     public void getUidShouldTheCorrespondingUidIfItExist() {
         testee.addUid(messageUid1);
 


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org