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/03/08 05:44:14 UTC
[james-project] 04/30: JAMES-3506 Avoid a memory copy when saving
attachments
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 4e0ff6fe53ba600d342eb5e6ced394d4167923b7
Author: Benoit Tellier <bt...@linagora.com>
AuthorDate: Fri Feb 26 09:27:56 2021 +0700
JAMES-3506 Avoid a memory copy when saving attachments
---
.../james/mailbox/model/ParsedAttachment.java | 12 ++++++-----
.../cassandra/mail/CassandraAttachmentMapper.java | 18 +++++++++++------
.../inmemory/mail/InMemoryAttachmentMapper.java | 23 ++++++++++++----------
.../store/mail/model/impl/MessageParser.java | 17 +++++++++++++++-
.../store/mail/model/AttachmentMapperTest.java | 9 +++++----
.../model/MessageWithAttachmentMapperTest.java | 7 ++++---
.../store/mail/model/impl/MessageParserTest.java | 2 +-
7 files changed, 58 insertions(+), 30 deletions(-)
diff --git a/mailbox/api/src/main/java/org/apache/james/mailbox/model/ParsedAttachment.java b/mailbox/api/src/main/java/org/apache/james/mailbox/model/ParsedAttachment.java
index 429528e..a85dcf9 100644
--- a/mailbox/api/src/main/java/org/apache/james/mailbox/model/ParsedAttachment.java
+++ b/mailbox/api/src/main/java/org/apache/james/mailbox/model/ParsedAttachment.java
@@ -22,6 +22,8 @@ package org.apache.james.mailbox.model;
import java.io.IOException;
import java.util.Optional;
+import com.google.common.io.ByteSource;
+
public class ParsedAttachment {
interface Builder {
@FunctionalInterface
@@ -35,7 +37,7 @@ public class ParsedAttachment {
@FunctionalInterface
interface RequireContent {
- RequireName content(byte[] bytes);
+ RequireName content(ByteSource bytes);
}
@FunctionalInterface
@@ -79,12 +81,12 @@ public class ParsedAttachment {
}
private final ContentType contentType;
- private final byte[] content;
+ private final ByteSource content;
private final Optional<String> name;
private final Optional<Cid> cid;
private final boolean isInline;
- private ParsedAttachment(ContentType contentType, byte[] content, Optional<String> name, Optional<Cid> cid, boolean isInline) {
+ private ParsedAttachment(ContentType contentType, ByteSource content, Optional<String> name, Optional<Cid> cid, boolean isInline) {
this.contentType = contentType;
this.content = content;
this.name = name;
@@ -96,7 +98,7 @@ public class ParsedAttachment {
return contentType;
}
- public byte[] getContent() {
+ public ByteSource getContent() {
return content;
}
@@ -130,7 +132,7 @@ public class ParsedAttachment {
.attachment(AttachmentMetadata.builder()
.attachmentId(attachmentId)
.type(contentType)
- .size(content.length)
+ .size(content.size())
.build())
.name(name)
.cid(cid)
diff --git a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentMapper.java b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentMapper.java
index ce98ec0..a53e4b4 100644
--- a/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentMapper.java
+++ b/mailbox/cassandra/src/main/java/org/apache/james/mailbox/cassandra/mail/CassandraAttachmentMapper.java
@@ -48,6 +48,7 @@ import org.slf4j.LoggerFactory;
import com.github.steveash.guavate.Guavate;
import com.google.common.base.Preconditions;
+import com.google.common.io.ByteSource;
import reactor.core.publisher.Flux;
import reactor.core.publisher.Mono;
@@ -139,12 +140,17 @@ public class CassandraAttachmentMapper implements AttachmentMapper {
}
private Mono<MessageAttachmentMetadata> storeAttachmentAsync(ParsedAttachment parsedAttachment, MessageId ownerMessageId) {
- AttachmentId attachmentId = AttachmentId.random();
- byte[] content = parsedAttachment.getContent();
- return Mono.from(blobStore.save(blobStore.getDefaultBucketName(), content, LOW_COST))
- .map(blobId -> new DAOAttachment(attachmentId, blobId, parsedAttachment.getContentType(), content.length))
- .flatMap(daoAttachment -> storeAttachmentWithIndex(daoAttachment, ownerMessageId))
- .then(Mono.defer(() -> Mono.just(parsedAttachment.asMessageAttachment(attachmentId, content.length))));
+ try {
+ AttachmentId attachmentId = AttachmentId.random();
+ ByteSource content = parsedAttachment.getContent();
+ long size = content.size();
+ return Mono.from(blobStore.save(blobStore.getDefaultBucketName(), content, LOW_COST))
+ .map(blobId -> new DAOAttachment(attachmentId, blobId, parsedAttachment.getContentType(), size))
+ .flatMap(daoAttachment -> storeAttachmentWithIndex(daoAttachment, ownerMessageId))
+ .then(Mono.defer(() -> Mono.just(parsedAttachment.asMessageAttachment(attachmentId, size))));
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
}
private Mono<Void> storeAttachmentWithIndex(DAOAttachment daoAttachment, MessageId ownerMessageId) {
diff --git a/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/mail/InMemoryAttachmentMapper.java b/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/mail/InMemoryAttachmentMapper.java
index 80e0e83..d948199 100644
--- a/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/mail/InMemoryAttachmentMapper.java
+++ b/mailbox/memory/src/main/java/org/apache/james/mailbox/inmemory/mail/InMemoryAttachmentMapper.java
@@ -121,16 +121,19 @@ public class InMemoryAttachmentMapper implements AttachmentMapper {
private MessageAttachmentMetadata storeAttachmentForMessage(MessageId ownerMessageId, ParsedAttachment parsedAttachment) throws MailboxException {
AttachmentId attachmentId = AttachmentId.random();
- byte[] bytes = parsedAttachment.getContent();
-
- attachmentsById.put(attachmentId, AttachmentMetadata.builder()
- .attachmentId(attachmentId)
- .type(parsedAttachment.getContentType())
- .size(bytes.length)
- .build());
- attachmentsRawContentById.put(attachmentId, bytes);
- messageIdsByAttachmentId.put(attachmentId, ownerMessageId);
- return parsedAttachment.asMessageAttachment(attachmentId, bytes.length);
+ try {
+ byte[] bytes = IOUtils.toByteArray(parsedAttachment.getContent().openStream());
+ attachmentsById.put(attachmentId, AttachmentMetadata.builder()
+ .attachmentId(attachmentId)
+ .type(parsedAttachment.getContentType())
+ .size(bytes.length)
+ .build());
+ attachmentsRawContentById.put(attachmentId, bytes);
+ messageIdsByAttachmentId.put(attachmentId, ownerMessageId);
+ return parsedAttachment.asMessageAttachment(attachmentId, bytes.length);
+ } catch (IOException e) {
+ throw new RuntimeException(e);
+ }
}
@Override
diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/model/impl/MessageParser.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/model/impl/MessageParser.java
index 169b0f7..44552b8 100644
--- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/model/impl/MessageParser.java
+++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/mail/model/impl/MessageParser.java
@@ -19,6 +19,7 @@
package org.apache.james.mailbox.store.mail.model.impl;
+import java.io.ByteArrayInputStream;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
@@ -36,6 +37,7 @@ import org.apache.james.mime4j.dom.Body;
import org.apache.james.mime4j.dom.Entity;
import org.apache.james.mime4j.dom.Message;
import org.apache.james.mime4j.dom.Multipart;
+import org.apache.james.mime4j.dom.SingleBody;
import org.apache.james.mime4j.dom.field.ContentDispositionField;
import org.apache.james.mime4j.dom.field.ContentIdField;
import org.apache.james.mime4j.dom.field.ContentTypeField;
@@ -51,6 +53,7 @@ import org.slf4j.LoggerFactory;
import com.github.steveash.guavate.Guavate;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
+import com.google.common.io.ByteSource;
public class MessageParser {
@@ -137,7 +140,19 @@ public class MessageParser {
return ParsedAttachment.builder()
.contentType(contentType.orElse(DEFAULT_CONTENT_TYPE))
- .content(getContent(entity.getBody()))
+ .content(new ByteSource() {
+ @Override
+ public InputStream openStream() throws IOException {
+ Body body = entity.getBody();
+ if (body instanceof SingleBody) {
+ // Avoid copies for BinaryBody / TextBody
+ SingleBody singleBody = (SingleBody) body;
+ return singleBody.getInputStream();
+ }
+ // Fallback to a memory copy
+ return new ByteArrayInputStream(getContent(body));
+ }
+ })
.name(name)
.cid(cid)
.inline(isInline);
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/AttachmentMapperTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/AttachmentMapperTest.java
index 89c9c9c..39b72fa 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/AttachmentMapperTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/AttachmentMapperTest.java
@@ -40,6 +40,7 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import com.google.common.collect.ImmutableList;
+import com.google.common.io.ByteSource;
import reactor.core.publisher.Mono;
@@ -159,7 +160,7 @@ public abstract class AttachmentMapperTest {
MessageId messageId = generateMessageId();
AttachmentId attachmentId = attachmentMapper.storeAttachmentsForMessage(ImmutableList.of(ParsedAttachment.builder()
.contentType("content")
- .content("".getBytes(StandardCharsets.UTF_8))
+ .content(ByteSource.empty())
.noName()
.noCid()
.inline(false)), messageId)
@@ -173,14 +174,14 @@ public abstract class AttachmentMapperTest {
MessageId messageId1 = generateMessageId();
AttachmentId attachmentId1 = attachmentMapper.storeAttachmentsForMessage(ImmutableList.of(ParsedAttachment.builder()
.contentType("content")
- .content("".getBytes(StandardCharsets.UTF_8))
+ .content(ByteSource.wrap("".getBytes(StandardCharsets.UTF_8)))
.noName()
.noCid()
.inline(false)), messageId1)
.get(0).getAttachmentId();
attachmentMapper.storeAttachmentsForMessage(ImmutableList.of(ParsedAttachment.builder()
.contentType("content")
- .content("".getBytes(StandardCharsets.UTF_8))
+ .content(ByteSource.wrap("".getBytes(StandardCharsets.UTF_8)))
.noName()
.noCid()
.inline(false)), generateMessageId())
@@ -218,7 +219,7 @@ public abstract class AttachmentMapperTest {
void getOwnersShouldReturnEmptyWhenMessageIdReferenced() throws Exception {
AttachmentId attachmentId = attachmentMapper.storeAttachmentsForMessage(ImmutableList.of(ParsedAttachment.builder()
.contentType("content")
- .content("".getBytes(StandardCharsets.UTF_8))
+ .content(ByteSource.wrap("".getBytes(StandardCharsets.UTF_8)))
.noName()
.noCid()
.inline(false)), generateMessageId())
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageWithAttachmentMapperTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageWithAttachmentMapperTest.java
index a08cb2b..aef1cbe 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageWithAttachmentMapperTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/MessageWithAttachmentMapperTest.java
@@ -50,6 +50,7 @@ import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import com.google.common.collect.ImmutableList;
+import com.google.common.io.ByteSource;
public abstract class MessageWithAttachmentMapperTest {
@@ -82,19 +83,19 @@ public abstract class MessageWithAttachmentMapperTest {
attachmentsMailbox = createMailbox(MailboxPath.forUser(Username.of("benwa"), "Attachments"));
ParsedAttachment attachment1 = ParsedAttachment.builder()
.contentType("content")
- .content("attachment".getBytes(StandardCharsets.UTF_8))
+ .content(ByteSource.wrap("attachment".getBytes(StandardCharsets.UTF_8)))
.noName()
.cid(Cid.from("cid"))
.inline();
ParsedAttachment attachment2 = ParsedAttachment.builder()
.contentType("content")
- .content("attachment2".getBytes(StandardCharsets.UTF_8))
+ .content(ByteSource.wrap("attachment2".getBytes(StandardCharsets.UTF_8)))
.noName()
.cid(Cid.from("cid"))
.inline();
ParsedAttachment attachment3 = ParsedAttachment.builder()
.contentType("content")
- .content("attachment3".getBytes(StandardCharsets.UTF_8))
+ .content(ByteSource.wrap("attachment3".getBytes(StandardCharsets.UTF_8)))
.noName()
.cid(Cid.from("cid"))
.inline(false);
diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/impl/MessageParserTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/impl/MessageParserTest.java
index 393c673..a1fd7af 100644
--- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/impl/MessageParserTest.java
+++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/mail/model/impl/MessageParserTest.java
@@ -141,7 +141,7 @@ class MessageParserTest {
List<ParsedAttachment> attachments = testee.retrieveAttachments(ClassLoader.getSystemResourceAsStream("eml/oneAttachmentAndSomeTextInlined.eml"));
ParsedAttachment attachment = attachments.get(0);
- assertThat(new ByteArrayInputStream(attachment.getContent()))
+ assertThat(attachment.getContent().openStream())
.hasSameContentAs(ClassLoader.getSystemResourceAsStream("eml/gimp.png"));
}
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org