You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@james.apache.org by GitBox <gi...@apache.org> on 2020/12/11 11:05:25 UTC
[GitHub] [james-project] chibenwa opened a new pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
chibenwa opened a new pull request #280:
URL: https://github.com/apache/james-project/pull/280
Thanks to @jeantil @mbaechler
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #280:
URL: https://github.com/apache/james-project/pull/280#discussion_r542368748
##########
File path: server/mailet/integration-testing/src/test/java/org/apache/james/mailets/MimeMessageBenchTest.java
##########
@@ -0,0 +1,146 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.mailets;
+
+import static org.apache.james.mailets.configuration.Constants.DEFAULT_DOMAIN;
+import static org.apache.james.mailets.configuration.Constants.FROM;
+import static org.apache.james.mailets.configuration.Constants.LOCALHOST_IP;
+import static org.apache.james.mailets.configuration.Constants.PASSWORD;
+import static org.apache.james.mailets.configuration.Constants.awaitAtMostOneMinute;
+
+import java.util.concurrent.TimeUnit;
+import java.util.stream.IntStream;
+
+import org.apache.james.MemoryJamesServerMain;
+import org.apache.james.core.builder.MimeMessageBuilder;
+import org.apache.james.mailets.configuration.CommonProcessors;
+import org.apache.james.mailets.configuration.MailetConfiguration;
+import org.apache.james.mailets.configuration.MailetContainer;
+import org.apache.james.mailets.configuration.ProcessorConfiguration;
+import org.apache.james.modules.protocols.SmtpGuiceProbe;
+import org.apache.james.probe.DataProbe;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.transport.mailets.LogMessage;
+import org.apache.james.transport.matchers.RecipientIs;
+import org.apache.james.utils.DataProbeImpl;
+import org.apache.james.utils.SMTPMessageSender;
+import org.apache.james.utils.SpoolerProbe;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.jupiter.api.Disabled;
+import org.junit.rules.TemporaryFolder;
+
+import com.github.fge.lambdas.Throwing;
+import com.google.common.base.Stopwatch;
+
+/**
+ * This benches the efficiency at which James mailetcontainer splits emails.
+ */
+@Disabled
+public class MimeMessageBenchTest {
+ private static String CONTENT = "0123456789\r\n".repeat(1024 * 10); // 120KB message
+
+ @Rule
+ public TemporaryFolder temporaryFolder = new TemporaryFolder();
+ @Rule
+ public SMTPMessageSender messageSender = new SMTPMessageSender(DEFAULT_DOMAIN);
+
+ private TemporaryJamesServer jamesServer;
+
+ @Before
+ public void setup() throws Exception {
+ jamesServer = TemporaryJamesServer.builder()
+ .withBase(MemoryJamesServerMain.SMTP_AND_IMAP_MODULE)
+ .withMailetContainer(
+ generateMailetContainerConfiguration())
+ .build(temporaryFolder.newFolder());
+ jamesServer.start();
+
+ DataProbe dataProbe = jamesServer.getProbe(DataProbeImpl.class);
+ dataProbe.addDomain(DEFAULT_DOMAIN);
+ dataProbe.addUser("rcpt1@" + DEFAULT_DOMAIN, PASSWORD);
+ dataProbe.addUser("rcpt2@" + DEFAULT_DOMAIN, PASSWORD);
+ dataProbe.addUser("rcpt3@" + DEFAULT_DOMAIN, PASSWORD);
+ dataProbe.addUser("rcpt4@" + DEFAULT_DOMAIN, PASSWORD);
+ dataProbe.addUser("rcpt5@" + DEFAULT_DOMAIN, PASSWORD);
+ }
+
+ @After
+ public void tearDown() {
+ jamesServer.shutdown();
+ }
+
+ @Test
+ public void receivedMessagesShouldContainDeliveredToHeaders() throws Exception {
+ messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort());
+
+ Stopwatch stopwatch = Stopwatch.createStarted();
Review comment:
Thanks! Junit integration sounds really easy to use.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] mbaechler commented on pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
mbaechler commented on pull request #280:
URL: https://github.com/apache/james-project/pull/280#issuecomment-745275571
BTW, your JMH work is awesome
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #280:
URL: https://github.com/apache/james-project/pull/280#issuecomment-751318609
Let's drop the COW.
See https://github.com/apache/james-project/pull/282
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #280:
URL: https://github.com/apache/james-project/pull/280#discussion_r542366228
##########
File path: server/mailet/integration-testing/src/test/java/org/apache/james/mailets/MimeMessageBenchTest.java
##########
@@ -0,0 +1,146 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.mailets;
+
+import static org.apache.james.mailets.configuration.Constants.DEFAULT_DOMAIN;
+import static org.apache.james.mailets.configuration.Constants.FROM;
+import static org.apache.james.mailets.configuration.Constants.LOCALHOST_IP;
+import static org.apache.james.mailets.configuration.Constants.PASSWORD;
+import static org.apache.james.mailets.configuration.Constants.awaitAtMostOneMinute;
+
+import java.util.concurrent.TimeUnit;
+import java.util.stream.IntStream;
+
+import org.apache.james.MemoryJamesServerMain;
+import org.apache.james.core.builder.MimeMessageBuilder;
+import org.apache.james.mailets.configuration.CommonProcessors;
+import org.apache.james.mailets.configuration.MailetConfiguration;
+import org.apache.james.mailets.configuration.MailetContainer;
+import org.apache.james.mailets.configuration.ProcessorConfiguration;
+import org.apache.james.modules.protocols.SmtpGuiceProbe;
+import org.apache.james.probe.DataProbe;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.transport.mailets.LogMessage;
+import org.apache.james.transport.matchers.RecipientIs;
+import org.apache.james.utils.DataProbeImpl;
+import org.apache.james.utils.SMTPMessageSender;
+import org.apache.james.utils.SpoolerProbe;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.jupiter.api.Disabled;
+import org.junit.rules.TemporaryFolder;
+
+import com.github.fge.lambdas.Throwing;
+import com.google.common.base.Stopwatch;
+
+/**
+ * This benches the efficiency at which James mailetcontainer splits emails.
+ */
+@Disabled
+public class MimeMessageBenchTest {
+ private static String CONTENT = "0123456789\r\n".repeat(1024 * 10); // 120KB message
+
+ @Rule
+ public TemporaryFolder temporaryFolder = new TemporaryFolder();
+ @Rule
+ public SMTPMessageSender messageSender = new SMTPMessageSender(DEFAULT_DOMAIN);
+
+ private TemporaryJamesServer jamesServer;
+
+ @Before
+ public void setup() throws Exception {
+ jamesServer = TemporaryJamesServer.builder()
+ .withBase(MemoryJamesServerMain.SMTP_AND_IMAP_MODULE)
+ .withMailetContainer(
+ generateMailetContainerConfiguration())
+ .build(temporaryFolder.newFolder());
+ jamesServer.start();
+
+ DataProbe dataProbe = jamesServer.getProbe(DataProbeImpl.class);
+ dataProbe.addDomain(DEFAULT_DOMAIN);
+ dataProbe.addUser("rcpt1@" + DEFAULT_DOMAIN, PASSWORD);
+ dataProbe.addUser("rcpt2@" + DEFAULT_DOMAIN, PASSWORD);
+ dataProbe.addUser("rcpt3@" + DEFAULT_DOMAIN, PASSWORD);
+ dataProbe.addUser("rcpt4@" + DEFAULT_DOMAIN, PASSWORD);
+ dataProbe.addUser("rcpt5@" + DEFAULT_DOMAIN, PASSWORD);
+ }
+
+ @After
+ public void tearDown() {
+ jamesServer.shutdown();
+ }
+
+ @Test
+ public void receivedMessagesShouldContainDeliveredToHeaders() throws Exception {
+ messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort());
+
+ Stopwatch stopwatch = Stopwatch.createStarted();
+ IntStream.range(0, 100)
+ .forEach(Throwing.intConsumer(i -> messageSender.sendMessage(MailImpl.builder()
+ .name("name" + i)
+ .sender(FROM)
+ .addRecipients("rcpt1@" + DEFAULT_DOMAIN,
+ "rcpt2@" + DEFAULT_DOMAIN,
+ "rcpt3@" + DEFAULT_DOMAIN,
+ "rcpt4@" + DEFAULT_DOMAIN,
+ "rcpt5@" + DEFAULT_DOMAIN)
+ .mimeMessage(MimeMessageBuilder.mimeMessageBuilder()
+ .setSubject("subject i")
+ .setText(CONTENT))
+ .build())));
+
+ awaitAtMostOneMinute.until(() -> jamesServer.getProbe(SpoolerProbe.class).processingFinished());
+ System.out.println("Spent: " + stopwatch.elapsed(TimeUnit.MILLISECONDS));
+ }
+
+
+ private MailetContainer.Builder generateMailetContainerConfiguration() {
+ return TemporaryJamesServer.defaultMailetContainerConfiguration()
+ .putProcessor(ProcessorConfiguration.transport()
+ .addMailet(MailetConfiguration.builder()
+ .matcher(RecipientIs.class)
+ .matcherCondition("rcpt1@" + DEFAULT_DOMAIN)
+ .mailet(LogMessage.class)
+ .addProperty("passThrough", "true"))
+ .addMailet(MailetConfiguration.builder()
+ .matcher(RecipientIs.class)
+ .matcherCondition("rcpt2@" + DEFAULT_DOMAIN)
+ .mailet(LogMessage.class)
+ .addProperty("passThrough", "true"))
+ .addMailet(MailetConfiguration.builder()
+ .matcher(RecipientIs.class)
+ .matcherCondition("rcpt3@" + DEFAULT_DOMAIN)
+ .mailet(LogMessage.class)
+ .addProperty("passThrough", "true"))
+ .addMailet(MailetConfiguration.builder()
+ .matcher(RecipientIs.class)
+ .matcherCondition("rcpt4@" + DEFAULT_DOMAIN)
+ .mailet(LogMessage.class)
+ .addProperty("passThrough", "true"))
+ .addMailet(MailetConfiguration.builder()
+ .matcher(RecipientIs.class)
+ .matcherCondition("rcpt5@" + DEFAULT_DOMAIN)
+ .mailet(LogMessage.class)
+ .addProperty("passThrough", "true"))
+ .addMailetsFrom(CommonProcessors.deliverOnlyTransport()));
Review comment:
If "writes" are performed, then content are not shareable anymore, and copy will happen anyway with the cow proxy.
Doing only reads, where things can be shared, and avoiding copy, was my intention...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] mbaechler commented on pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
mbaechler commented on pull request #280:
URL: https://github.com/apache/james-project/pull/280#issuecomment-745275289
AFAICT, "always copy" is always faster. GC time is always comparable.
Did you read the right column?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #280:
URL: https://github.com/apache/james-project/pull/280#issuecomment-744378892
Add a quick 'toy' micro-benchmark for evaluating mailetcon…
32d1fca
…tainer efficiency
On top of master: 16.691 ms
On top of the proposed changes: 17.126 ms
With "always copy" behavior: 15.442 ms
Implementation of "always copy" behavior:
```
private MimeMessageCopyOnWriteProxy(MimeMessage original, boolean originalIsAReference) {
super(Session.getDefaultInstance(System.getProperties(), null));
try {
refCount = new MessageReferenceTracker(new MimeMessageWrapper(original));
} catch (MessagingException e) {
throw new RuntimeException(e);
}
}
```
Performance gains of copy-on-write proxy are not obvious...
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #280:
URL: https://github.com/apache/james-project/pull/280#issuecomment-748935269
> I know it's Christmas time and all so no hurry :)
Not in VN, but avoid disturbance during new year ;-)
I would say:
- 1. Have a always-copy viable PR (that equals to MimeCow stuff removal)
- 2. Have a bench on real Mail Queue implems (today & always-copy) - the memory mail queue does extra copies.
We may also want to bring the topic back on the mailing list now that we have gained more insight.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] jeantil commented on a change in pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
jeantil commented on a change in pull request #280:
URL: https://github.com/apache/james-project/pull/280#discussion_r542328329
##########
File path: server/mailet/integration-testing/src/test/java/org/apache/james/mailets/MimeMessageBenchTest.java
##########
@@ -0,0 +1,146 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.mailets;
+
+import static org.apache.james.mailets.configuration.Constants.DEFAULT_DOMAIN;
+import static org.apache.james.mailets.configuration.Constants.FROM;
+import static org.apache.james.mailets.configuration.Constants.LOCALHOST_IP;
+import static org.apache.james.mailets.configuration.Constants.PASSWORD;
+import static org.apache.james.mailets.configuration.Constants.awaitAtMostOneMinute;
+
+import java.util.concurrent.TimeUnit;
+import java.util.stream.IntStream;
+
+import org.apache.james.MemoryJamesServerMain;
+import org.apache.james.core.builder.MimeMessageBuilder;
+import org.apache.james.mailets.configuration.CommonProcessors;
+import org.apache.james.mailets.configuration.MailetConfiguration;
+import org.apache.james.mailets.configuration.MailetContainer;
+import org.apache.james.mailets.configuration.ProcessorConfiguration;
+import org.apache.james.modules.protocols.SmtpGuiceProbe;
+import org.apache.james.probe.DataProbe;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.transport.mailets.LogMessage;
+import org.apache.james.transport.matchers.RecipientIs;
+import org.apache.james.utils.DataProbeImpl;
+import org.apache.james.utils.SMTPMessageSender;
+import org.apache.james.utils.SpoolerProbe;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.jupiter.api.Disabled;
+import org.junit.rules.TemporaryFolder;
+
+import com.github.fge.lambdas.Throwing;
+import com.google.common.base.Stopwatch;
+
+/**
+ * This benches the efficiency at which James mailetcontainer splits emails.
+ */
+@Disabled
+public class MimeMessageBenchTest {
+ private static String CONTENT = "0123456789\r\n".repeat(1024 * 10); // 120KB message
+
+ @Rule
+ public TemporaryFolder temporaryFolder = new TemporaryFolder();
+ @Rule
+ public SMTPMessageSender messageSender = new SMTPMessageSender(DEFAULT_DOMAIN);
+
+ private TemporaryJamesServer jamesServer;
+
+ @Before
+ public void setup() throws Exception {
+ jamesServer = TemporaryJamesServer.builder()
+ .withBase(MemoryJamesServerMain.SMTP_AND_IMAP_MODULE)
+ .withMailetContainer(
+ generateMailetContainerConfiguration())
+ .build(temporaryFolder.newFolder());
+ jamesServer.start();
+
+ DataProbe dataProbe = jamesServer.getProbe(DataProbeImpl.class);
+ dataProbe.addDomain(DEFAULT_DOMAIN);
+ dataProbe.addUser("rcpt1@" + DEFAULT_DOMAIN, PASSWORD);
+ dataProbe.addUser("rcpt2@" + DEFAULT_DOMAIN, PASSWORD);
+ dataProbe.addUser("rcpt3@" + DEFAULT_DOMAIN, PASSWORD);
+ dataProbe.addUser("rcpt4@" + DEFAULT_DOMAIN, PASSWORD);
+ dataProbe.addUser("rcpt5@" + DEFAULT_DOMAIN, PASSWORD);
+ }
+
+ @After
+ public void tearDown() {
+ jamesServer.shutdown();
+ }
+
+ @Test
+ public void receivedMessagesShouldContainDeliveredToHeaders() throws Exception {
+ messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort());
+
+ Stopwatch stopwatch = Stopwatch.createStarted();
Review comment:
I think it would be worth it to investigate jmh microbenchmarks in the future. Some useful resources:
- https://github.com/openjdk/jmh
- https://stackoverflow.com/questions/30485856/how-to-run-jmh-from-inside-junit-tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #280:
URL: https://github.com/apache/james-project/pull/280#issuecomment-745701937
BTW here is a run for 10MB
```
Scenario: Startup + 3 messages for 5 rcpts, 10MB each + Shutdown
Code version: Locking version of COW
Benchmark Mode Cnt Score Error Units
MimeMessageBenchTest.benchmark1 avgt 3 49901.215 ± 92917.167 ms/op
MimeMessageBenchTest.benchmark1:·gc.alloc.rate avgt 3 85.808 ± 156.009 MB/sec
MimeMessageBenchTest.benchmark1:·gc.churn.G1_Eden_Space avgt 3 511.349 ± 977.955 MB/sec
MimeMessageBenchTest.benchmark1:·gc.churn.G1_Old_Gen avgt 3 76.985 ± 165.456 MB/sec
MimeMessageBenchTest.benchmark1:·gc.churn.G1_Survivor_Space avgt 3 2.232 ± 13.451 MB/sec
MimeMessageBenchTest.benchmark1:·gc.count avgt 3 549.000 counts
MimeMessageBenchTest.benchmark1:·gc.time avgt 3 8716.000 ms
Scenario: Startup + 3 messages for 5 rcpts, 10MB each + Shutdown
Code version: Always COPY
Benchmark Mode Cnt Score Error Units
MimeMessageBenchTest.benchmark1 avgt 3 55688.810 ± 163013.521 ms/op
MimeMessageBenchTest.benchmark1:·gc.alloc.rate avgt 3 77.816 ± 207.068 MB/sec
MimeMessageBenchTest.benchmark1:·gc.churn.G1_Eden_Space avgt 3 462.497 ± 1271.321 MB/sec
MimeMessageBenchTest.benchmark1:·gc.churn.G1_Old_Gen avgt 3 70.097 ± 189.172 MB/sec
MimeMessageBenchTest.benchmark1:·gc.churn.G1_Survivor_Space avgt 3 1.939 ± 13.101 MB/sec
MimeMessageBenchTest.benchmark1:·gc.count avgt 3 558.000 counts
MimeMessageBenchTest.benchmark1:·gc.time avgt 3 9021.000 ms
```
Maybe we should plan running similar bench on real implems, not just memory, before taking decisions?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] jeantil commented on a change in pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
jeantil commented on a change in pull request #280:
URL: https://github.com/apache/james-project/pull/280#discussion_r542329956
##########
File path: server/mailet/integration-testing/src/test/java/org/apache/james/mailets/MimeMessageBenchTest.java
##########
@@ -0,0 +1,146 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.mailets;
+
+import static org.apache.james.mailets.configuration.Constants.DEFAULT_DOMAIN;
+import static org.apache.james.mailets.configuration.Constants.FROM;
+import static org.apache.james.mailets.configuration.Constants.LOCALHOST_IP;
+import static org.apache.james.mailets.configuration.Constants.PASSWORD;
+import static org.apache.james.mailets.configuration.Constants.awaitAtMostOneMinute;
+
+import java.util.concurrent.TimeUnit;
+import java.util.stream.IntStream;
+
+import org.apache.james.MemoryJamesServerMain;
+import org.apache.james.core.builder.MimeMessageBuilder;
+import org.apache.james.mailets.configuration.CommonProcessors;
+import org.apache.james.mailets.configuration.MailetConfiguration;
+import org.apache.james.mailets.configuration.MailetContainer;
+import org.apache.james.mailets.configuration.ProcessorConfiguration;
+import org.apache.james.modules.protocols.SmtpGuiceProbe;
+import org.apache.james.probe.DataProbe;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.transport.mailets.LogMessage;
+import org.apache.james.transport.matchers.RecipientIs;
+import org.apache.james.utils.DataProbeImpl;
+import org.apache.james.utils.SMTPMessageSender;
+import org.apache.james.utils.SpoolerProbe;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.jupiter.api.Disabled;
+import org.junit.rules.TemporaryFolder;
+
+import com.github.fge.lambdas.Throwing;
+import com.google.common.base.Stopwatch;
+
+/**
+ * This benches the efficiency at which James mailetcontainer splits emails.
+ */
+@Disabled
+public class MimeMessageBenchTest {
+ private static String CONTENT = "0123456789\r\n".repeat(1024 * 10); // 120KB message
+
+ @Rule
+ public TemporaryFolder temporaryFolder = new TemporaryFolder();
+ @Rule
+ public SMTPMessageSender messageSender = new SMTPMessageSender(DEFAULT_DOMAIN);
+
+ private TemporaryJamesServer jamesServer;
+
+ @Before
+ public void setup() throws Exception {
+ jamesServer = TemporaryJamesServer.builder()
+ .withBase(MemoryJamesServerMain.SMTP_AND_IMAP_MODULE)
+ .withMailetContainer(
+ generateMailetContainerConfiguration())
+ .build(temporaryFolder.newFolder());
+ jamesServer.start();
+
+ DataProbe dataProbe = jamesServer.getProbe(DataProbeImpl.class);
+ dataProbe.addDomain(DEFAULT_DOMAIN);
+ dataProbe.addUser("rcpt1@" + DEFAULT_DOMAIN, PASSWORD);
+ dataProbe.addUser("rcpt2@" + DEFAULT_DOMAIN, PASSWORD);
+ dataProbe.addUser("rcpt3@" + DEFAULT_DOMAIN, PASSWORD);
+ dataProbe.addUser("rcpt4@" + DEFAULT_DOMAIN, PASSWORD);
+ dataProbe.addUser("rcpt5@" + DEFAULT_DOMAIN, PASSWORD);
+ }
+
+ @After
+ public void tearDown() {
+ jamesServer.shutdown();
+ }
+
+ @Test
+ public void receivedMessagesShouldContainDeliveredToHeaders() throws Exception {
+ messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort());
+
+ Stopwatch stopwatch = Stopwatch.createStarted();
+ IntStream.range(0, 100)
+ .forEach(Throwing.intConsumer(i -> messageSender.sendMessage(MailImpl.builder()
+ .name("name" + i)
+ .sender(FROM)
+ .addRecipients("rcpt1@" + DEFAULT_DOMAIN,
+ "rcpt2@" + DEFAULT_DOMAIN,
+ "rcpt3@" + DEFAULT_DOMAIN,
+ "rcpt4@" + DEFAULT_DOMAIN,
+ "rcpt5@" + DEFAULT_DOMAIN)
+ .mimeMessage(MimeMessageBuilder.mimeMessageBuilder()
+ .setSubject("subject i")
+ .setText(CONTENT))
+ .build())));
+
+ awaitAtMostOneMinute.until(() -> jamesServer.getProbe(SpoolerProbe.class).processingFinished());
+ System.out.println("Spent: " + stopwatch.elapsed(TimeUnit.MILLISECONDS));
+ }
+
+
+ private MailetContainer.Builder generateMailetContainerConfiguration() {
+ return TemporaryJamesServer.defaultMailetContainerConfiguration()
+ .putProcessor(ProcessorConfiguration.transport()
+ .addMailet(MailetConfiguration.builder()
+ .matcher(RecipientIs.class)
+ .matcherCondition("rcpt1@" + DEFAULT_DOMAIN)
+ .mailet(LogMessage.class)
+ .addProperty("passThrough", "true"))
+ .addMailet(MailetConfiguration.builder()
+ .matcher(RecipientIs.class)
+ .matcherCondition("rcpt2@" + DEFAULT_DOMAIN)
+ .mailet(LogMessage.class)
+ .addProperty("passThrough", "true"))
+ .addMailet(MailetConfiguration.builder()
+ .matcher(RecipientIs.class)
+ .matcherCondition("rcpt3@" + DEFAULT_DOMAIN)
+ .mailet(LogMessage.class)
+ .addProperty("passThrough", "true"))
+ .addMailet(MailetConfiguration.builder()
+ .matcher(RecipientIs.class)
+ .matcherCondition("rcpt4@" + DEFAULT_DOMAIN)
+ .mailet(LogMessage.class)
+ .addProperty("passThrough", "true"))
+ .addMailet(MailetConfiguration.builder()
+ .matcher(RecipientIs.class)
+ .matcherCondition("rcpt5@" + DEFAULT_DOMAIN)
+ .mailet(LogMessage.class)
+ .addProperty("passThrough", "true"))
+ .addMailetsFrom(CommonProcessors.deliverOnlyTransport()));
Review comment:
while this configuration does split message, as far as I can tell, none of the mailets attempt to concurrenlty "write" to the message, hence the exercised concurrency is actually pretty low.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #280:
URL: https://github.com/apache/james-project/pull/280#issuecomment-744292453
> I know all the effort that has been invested in all this, but I would very much like to see the benchmark that led to implement this copy on write scheme instead of simply copying on every instantiation.
:+1: we need to know if it worth it
We need to see if this code complexity is worth it (I am not super fan of the proposed solution here that partially resolves the problem)
> It feels like trying to preserver heap space at the cost of synchronization this is optimization territory and should be backed by measurements that are kept up to date for new releases.
We are trying to avoid disk writes that could happen from this copy from what I understand.
> I wonder what the perf profile of the server would look like with a more naive copy approach.
We need to design a test, where emails are actually split (matcher splitting leads to several copies being hold)
We could do micro benchmark of 1000 processed emails of 100KB split 5 time against both implems. What would you think ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] jeantil commented on a change in pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
jeantil commented on a change in pull request #280:
URL: https://github.com/apache/james-project/pull/280#discussion_r542206112
##########
File path: server/container/core/src/main/java/org/apache/james/server/core/MimeMessageCopyOnWriteProxy.java
##########
@@ -67,98 +93,208 @@ public MessageReferenceTracker(MimeMessage ref) {
wrapped = ref;
}
- protected synchronized void incrementReferenceCount() {
- referenceCount++;
+ protected void dispose() {
+ ReentrantReadWriteLock.WriteLock writeLock = lock.writeLock();
+ writeLock.lock();
+ try {
+ referenceCount--;
+ if (referenceCount <= 0) {
+ LifecycleUtil.dispose(wrapped);
+ wrapped = null;
+ }
+ } finally {
+ writeLock.unlock();
+ }
}
- protected synchronized void decrementReferenceCount() {
- referenceCount--;
- if (referenceCount <= 0) {
- LifecycleUtil.dispose(wrapped);
- wrapped = null;
+ protected void incrementReferences() {
+ ReentrantReadWriteLock.WriteLock writeLock = lock.writeLock();
+ writeLock.lock();
+ try {
+ referenceCount++;
Review comment:
out of curiosity: what meaningful exception could possibly be thrown here ?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] jeantil commented on a change in pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
jeantil commented on a change in pull request #280:
URL: https://github.com/apache/james-project/pull/280#discussion_r542320585
##########
File path: server/mailet/integration-testing/src/test/java/org/apache/james/mailets/MimeMessageBenchTest.java
##########
@@ -0,0 +1,146 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.mailets;
+
+import static org.apache.james.mailets.configuration.Constants.DEFAULT_DOMAIN;
+import static org.apache.james.mailets.configuration.Constants.FROM;
+import static org.apache.james.mailets.configuration.Constants.LOCALHOST_IP;
+import static org.apache.james.mailets.configuration.Constants.PASSWORD;
+import static org.apache.james.mailets.configuration.Constants.awaitAtMostOneMinute;
+
+import java.util.concurrent.TimeUnit;
+import java.util.stream.IntStream;
+
+import org.apache.james.MemoryJamesServerMain;
+import org.apache.james.core.builder.MimeMessageBuilder;
+import org.apache.james.mailets.configuration.CommonProcessors;
+import org.apache.james.mailets.configuration.MailetConfiguration;
+import org.apache.james.mailets.configuration.MailetContainer;
+import org.apache.james.mailets.configuration.ProcessorConfiguration;
+import org.apache.james.modules.protocols.SmtpGuiceProbe;
+import org.apache.james.probe.DataProbe;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.transport.mailets.LogMessage;
+import org.apache.james.transport.matchers.RecipientIs;
+import org.apache.james.utils.DataProbeImpl;
+import org.apache.james.utils.SMTPMessageSender;
+import org.apache.james.utils.SpoolerProbe;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.jupiter.api.Disabled;
+import org.junit.rules.TemporaryFolder;
+
+import com.github.fge.lambdas.Throwing;
+import com.google.common.base.Stopwatch;
+
+/**
+ * This benches the efficiency at which James mailetcontainer splits emails.
+ */
+@Disabled
+public class MimeMessageBenchTest {
+ private static String CONTENT = "0123456789\r\n".repeat(1024 * 10); // 120KB message
Review comment:
I would recommend using random contents, the JVM is really good at optimizing things that always look the same
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #280:
URL: https://github.com/apache/james-project/pull/280#issuecomment-745211736
I played a bit with JMH, garbage collector reports & message size...
```
Scenario: Startup + 100 messages for 5 rcpts, 12KB each + Shutdown
Code version: Locking version of COW
Benchmark Mode Cnt Score Error Units
MimeMessageBenchTest.benchmark1 avgt 10 5954.512 ± 110.288 ms/op
MimeMessageBenchTest.benchmark1:·gc.alloc.rate avgt 10 116.627 ± 1.936 MB/sec
MimeMessageBenchTest.benchmark1:·gc.churn.G1_Eden_Space avgt 10 280.591 ± 4.747 MB/sec
MimeMessageBenchTest.benchmark1:·gc.churn.G1_Old_Gen avgt 10 0.171 ± 0.098 MB/sec
MimeMessageBenchTest.benchmark1:·gc.count avgt 10 60.000 counts
MimeMessageBenchTest.benchmark1:·gc.time avgt 10 302.000 ms
Scenario: Startup + 100 messages for 5 rcpts, 12KB each + Shutdown
Code version: Always COPY
Benchmark Mode Cnt Score Error Units
MimeMessageBenchTest.benchmark1 avgt 10 6024.282 ± 130.217 ms/op
MimeMessageBenchTest.benchmark1:·gc.alloc.rate avgt 10 115.991 ± 2.242 MB/sec
MimeMessageBenchTest.benchmark1:·gc.churn.G1_Eden_Space avgt 10 277.597 ± 5.493 MB/sec
MimeMessageBenchTest.benchmark1:·gc.churn.G1_Old_Gen avgt 10 0.178 ± 0.089 MB/sec
MimeMessageBenchTest.benchmark1:·gc.count avgt 10 60.000 counts
MimeMessageBenchTest.benchmark1:·gc.time avgt 10 298.000 ms
Scenario: Startup + 100 messages for 5 rcpts, 120KB each + Shutdown
Code version: Locking version of COW
Benchmark Mode Cnt Score Error Units
MimeMessageBenchTest.benchmark1 avgt 10 11838.707 ± 1909.568 ms/op
MimeMessageBenchTest.benchmark1:·gc.alloc.rate avgt 10 167.785 ± 22.464 MB/sec
MimeMessageBenchTest.benchmark1:·gc.churn.G1_Eden_Space avgt 10 1059.389 ± 154.302 MB/sec
MimeMessageBenchTest.benchmark1:·gc.churn.G1_Old_Gen avgt 10 0.583 ± 0.915 MB/sec
MimeMessageBenchTest.benchmark1:·gc.churn.G1_Survivor_Space avgt 10 3.754 ± 3.913 MB/sec
MimeMessageBenchTest.benchmark1:·gc.count avgt 10 1179.000 counts
MimeMessageBenchTest.benchmark1:·gc.time avgt 10 5129.000 ms
Scenario: Startup + 100 messages for 5 rcpts, 120KB each + Shutdown
Code version: Always COPY
MimeMessageBenchTest.benchmark1 avgt 10 12177.190 ± 1711.754 ms/op
MimeMessageBenchTest.benchmark1:·gc.alloc.rate avgt 10 164.735 ± 20.484 MB/sec
MimeMessageBenchTest.benchmark1:·gc.churn.G1_Eden_Space avgt 10 1046.940 ± 134.789 MB/sec
MimeMessageBenchTest.benchmark1:·gc.churn.G1_Old_Gen avgt 10 0.586 ± 1.042 MB/sec
MimeMessageBenchTest.benchmark1:·gc.churn.G1_Survivor_Space avgt 10 3.900 ± 3.591 MB/sec
MimeMessageBenchTest.benchmark1:·gc.count avgt 10 1202.000 counts
MimeMessageBenchTest.benchmark1:·gc.time avgt 10 5273.000 ms
Scenario: Startup + 10 messages for 5 rcpts, 1.2MB each + Shutdown
Code version: Locking version of COW
Benchmark Mode Cnt Score Error Units
MimeMessageBenchTest.benchmark1 avgt 3 12911.341 ± 3552.079 ms/op
MimeMessageBenchTest.benchmark1:·gc.alloc.rate avgt 3 135.412 ± 34.923 MB/sec
MimeMessageBenchTest.benchmark1:·gc.churn.G1_Eden_Space avgt 3 799.061 ± 217.977 MB/sec
MimeMessageBenchTest.benchmark1:·gc.churn.G1_Old_Gen avgt 3 173.397 ± 76.272 MB/sec
MimeMessageBenchTest.benchmark1:·gc.churn.G1_Survivor_Space avgt 3 9.891 ± 2.784 MB/sec
MimeMessageBenchTest.benchmark1:·gc.count avgt 3 228.000 counts
MimeMessageBenchTest.benchmark1:·gc.time avgt 3 3845.000 ms
Scenario: Startup + 10 messages for 5 rcpts, 1.2MB each + Shutdown
Code version: Always COPY
Benchmark Mode Cnt Score Error Units
MimeMessageBenchTest.benchmark1 avgt 3 13718.479 ± 11832.688 ms/op
MimeMessageBenchTest.benchmark1:·gc.alloc.rate avgt 3 128.467 ± 101.018 MB/sec
MimeMessageBenchTest.benchmark1:·gc.churn.G1_Eden_Space avgt 3 752.122 ± 633.602 MB/sec
MimeMessageBenchTest.benchmark1:·gc.churn.G1_Old_Gen avgt 3 163.080 ± 175.520 MB/sec
MimeMessageBenchTest.benchmark1:·gc.churn.G1_Survivor_Space avgt 3 9.312 ± 4.526 MB/sec
MimeMessageBenchTest.benchmark1:·gc.count avgt 3 229.000 counts
MimeMessageBenchTest.benchmark1:·gc.time avgt 3 3990.000 ms
```
Conclusion:
- Always copy generates slightly more GC
- Runs sligthly slower
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #280:
URL: https://github.com/apache/james-project/pull/280#issuecomment-744293616
(for the record I see the double lock solution more as a personal challenge than necessary a long term solution)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #280:
URL: https://github.com/apache/james-project/pull/280#issuecomment-745690912
> AFAICT, "always copy" is always faster. GC time is always comparable. Did you read the right column?
I do not understand...
13718ms VS 12911ms, 12177 ms vs 11838ms & 6024 VS 5954ms => Always copy is always slightly slower (3% - 1%)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] jeantil commented on pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
jeantil commented on pull request #280:
URL: https://github.com/apache/james-project/pull/280#issuecomment-748848156
Hello,
I know it's Christmas time and all so no hurry :)
I am pretty confident that the COW currently on master has a very negative impact on the tests on the CI. It also sounds like a critical issue for production systems (since the failures exhibit total dataloss for incoming emails after they have been successfully accepted by the server ). So I would like to know how this PR or a similar fix can proceed ?
What I understand is we have to choose between :
- an always copy implementation which has a much higher GC alloc rate for larger mails (perfectly understandable since it copies the whole mail) and seems to be quite a bit slower for large mails >10MB
- A COW version based on ReentrantLock, with more complex code but better performance in the benchmarks.
My personal preference would be for the always copy because the code is simpler and it feels safer even though it is slower in micro benchmarks. But my opinion doesn't matter much and all I really care about is a safe implementation :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #280:
URL: https://github.com/apache/james-project/pull/280#issuecomment-751957047
I started a mailig list discussion https://www.mail-archive.com/server-dev@james.apache.org/msg69361.html
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] Arsnael commented on a change in pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
Arsnael commented on a change in pull request #280:
URL: https://github.com/apache/james-project/pull/280#discussion_r542099368
##########
File path: server/container/core/src/main/java/org/apache/james/server/core/MimeMessageCopyOnWriteProxy.java
##########
@@ -67,98 +92,208 @@ public MessageReferenceTracker(MimeMessage ref) {
wrapped = ref;
}
- protected synchronized void incrementReferenceCount() {
- referenceCount++;
+ protected void dispose() {
+ ReentrantReadWriteLock.WriteLock lock = this.lock.writeLock();
+ lock.lock();
+ try {
+ referenceCount--;
+ if (referenceCount <= 0) {
+ LifecycleUtil.dispose(wrapped);
+ wrapped = null;
+ }
+ } finally {
+ lock.unlock();
+ }
}
- protected synchronized void decrementReferenceCount() {
- referenceCount--;
- if (referenceCount <= 0) {
- LifecycleUtil.dispose(wrapped);
- wrapped = null;
+ protected void incrementReferences() {
+ ReentrantReadWriteLock.WriteLock lock = this.lock.writeLock();
Review comment:
idem
##########
File path: server/container/core/src/main/java/org/apache/james/server/core/MimeMessageCopyOnWriteProxy.java
##########
@@ -67,98 +92,208 @@ public MessageReferenceTracker(MimeMessage ref) {
wrapped = ref;
}
- protected synchronized void incrementReferenceCount() {
- referenceCount++;
+ protected void dispose() {
+ ReentrantReadWriteLock.WriteLock lock = this.lock.writeLock();
Review comment:
I know this is code correct, but maybe I would rename the local variable `writeLock` to avoid any confusion with the class variable?
##########
File path: server/container/core/src/main/java/org/apache/james/server/core/MimeMessageCopyOnWriteProxy.java
##########
@@ -67,98 +92,208 @@ public MessageReferenceTracker(MimeMessage ref) {
wrapped = ref;
}
- protected synchronized void incrementReferenceCount() {
- referenceCount++;
+ protected void dispose() {
+ ReentrantReadWriteLock.WriteLock lock = this.lock.writeLock();
+ lock.lock();
+ try {
+ referenceCount--;
+ if (referenceCount <= 0) {
+ LifecycleUtil.dispose(wrapped);
+ wrapped = null;
+ }
+ } finally {
+ lock.unlock();
+ }
}
- protected synchronized void decrementReferenceCount() {
- referenceCount--;
- if (referenceCount <= 0) {
- LifecycleUtil.dispose(wrapped);
- wrapped = null;
+ protected void incrementReferences() {
+ ReentrantReadWriteLock.WriteLock lock = this.lock.writeLock();
+ lock.lock();
+ try {
+ referenceCount++;
+ } finally {
+ lock.unlock();
}
}
- protected synchronized int getReferenceCount() {
- return referenceCount;
+ protected <T> T wrapRead(Read<T> op) throws MessagingException {
+ ReentrantReadWriteLock.ReadLock lock = this.lock.readLock();
+ lock.lock();
+ try {
+ Preconditions.checkState(referenceCount > 0, "Attempt to read a disposed message");
+ return op.read(wrapped);
+ } finally {
+ lock.unlock();
+ }
}
- public synchronized MimeMessage getWrapped() {
- return wrapped;
+ protected <T> T wrapReadIO(ReadIO<T> op) throws MessagingException, IOException {
+ ReentrantReadWriteLock.ReadLock lock = this.lock.readLock();
+ lock.lock();
+ try {
+ Preconditions.checkState(referenceCount > 0, "Attempt to read a disposed message");
+ return op.read(wrapped);
+ } finally {
+ lock.unlock();
+ }
}
- }
+ protected <T> T wrapReadNoException(Function<MimeMessage, T> op) {
+ ReentrantReadWriteLock.ReadLock lock = this.lock.readLock();
Review comment:
idem
##########
File path: server/container/core/src/main/java/org/apache/james/server/core/MimeMessageCopyOnWriteProxy.java
##########
@@ -67,98 +92,208 @@ public MessageReferenceTracker(MimeMessage ref) {
wrapped = ref;
}
- protected synchronized void incrementReferenceCount() {
- referenceCount++;
+ protected void dispose() {
+ ReentrantReadWriteLock.WriteLock lock = this.lock.writeLock();
+ lock.lock();
+ try {
+ referenceCount--;
+ if (referenceCount <= 0) {
+ LifecycleUtil.dispose(wrapped);
+ wrapped = null;
+ }
+ } finally {
+ lock.unlock();
+ }
}
- protected synchronized void decrementReferenceCount() {
- referenceCount--;
- if (referenceCount <= 0) {
- LifecycleUtil.dispose(wrapped);
- wrapped = null;
+ protected void incrementReferences() {
+ ReentrantReadWriteLock.WriteLock lock = this.lock.writeLock();
+ lock.lock();
+ try {
+ referenceCount++;
+ } finally {
+ lock.unlock();
}
}
- protected synchronized int getReferenceCount() {
- return referenceCount;
+ protected <T> T wrapRead(Read<T> op) throws MessagingException {
+ ReentrantReadWriteLock.ReadLock lock = this.lock.readLock();
+ lock.lock();
+ try {
+ Preconditions.checkState(referenceCount > 0, "Attempt to read a disposed message");
+ return op.read(wrapped);
+ } finally {
+ lock.unlock();
+ }
}
- public synchronized MimeMessage getWrapped() {
- return wrapped;
+ protected <T> T wrapReadIO(ReadIO<T> op) throws MessagingException, IOException {
+ ReentrantReadWriteLock.ReadLock lock = this.lock.readLock();
Review comment:
idem
##########
File path: server/container/core/src/main/java/org/apache/james/server/core/MimeMessageCopyOnWriteProxy.java
##########
@@ -67,98 +92,208 @@ public MessageReferenceTracker(MimeMessage ref) {
wrapped = ref;
}
- protected synchronized void incrementReferenceCount() {
- referenceCount++;
+ protected void dispose() {
+ ReentrantReadWriteLock.WriteLock lock = this.lock.writeLock();
+ lock.lock();
+ try {
+ referenceCount--;
+ if (referenceCount <= 0) {
+ LifecycleUtil.dispose(wrapped);
+ wrapped = null;
+ }
+ } finally {
+ lock.unlock();
+ }
}
- protected synchronized void decrementReferenceCount() {
- referenceCount--;
- if (referenceCount <= 0) {
- LifecycleUtil.dispose(wrapped);
- wrapped = null;
+ protected void incrementReferences() {
+ ReentrantReadWriteLock.WriteLock lock = this.lock.writeLock();
+ lock.lock();
+ try {
+ referenceCount++;
+ } finally {
+ lock.unlock();
}
}
- protected synchronized int getReferenceCount() {
- return referenceCount;
+ protected <T> T wrapRead(Read<T> op) throws MessagingException {
+ ReentrantReadWriteLock.ReadLock lock = this.lock.readLock();
Review comment:
idem with `readLock`
##########
File path: server/container/core/src/main/java/org/apache/james/server/core/MimeMessageCopyOnWriteProxy.java
##########
@@ -67,98 +92,208 @@ public MessageReferenceTracker(MimeMessage ref) {
wrapped = ref;
}
- protected synchronized void incrementReferenceCount() {
- referenceCount++;
+ protected void dispose() {
+ ReentrantReadWriteLock.WriteLock lock = this.lock.writeLock();
+ lock.lock();
+ try {
+ referenceCount--;
+ if (referenceCount <= 0) {
+ LifecycleUtil.dispose(wrapped);
+ wrapped = null;
+ }
+ } finally {
+ lock.unlock();
+ }
}
- protected synchronized void decrementReferenceCount() {
- referenceCount--;
- if (referenceCount <= 0) {
- LifecycleUtil.dispose(wrapped);
- wrapped = null;
+ protected void incrementReferences() {
+ ReentrantReadWriteLock.WriteLock lock = this.lock.writeLock();
+ lock.lock();
+ try {
+ referenceCount++;
+ } finally {
+ lock.unlock();
}
}
- protected synchronized int getReferenceCount() {
- return referenceCount;
+ protected <T> T wrapRead(Read<T> op) throws MessagingException {
+ ReentrantReadWriteLock.ReadLock lock = this.lock.readLock();
+ lock.lock();
+ try {
+ Preconditions.checkState(referenceCount > 0, "Attempt to read a disposed message");
+ return op.read(wrapped);
+ } finally {
+ lock.unlock();
+ }
}
- public synchronized MimeMessage getWrapped() {
- return wrapped;
+ protected <T> T wrapReadIO(ReadIO<T> op) throws MessagingException, IOException {
+ ReentrantReadWriteLock.ReadLock lock = this.lock.readLock();
+ lock.lock();
+ try {
+ Preconditions.checkState(referenceCount > 0, "Attempt to read a disposed message");
+ return op.read(wrapped);
+ } finally {
+ lock.unlock();
+ }
}
- }
+ protected <T> T wrapReadNoException(Function<MimeMessage, T> op) {
+ ReentrantReadWriteLock.ReadLock lock = this.lock.readLock();
+ lock.lock();
+ try {
+ Preconditions.checkState(referenceCount > 0, "Attempt to read a disposed message");
+ return op.apply(wrapped);
+ } finally {
+ lock.unlock();
+ }
+ }
- protected MessageReferenceTracker refCount;
+ protected MessageReferenceTracker wrapWrite(Write op) throws MessagingException {
+ ReentrantReadWriteLock.WriteLock lock = this.lock.writeLock();
+ lock.lock();
+ try {
+ Preconditions.checkState(referenceCount > 0, "Attempt to write a disposed message");
+ if (referenceCount > 1) {
+ referenceCount--;
+ MessageReferenceTracker newRef = new MessageReferenceTracker(new MimeMessageWrapper(wrapped));
+ newRef.wrapWrite(op);
+ return newRef;
+ } else {
+ op.write(wrapped);
+ return this;
+ }
+ } finally {
+ lock.unlock();
+ }
+ }
+
+ protected MessageReferenceTracker wrapWriteIO(WriteIO op) throws MessagingException, IOException {
+ ReentrantReadWriteLock.WriteLock lock = this.lock.writeLock();
Review comment:
idem
##########
File path: server/container/core/src/main/java/org/apache/james/server/core/MimeMessageCopyOnWriteProxy.java
##########
@@ -67,98 +92,208 @@ public MessageReferenceTracker(MimeMessage ref) {
wrapped = ref;
}
- protected synchronized void incrementReferenceCount() {
- referenceCount++;
+ protected void dispose() {
+ ReentrantReadWriteLock.WriteLock lock = this.lock.writeLock();
+ lock.lock();
+ try {
+ referenceCount--;
+ if (referenceCount <= 0) {
+ LifecycleUtil.dispose(wrapped);
+ wrapped = null;
+ }
+ } finally {
+ lock.unlock();
+ }
}
- protected synchronized void decrementReferenceCount() {
- referenceCount--;
- if (referenceCount <= 0) {
- LifecycleUtil.dispose(wrapped);
- wrapped = null;
+ protected void incrementReferences() {
+ ReentrantReadWriteLock.WriteLock lock = this.lock.writeLock();
+ lock.lock();
+ try {
+ referenceCount++;
+ } finally {
+ lock.unlock();
}
}
- protected synchronized int getReferenceCount() {
- return referenceCount;
+ protected <T> T wrapRead(Read<T> op) throws MessagingException {
+ ReentrantReadWriteLock.ReadLock lock = this.lock.readLock();
+ lock.lock();
+ try {
+ Preconditions.checkState(referenceCount > 0, "Attempt to read a disposed message");
+ return op.read(wrapped);
+ } finally {
+ lock.unlock();
+ }
}
- public synchronized MimeMessage getWrapped() {
- return wrapped;
+ protected <T> T wrapReadIO(ReadIO<T> op) throws MessagingException, IOException {
+ ReentrantReadWriteLock.ReadLock lock = this.lock.readLock();
+ lock.lock();
+ try {
+ Preconditions.checkState(referenceCount > 0, "Attempt to read a disposed message");
+ return op.read(wrapped);
+ } finally {
+ lock.unlock();
+ }
}
- }
+ protected <T> T wrapReadNoException(Function<MimeMessage, T> op) {
+ ReentrantReadWriteLock.ReadLock lock = this.lock.readLock();
+ lock.lock();
+ try {
+ Preconditions.checkState(referenceCount > 0, "Attempt to read a disposed message");
+ return op.apply(wrapped);
+ } finally {
+ lock.unlock();
+ }
+ }
- protected MessageReferenceTracker refCount;
+ protected MessageReferenceTracker wrapWrite(Write op) throws MessagingException {
+ ReentrantReadWriteLock.WriteLock lock = this.lock.writeLock();
+ lock.lock();
+ try {
+ Preconditions.checkState(referenceCount > 0, "Attempt to write a disposed message");
+ if (referenceCount > 1) {
+ referenceCount--;
+ MessageReferenceTracker newRef = new MessageReferenceTracker(new MimeMessageWrapper(wrapped));
+ newRef.wrapWrite(op);
+ return newRef;
+ } else {
+ op.write(wrapped);
+ return this;
+ }
+ } finally {
+ lock.unlock();
+ }
+ }
+
+ protected MessageReferenceTracker wrapWriteIO(WriteIO op) throws MessagingException, IOException {
+ ReentrantReadWriteLock.WriteLock lock = this.lock.writeLock();
+ lock.lock();
+ try {
+ Preconditions.checkState(referenceCount > 0, "Attempt to write a disposed message");
+ if (referenceCount > 1) {
+ referenceCount--;
+ MessageReferenceTracker newRef = new MessageReferenceTracker(new MimeMessageWrapper(wrapped));
+ newRef.wrapWriteIO(op);
+ return newRef;
+ } else {
+ op.write(wrapped);
+ return this;
+ }
+ } finally {
+ lock.unlock();
+ }
+ }
- public MimeMessageCopyOnWriteProxy(MimeMessage original) throws MessagingException {
- this(original, false);
+ private MimeMessage getWrapped() {
+ return wrapped;
+ }
+
+ protected MessageReferenceTracker newRef() {
+ ReentrantReadWriteLock.WriteLock lock = this.lock.writeLock();
Review comment:
idem
##########
File path: server/container/core/src/main/java/org/apache/james/server/core/MimeMessageCopyOnWriteProxy.java
##########
@@ -67,98 +92,208 @@ public MessageReferenceTracker(MimeMessage ref) {
wrapped = ref;
}
- protected synchronized void incrementReferenceCount() {
- referenceCount++;
+ protected void dispose() {
+ ReentrantReadWriteLock.WriteLock lock = this.lock.writeLock();
+ lock.lock();
+ try {
+ referenceCount--;
+ if (referenceCount <= 0) {
+ LifecycleUtil.dispose(wrapped);
+ wrapped = null;
+ }
+ } finally {
+ lock.unlock();
+ }
}
- protected synchronized void decrementReferenceCount() {
- referenceCount--;
- if (referenceCount <= 0) {
- LifecycleUtil.dispose(wrapped);
- wrapped = null;
+ protected void incrementReferences() {
+ ReentrantReadWriteLock.WriteLock lock = this.lock.writeLock();
+ lock.lock();
+ try {
+ referenceCount++;
+ } finally {
+ lock.unlock();
}
}
- protected synchronized int getReferenceCount() {
- return referenceCount;
+ protected <T> T wrapRead(Read<T> op) throws MessagingException {
+ ReentrantReadWriteLock.ReadLock lock = this.lock.readLock();
+ lock.lock();
+ try {
+ Preconditions.checkState(referenceCount > 0, "Attempt to read a disposed message");
+ return op.read(wrapped);
+ } finally {
+ lock.unlock();
+ }
}
- public synchronized MimeMessage getWrapped() {
- return wrapped;
+ protected <T> T wrapReadIO(ReadIO<T> op) throws MessagingException, IOException {
+ ReentrantReadWriteLock.ReadLock lock = this.lock.readLock();
+ lock.lock();
+ try {
+ Preconditions.checkState(referenceCount > 0, "Attempt to read a disposed message");
+ return op.read(wrapped);
+ } finally {
+ lock.unlock();
+ }
}
- }
+ protected <T> T wrapReadNoException(Function<MimeMessage, T> op) {
+ ReentrantReadWriteLock.ReadLock lock = this.lock.readLock();
+ lock.lock();
+ try {
+ Preconditions.checkState(referenceCount > 0, "Attempt to read a disposed message");
+ return op.apply(wrapped);
+ } finally {
+ lock.unlock();
+ }
+ }
- protected MessageReferenceTracker refCount;
+ protected MessageReferenceTracker wrapWrite(Write op) throws MessagingException {
+ ReentrantReadWriteLock.WriteLock lock = this.lock.writeLock();
Review comment:
idem
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
chibenwa commented on pull request #280:
URL: https://github.com/apache/james-project/pull/280#issuecomment-748935370
And thanks for pushing on this topic ;-)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] mbaechler commented on pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
mbaechler commented on pull request #280:
URL: https://github.com/apache/james-project/pull/280#issuecomment-745875476
> > AFAICT, "always copy" is always faster. GC time is always comparable. Did you read the right column?
>
> I do not understand...
>
> 13718ms VS 12911ms, 12177 ms vs 11838ms & 6024 VS 5954ms => Always copy is always slightly slower (3% - 1%)
Oh yes, smaller is better, I'm wrong.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa commented on a change in pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
chibenwa commented on a change in pull request #280:
URL: https://github.com/apache/james-project/pull/280#discussion_r542255839
##########
File path: server/container/core/src/main/java/org/apache/james/server/core/MimeMessageCopyOnWriteProxy.java
##########
@@ -67,98 +93,208 @@ public MessageReferenceTracker(MimeMessage ref) {
wrapped = ref;
}
- protected synchronized void incrementReferenceCount() {
- referenceCount++;
+ protected void dispose() {
+ ReentrantReadWriteLock.WriteLock writeLock = lock.writeLock();
+ writeLock.lock();
+ try {
+ referenceCount--;
+ if (referenceCount <= 0) {
+ LifecycleUtil.dispose(wrapped);
+ wrapped = null;
+ }
+ } finally {
+ writeLock.unlock();
+ }
}
- protected synchronized void decrementReferenceCount() {
- referenceCount--;
- if (referenceCount <= 0) {
- LifecycleUtil.dispose(wrapped);
- wrapped = null;
+ protected void incrementReferences() {
+ ReentrantReadWriteLock.WriteLock writeLock = lock.writeLock();
+ writeLock.lock();
+ try {
+ referenceCount++;
Review comment:
I'm becoming paranoid when locks are around I guess!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] chibenwa closed pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
chibenwa closed pull request #280:
URL: https://github.com/apache/james-project/pull/280
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org
[GitHub] [james-project] jeantil commented on a change in pull request #280: JAMES-3477 demonstrates concurrency issue in MimeMessageCOW
Posted by GitBox <gi...@apache.org>.
jeantil commented on a change in pull request #280:
URL: https://github.com/apache/james-project/pull/280#discussion_r542370160
##########
File path: server/mailet/integration-testing/src/test/java/org/apache/james/mailets/MimeMessageBenchTest.java
##########
@@ -0,0 +1,146 @@
+/****************************************************************
+ * Licensed to the Apache Software Foundation (ASF) under one *
+ * or more contributor license agreements. See the NOTICE file *
+ * distributed with this work for additional information *
+ * regarding copyright ownership. The ASF licenses this file *
+ * to you under the Apache License, Version 2.0 (the *
+ * "License"); you may not use this file except in compliance *
+ * with the License. You may obtain a copy of the License at *
+ * *
+ * http://www.apache.org/licenses/LICENSE-2.0 *
+ * *
+ * Unless required by applicable law or agreed to in writing, *
+ * software distributed under the License is distributed on an *
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY *
+ * KIND, either express or implied. See the License for the *
+ * specific language governing permissions and limitations *
+ * under the License. *
+ ****************************************************************/
+
+package org.apache.james.mailets;
+
+import static org.apache.james.mailets.configuration.Constants.DEFAULT_DOMAIN;
+import static org.apache.james.mailets.configuration.Constants.FROM;
+import static org.apache.james.mailets.configuration.Constants.LOCALHOST_IP;
+import static org.apache.james.mailets.configuration.Constants.PASSWORD;
+import static org.apache.james.mailets.configuration.Constants.awaitAtMostOneMinute;
+
+import java.util.concurrent.TimeUnit;
+import java.util.stream.IntStream;
+
+import org.apache.james.MemoryJamesServerMain;
+import org.apache.james.core.builder.MimeMessageBuilder;
+import org.apache.james.mailets.configuration.CommonProcessors;
+import org.apache.james.mailets.configuration.MailetConfiguration;
+import org.apache.james.mailets.configuration.MailetContainer;
+import org.apache.james.mailets.configuration.ProcessorConfiguration;
+import org.apache.james.modules.protocols.SmtpGuiceProbe;
+import org.apache.james.probe.DataProbe;
+import org.apache.james.server.core.MailImpl;
+import org.apache.james.transport.mailets.LogMessage;
+import org.apache.james.transport.matchers.RecipientIs;
+import org.apache.james.utils.DataProbeImpl;
+import org.apache.james.utils.SMTPMessageSender;
+import org.apache.james.utils.SpoolerProbe;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.jupiter.api.Disabled;
+import org.junit.rules.TemporaryFolder;
+
+import com.github.fge.lambdas.Throwing;
+import com.google.common.base.Stopwatch;
+
+/**
+ * This benches the efficiency at which James mailetcontainer splits emails.
+ */
+@Disabled
+public class MimeMessageBenchTest {
+ private static String CONTENT = "0123456789\r\n".repeat(1024 * 10); // 120KB message
+
+ @Rule
+ public TemporaryFolder temporaryFolder = new TemporaryFolder();
+ @Rule
+ public SMTPMessageSender messageSender = new SMTPMessageSender(DEFAULT_DOMAIN);
+
+ private TemporaryJamesServer jamesServer;
+
+ @Before
+ public void setup() throws Exception {
+ jamesServer = TemporaryJamesServer.builder()
+ .withBase(MemoryJamesServerMain.SMTP_AND_IMAP_MODULE)
+ .withMailetContainer(
+ generateMailetContainerConfiguration())
+ .build(temporaryFolder.newFolder());
+ jamesServer.start();
+
+ DataProbe dataProbe = jamesServer.getProbe(DataProbeImpl.class);
+ dataProbe.addDomain(DEFAULT_DOMAIN);
+ dataProbe.addUser("rcpt1@" + DEFAULT_DOMAIN, PASSWORD);
+ dataProbe.addUser("rcpt2@" + DEFAULT_DOMAIN, PASSWORD);
+ dataProbe.addUser("rcpt3@" + DEFAULT_DOMAIN, PASSWORD);
+ dataProbe.addUser("rcpt4@" + DEFAULT_DOMAIN, PASSWORD);
+ dataProbe.addUser("rcpt5@" + DEFAULT_DOMAIN, PASSWORD);
+ }
+
+ @After
+ public void tearDown() {
+ jamesServer.shutdown();
+ }
+
+ @Test
+ public void receivedMessagesShouldContainDeliveredToHeaders() throws Exception {
+ messageSender.connect(LOCALHOST_IP, jamesServer.getProbe(SmtpGuiceProbe.class).getSmtpPort());
+
+ Stopwatch stopwatch = Stopwatch.createStarted();
+ IntStream.range(0, 100)
+ .forEach(Throwing.intConsumer(i -> messageSender.sendMessage(MailImpl.builder()
+ .name("name" + i)
+ .sender(FROM)
+ .addRecipients("rcpt1@" + DEFAULT_DOMAIN,
+ "rcpt2@" + DEFAULT_DOMAIN,
+ "rcpt3@" + DEFAULT_DOMAIN,
+ "rcpt4@" + DEFAULT_DOMAIN,
+ "rcpt5@" + DEFAULT_DOMAIN)
+ .mimeMessage(MimeMessageBuilder.mimeMessageBuilder()
+ .setSubject("subject i")
+ .setText(CONTENT))
+ .build())));
+
+ awaitAtMostOneMinute.until(() -> jamesServer.getProbe(SpoolerProbe.class).processingFinished());
+ System.out.println("Spent: " + stopwatch.elapsed(TimeUnit.MILLISECONDS));
+ }
+
+
+ private MailetContainer.Builder generateMailetContainerConfiguration() {
+ return TemporaryJamesServer.defaultMailetContainerConfiguration()
+ .putProcessor(ProcessorConfiguration.transport()
+ .addMailet(MailetConfiguration.builder()
+ .matcher(RecipientIs.class)
+ .matcherCondition("rcpt1@" + DEFAULT_DOMAIN)
+ .mailet(LogMessage.class)
+ .addProperty("passThrough", "true"))
+ .addMailet(MailetConfiguration.builder()
+ .matcher(RecipientIs.class)
+ .matcherCondition("rcpt2@" + DEFAULT_DOMAIN)
+ .mailet(LogMessage.class)
+ .addProperty("passThrough", "true"))
+ .addMailet(MailetConfiguration.builder()
+ .matcher(RecipientIs.class)
+ .matcherCondition("rcpt3@" + DEFAULT_DOMAIN)
+ .mailet(LogMessage.class)
+ .addProperty("passThrough", "true"))
+ .addMailet(MailetConfiguration.builder()
+ .matcher(RecipientIs.class)
+ .matcherCondition("rcpt4@" + DEFAULT_DOMAIN)
+ .mailet(LogMessage.class)
+ .addProperty("passThrough", "true"))
+ .addMailet(MailetConfiguration.builder()
+ .matcher(RecipientIs.class)
+ .matcherCondition("rcpt5@" + DEFAULT_DOMAIN)
+ .mailet(LogMessage.class)
+ .addProperty("passThrough", "true"))
+ .addMailetsFrom(CommonProcessors.deliverOnlyTransport()));
Review comment:
alright
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@james.apache.org
For additional commands, e-mail: notifications-help@james.apache.org