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 2021/05/24 09:16:54 UTC

[james-project] branch master updated: [PERFORMANCE] Cache and reuse Object mappers for writing JMAP responses (#440)

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


The following commit(s) were added to refs/heads/master by this push:
     new 79ee79e  [PERFORMANCE] Cache and reuse Object mappers for writing JMAP responses (#440)
79ee79e is described below

commit 79ee79ee10c492889f373270abfad37a8c3b343b
Author: Tellier Benoit <bt...@linagora.com>
AuthorDate: Mon May 24 16:16:45 2021 +0700

    [PERFORMANCE] Cache and reuse Object mappers for writing JMAP responses (#440)
    
    * [REFACTORING] Cache and reuse Object mappers for writing JMAP responses
    
    Clients use a close ensemble of properties combination in JMAP responses.
    This enable reuse of jackson assemblies and achieve significant speedup
    of JMAP responses serialisation.
    
    Note that protections against hash collision are setted up.
---
 .../jmap/draft/methods/GetMessagesMethod.java      | 13 ++--
 .../james/jmap/draft/methods/JmapResponse.java     | 14 ++--
 .../jmap/draft/methods/JmapResponseWriterImpl.java | 78 +++++++++++++++++++---
 .../jmap/draft/methods/GetMessagesMethodTest.java  |  2 +-
 4 files changed, 86 insertions(+), 21 deletions(-)

diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetMessagesMethod.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetMessagesMethod.java
index fdd8df3..326375b 100644
--- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetMessagesMethod.java
+++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/GetMessagesMethod.java
@@ -23,9 +23,11 @@ import static org.apache.james.util.ReactorUtils.context;
 
 import java.util.List;
 import java.util.Optional;
+import java.util.Set;
 
 import javax.inject.Inject;
 
+import org.apache.commons.lang3.tuple.Pair;
 import org.apache.james.jmap.draft.exceptions.JmapFieldNotSupportedException;
 import org.apache.james.jmap.draft.json.FieldNamePropertyFilter;
 import org.apache.james.jmap.draft.model.GetMessagesRequest;
@@ -33,6 +35,7 @@ import org.apache.james.jmap.draft.model.GetMessagesResponse;
 import org.apache.james.jmap.draft.model.MessageProperties;
 import org.apache.james.jmap.draft.model.MessageProperties.HeaderProperty;
 import org.apache.james.jmap.draft.model.MethodCallId;
+import org.apache.james.jmap.draft.model.Property;
 import org.apache.james.jmap.draft.model.message.view.MessageView;
 import org.apache.james.jmap.draft.model.message.view.MessageViewFactory;
 import org.apache.james.jmap.draft.model.message.view.MetaMessageViewFactory;
@@ -82,6 +85,7 @@ public class GetMessagesMethod implements Method {
 
         GetMessagesRequest getMessagesRequest = (GetMessagesRequest) request;
         MessageProperties outputProperties = getMessagesRequest.getProperties().toOutputProperties();
+        Optional<Pair<? extends Set<? extends Property>, SimpleFilterProvider>> integerSimpleFilterProviderPair = buildOptionalHeadersFilteringFilterProvider(outputProperties);
 
         return Flux.from(metricFactory.decoratePublisherWithTimerMetric(JMAP_PREFIX + METHOD_NAME.getName(),
             Flux.from(getMessagesResponse(mailboxSession, getMessagesRequest)
@@ -89,7 +93,7 @@ public class GetMessagesMethod implements Method {
                     .response(response)
                     .responseName(RESPONSE_NAME)
                     .properties(outputProperties.getOptionalMessageProperties())
-                    .filterProvider(buildOptionalHeadersFilteringFilterProvider(outputProperties))
+                    .filterProvider(integerSimpleFilterProviderPair)
                     .build()))
             .subscriberContext(context("GET_MESSAGES", mdc(getMessagesRequest)))));
     }
@@ -102,11 +106,10 @@ public class GetMessagesMethod implements Method {
             .addContext("properties", getMessagesRequest.getProperties());
     }
 
-    private Optional<SimpleFilterProvider> buildOptionalHeadersFilteringFilterProvider(MessageProperties properties) {
+    private Optional<Pair<? extends Set<? extends Property>, SimpleFilterProvider>> buildOptionalHeadersFilteringFilterProvider(MessageProperties properties) {
         return properties.getOptionalHeadersProperties()
-            .map(this::buildHeadersPropertyFilter)
-            .map(propertyFilter -> new SimpleFilterProvider()
-                .addFilter(HEADERS_FILTER, propertyFilter));
+            .map(headerProperties -> Pair.of(headerProperties, new SimpleFilterProvider()
+                .addFilter(HEADERS_FILTER, buildHeadersPropertyFilter(headerProperties))));
     }
     
     private PropertyFilter buildHeadersPropertyFilter(ImmutableSet<HeaderProperty> headerProperties) {
diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/JmapResponse.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/JmapResponse.java
index 2706e25..0807aae 100644
--- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/JmapResponse.java
+++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/JmapResponse.java
@@ -22,6 +22,7 @@ package org.apache.james.jmap.draft.methods;
 import java.util.Optional;
 import java.util.Set;
 
+import org.apache.commons.lang3.tuple.Pair;
 import org.apache.james.jmap.draft.model.MethodCallId;
 import org.apache.james.jmap.draft.model.Property;
 
@@ -43,7 +44,7 @@ public class JmapResponse {
         private MethodCallId methodCallId;
         private Method.Response response;
         private Optional<? extends Set<? extends Property>> properties = Optional.empty();
-        private Optional<SimpleFilterProvider> filterProvider = Optional.empty();
+        private Optional<Pair<? extends Set<? extends Property>, SimpleFilterProvider>> filterProvider = Optional.empty();
 
         private Builder() {
         }
@@ -72,7 +73,7 @@ public class JmapResponse {
             return properties(Optional.ofNullable(properties));
         }
         
-        public Builder filterProvider(Optional<SimpleFilterProvider> filterProvider) {
+        public Builder filterProvider(Optional<Pair<? extends Set<? extends Property>, SimpleFilterProvider>> filterProvider) {
             this.filterProvider = filterProvider;
             return this;
         }
@@ -102,9 +103,12 @@ public class JmapResponse {
     private final MethodCallId methodCallId;
     private final Method.Response response;
     private final Optional<? extends Set<? extends Property>> properties;
-    private final Optional<SimpleFilterProvider> filterProvider;
+    private final Optional<Pair<? extends Set<? extends Property>, SimpleFilterProvider>> filterProvider;
     
-    private JmapResponse(Method.Response.Name method, MethodCallId methodCallId, Method.Response response, Optional<? extends Set<? extends Property>> properties, Optional<SimpleFilterProvider> filterProvider) {
+    private JmapResponse(Method.Response.Name method, MethodCallId methodCallId,
+                         Method.Response response,
+                         Optional<? extends Set<? extends Property>> properties,
+                         Optional<Pair<? extends Set<? extends Property>, SimpleFilterProvider>> filterProvider) {
         this.method = method;
         this.methodCallId = methodCallId;
         this.response = response;
@@ -128,7 +132,7 @@ public class JmapResponse {
         return properties;
     }
 
-    public Optional<SimpleFilterProvider> getFilterProvider() {
+    public Optional<Pair<? extends Set<? extends Property>, SimpleFilterProvider>> getFilterProvider() {
         return filterProvider;
     }
 
diff --git a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/JmapResponseWriterImpl.java b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/JmapResponseWriterImpl.java
index d2a346b..2e8f9e4 100644
--- a/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/JmapResponseWriterImpl.java
+++ b/server/protocols/jmap-draft/src/main/java/org/apache/james/jmap/draft/methods/JmapResponseWriterImpl.java
@@ -19,11 +19,14 @@
 
 package org.apache.james.jmap.draft.methods;
 
+import java.time.Duration;
 import java.util.Optional;
 import java.util.Set;
+import java.util.concurrent.ExecutionException;
 
 import javax.inject.Inject;
 
+import org.apache.commons.lang3.tuple.Pair;
 import org.apache.james.jmap.draft.json.ObjectMapperFactory;
 import org.apache.james.jmap.draft.model.InvocationResponse;
 import org.apache.james.jmap.draft.model.Property;
@@ -33,40 +36,95 @@ import com.fasterxml.jackson.databind.ser.FilterProvider;
 import com.fasterxml.jackson.databind.ser.PropertyFilter;
 import com.fasterxml.jackson.databind.ser.impl.SimpleBeanPropertyFilter;
 import com.fasterxml.jackson.databind.ser.impl.SimpleFilterProvider;
+import com.github.fge.lambdas.Throwing;
 import com.github.steveash.guavate.Guavate;
+import com.google.common.cache.Cache;
+import com.google.common.cache.CacheBuilder;
 
 import reactor.core.publisher.Flux;
 
 public class JmapResponseWriterImpl implements JmapResponseWriter {
-
     public static final String PROPERTIES_FILTER = "propertiesFilter";
+
+    private static class CachedObjectMapper {
+        private final ObjectMapper objectMapper;
+        private final Optional<? extends Set<? extends Property>> properties;
+        private final Optional<? extends Set<? extends Property>> subProperties;
+
+        private CachedObjectMapper(ObjectMapper objectMapper,
+                                   Optional<? extends Set<? extends Property>> properties,
+                                   Optional<? extends Set<? extends Property>> subProperties) {
+            this.objectMapper = objectMapper;
+            this.properties = properties;
+            this.subProperties = subProperties;
+        }
+
+        Optional<ObjectMapper> cachedMapperIfApplicable(JmapResponse jmapResponse) {
+            if (jmapResponse.getProperties().equals(properties)
+                && jmapResponse.getFilterProvider().map(Pair::getKey).equals(subProperties)) {
+
+                return Optional.of(objectMapper);
+            }
+            return Optional.empty();
+        }
+    }
+
+
     private final ObjectMapper objectMapper;
+    private final Cache<Long, CachedObjectMapper> writerCache;
 
     @Inject
     public JmapResponseWriterImpl(ObjectMapperFactory objectMapperFactory) {
         this.objectMapper = objectMapperFactory.forWriting();
+
+        writerCache = CacheBuilder.newBuilder()
+            .maximumSize(128)
+            .expireAfterAccess(Duration.ofMinutes(15))
+            .build();
     }
 
     @Override
     public Flux<InvocationResponse> formatMethodResponse(Flux<JmapResponse> jmapResponses) {
-        return jmapResponses.map(jmapResponse -> {
-            ObjectMapper objectMapper = newConfiguredObjectMapper(jmapResponse);
+        return jmapResponses.map(Throwing.function(jmapResponse -> {
+            ObjectMapper objectMapper = retrieveObjectMapperFromCache(jmapResponse)
+                .cachedMapperIfApplicable(jmapResponse)
+                .orElseGet(() -> newConfiguredObjectMapper(jmapResponse));
 
             return new InvocationResponse(
                     jmapResponse.getResponseName(),
                     objectMapper.valueToTree(jmapResponse.getResponse()),
                     jmapResponse.getMethodCallId());
-        });
+        }));
     }
-    
+
+    private CachedObjectMapper retrieveObjectMapperFromCache(JmapResponse jmapResponse) throws ExecutionException {
+        return writerCache.get(computeCachingKey(jmapResponse), () -> new CachedObjectMapper(
+            newConfiguredObjectMapper(jmapResponse),
+            jmapResponse.getProperties(),
+            jmapResponse.getFilterProvider().map(Pair::getKey)));
+    }
+
     private ObjectMapper newConfiguredObjectMapper(JmapResponse jmapResponse) {
         FilterProvider filterProvider = jmapResponse
-                .getFilterProvider()
-                .orElseGet(SimpleFilterProvider::new)
-                .setDefaultFilter(SimpleBeanPropertyFilter.serializeAll())
-                .addFilter(PROPERTIES_FILTER, getPropertiesFilter(jmapResponse.getProperties()));
-        
+            .getFilterProvider()
+            .map(Pair::getValue)
+            .orElseGet(SimpleFilterProvider::new)
+            .setDefaultFilter(SimpleBeanPropertyFilter.serializeAll())
+            .addFilter(PROPERTIES_FILTER, getPropertiesFilter(jmapResponse.getProperties()));
+
         return objectMapper.copy().setFilterProvider(filterProvider);
+
+    }
+
+    private long computeCachingKey(JmapResponse jmapResponse) {
+        long lowBits = jmapResponse.getProperties().hashCode();
+        long highBits = jmapResponse.getFilterProvider()
+            .map(Pair::getKey)
+            .map(Object::hashCode)
+            .map(i -> (long) i)
+            .orElse((long) jmapResponse.getResponseName().hashCode());
+
+        return lowBits + (highBits >> 32);
     }
     
     private PropertyFilter getPropertiesFilter(Optional<? extends Set<? extends Property>> properties) {
diff --git a/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/draft/methods/GetMessagesMethodTest.java b/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/draft/methods/GetMessagesMethodTest.java
index 3ad8864..c7e6d80 100644
--- a/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/draft/methods/GetMessagesMethodTest.java
+++ b/server/protocols/jmap-draft/src/test/java/org/apache/james/jmap/draft/methods/GetMessagesMethodTest.java
@@ -453,7 +453,7 @@ public class GetMessagesMethodTest {
             .hasSize(1)
             .extracting(JmapResponse::getFilterProvider)
             .are(new Condition<>(Optional::isPresent, "present"));
-        SimpleFilterProvider actualFilterProvider = result.get(0).getFilterProvider().get();
+        SimpleFilterProvider actualFilterProvider = result.get(0).getFilterProvider().get().getRight();
         ObjectMapper objectMapper = new ObjectMapper();
         objectMapper.registerModule(new JavaTimeModule());
         objectMapper.setFilterProvider(actualFilterProvider.setDefaultFilter(SimpleBeanPropertyFilter.serializeAll()));

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