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/04/02 04:32:09 UTC

[james-project] 02/15: [Performance] Avoid loading mailbox counters for getAllReadableMailboxes

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 ed3b695b0abd28c82ea37fe7de59aaa2cd6be74a
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Mon Mar 30 14:20:53 2020 +0700

    [Performance] Avoid loading mailbox counters for getAllReadableMailboxes
    
    Glowroot flameGraph suggest we spend 60% of GetMessageList time loading
    mailbox counters while we do not need them
---
 .../james/mailbox/store/StoreMailboxManager.java   | 28 ++++++++++++----------
 .../vault/DeletedMessageVaultIntegrationTest.java  |  4 ++--
 .../org/apache/james/jmap/JmapCommonRequests.java  |  4 ++--
 3 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java
index 6cb08a8..c2da6e9 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMailboxManager.java
@@ -592,19 +592,11 @@ public class StoreMailboxManager implements MailboxManager {
 
     @Override
     public List<MailboxMetaData> search(MailboxQuery mailboxExpression, MailboxSession session) throws MailboxException {
-        return searchMailboxes(mailboxExpression, session, Right.Lookup);
+        return searchMailboxesMetadata(mailboxExpression, session, Right.Lookup);
     }
 
-    private List<MailboxMetaData> searchMailboxes(MailboxQuery mailboxQuery, MailboxSession session, Right right) throws MailboxException {
-        MailboxMapper mailboxMapper = mailboxSessionMapperFactory.getMailboxMapper(session);
-        Stream<Mailbox> baseMailboxes = mailboxMapper
-            .findMailboxWithPathLike(toSingleUserQuery(mailboxQuery, session))
-            .stream();
-        Stream<Mailbox> delegatedMailboxes = getDelegatedMailboxes(mailboxMapper, mailboxQuery, right, session);
-        List<Mailbox> mailboxes = Stream.concat(baseMailboxes, delegatedMailboxes)
-            .distinct()
-            .filter(Throwing.predicate(mailbox -> storeRightManager.hasRight(mailbox, right, session)))
-            .collect(Guavate.toImmutableList());
+    private List<MailboxMetaData> searchMailboxesMetadata(MailboxQuery mailboxQuery, MailboxSession session, Right right) throws MailboxException {
+        List<Mailbox> mailboxes = searchMailboxes(mailboxQuery, session, right);
 
         ImmutableMap<MailboxId, MailboxCounters> counters = getMailboxCounters(mailboxes, session)
             .stream()
@@ -622,6 +614,18 @@ public class StoreMailboxManager implements MailboxManager {
             .collect(Guavate.toImmutableList());
     }
 
+    private List<Mailbox> searchMailboxes(MailboxQuery mailboxQuery, MailboxSession session, Right right) throws MailboxException {
+        MailboxMapper mailboxMapper = mailboxSessionMapperFactory.getMailboxMapper(session);
+        Stream<Mailbox> baseMailboxes = mailboxMapper
+            .findMailboxWithPathLike(toSingleUserQuery(mailboxQuery, session))
+            .stream();
+        Stream<Mailbox> delegatedMailboxes = getDelegatedMailboxes(mailboxMapper, mailboxQuery, right, session);
+        return Stream.concat(baseMailboxes, delegatedMailboxes)
+            .distinct()
+            .filter(Throwing.predicate(mailbox -> storeRightManager.hasRight(mailbox, right, session)))
+            .collect(Guavate.toImmutableList());
+    }
+
     static MailboxQuery.UserBound toSingleUserQuery(MailboxQuery mailboxQuery, MailboxSession mailboxSession) {
         return MailboxQuery.builder()
             .namespace(mailboxQuery.getNamespace().orElse(MailboxConstants.USER_NAMESPACE))
@@ -693,7 +697,7 @@ public class StoreMailboxManager implements MailboxManager {
     private Stream<MailboxId> getAllReadableMailbox(MailboxSession session) throws MailboxException {
         return searchMailboxes(MailboxQuery.builder().matchesAllMailboxNames().build(), session, Right.Read)
             .stream()
-            .map(MailboxMetaData::getId);
+            .map(Mailbox::getMailboxId);
     }
 
     @Override
diff --git a/server/protocols/webadmin-integration-test/webadmin-integration-test-common/src/main/java/org/apache/james/webadmin/integration/vault/DeletedMessageVaultIntegrationTest.java b/server/protocols/webadmin-integration-test/webadmin-integration-test-common/src/main/java/org/apache/james/webadmin/integration/vault/DeletedMessageVaultIntegrationTest.java
index f166582..ae04e91 100644
--- a/server/protocols/webadmin-integration-test/webadmin-integration-test-common/src/main/java/org/apache/james/webadmin/integration/vault/DeletedMessageVaultIntegrationTest.java
+++ b/server/protocols/webadmin-integration-test/webadmin-integration-test-common/src/main/java/org/apache/james/webadmin/integration/vault/DeletedMessageVaultIntegrationTest.java
@@ -30,7 +30,7 @@ import static org.apache.james.jmap.JMAPTestingConstants.calmlyAwait;
 import static org.apache.james.jmap.JMAPTestingConstants.jmapRequestSpecBuilder;
 import static org.apache.james.jmap.JmapCommonRequests.deleteMessages;
 import static org.apache.james.jmap.JmapCommonRequests.getAllMailboxesIds;
-import static org.apache.james.jmap.JmapCommonRequests.getLastMessageId;
+import static org.apache.james.jmap.JmapCommonRequests.getLatestMessageId;
 import static org.apache.james.jmap.JmapCommonRequests.getOutboxId;
 import static org.apache.james.jmap.JmapCommonRequests.listMessageIdsForAccount;
 import static org.apache.james.jmap.LocalHostURIBuilder.baseUri;
@@ -823,7 +823,7 @@ public abstract class DeletedMessageVaultIntegrationTest {
         exportVaultContent(webAdminApi, exportRequest);
 
         WAIT_TWO_MINUTES.until(() -> listMessageIdsForAccount(shareeAccessToken).size() == currentNumberOfMessages + 1);
-        String exportingMessageId = getLastMessageId(shareeAccessToken);
+        String exportingMessageId = getLatestMessageId(shareeAccessToken, Role.INBOX);
 
         return exportedFileLocationFromMailHeader(exportingMessageId, shareeAccessToken);
     }
diff --git a/server/testing/src/main/java/org/apache/james/jmap/JmapCommonRequests.java b/server/testing/src/main/java/org/apache/james/jmap/JmapCommonRequests.java
index 72138fe..981e549 100644
--- a/server/testing/src/main/java/org/apache/james/jmap/JmapCommonRequests.java
+++ b/server/testing/src/main/java/org/apache/james/jmap/JmapCommonRequests.java
@@ -141,10 +141,10 @@ public class JmapCommonRequests {
     }
 
     public static String getLatestMessageId(AccessToken accessToken, Role mailbox) {
-        String inboxId = getMailboxId(accessToken, mailbox);
+        String mailboxId = getMailboxId(accessToken, mailbox);
         return with()
                 .header("Authorization", accessToken.asString())
-                .body("[[\"getMessageList\", {\"filter\":{\"inMailboxes\":[\"" + inboxId + "\"]}, \"sort\":[\"date desc\"]}, \"#0\"]]")
+                .body("[[\"getMessageList\", {\"filter\":{\"inMailboxes\":[\"" + mailboxId + "\"]}, \"sort\":[\"date desc\"]}, \"#0\"]]")
                 .post("/jmap")
             .then()
                 .extract()


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