Skip to content

Various JMAP performance enhancements - #424

Merged
chibenwa merged 15 commits into
apache:masterfrom
chibenwa:various-improvments
May 14, 2021
Merged

Various JMAP performance enhancements#424
chibenwa merged 15 commits into
apache:masterfrom
chibenwa:various-improvments

Conversation

@chibenwa

Copy link
Copy Markdown
Contributor

Before

reff

After

Screenshot from 2021-05-11 08-13-29

@Arsnael

Arsnael commented May 11, 2021

Copy link
Copy Markdown
Contributor
01:19:32,589 [INFO] There is 1 error reported by Checkstyle 8.29 with checkstyle.xml ruleset.

01:19:32,593 [ERROR] src/main/java/org/apache/james/mailbox/store/quota/ListeningCurrentQuotaUpdater.java:[47,8] (imports) UnusedImports: Unused import - reactor.core.publisher.Flux.

@Arsnael Arsnael left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice series of improvements :)

chibenwa added 15 commits May 13, 2021 14:52
It was forcing to schedule the whole chain on an elastic scheduler...
It was forcing to schedule the whole chain on an elastic scheduler...
Detected thanks to https://github.com/reactor/BlockHound

They are blocking in case of connection recovery.

Doing that on the parallel pool
is bad as it "steals" threads dedicated to non
blocking tasks and slows the entire application down.
Detected thanks to https://github.com/reactor/BlockHound

ACKs are blocking (as we send a message over the
network to RabbitMQ). Doing that on the parallel pool
is bad as it "steals" threads dedicated to non
blocking tasks and slows the entire application down.
… threads upon nested block calls

This test mimics the previous behaviour of LocalDelivery

```
@test
        void testNestedBlocksWithElasticScheduler() {
            // mono1 corresponds to the append to the mailbox (blocking)
            Mono<Void> mono1 = Mono.fromRunnable(() -> {
                System.out.println("mono1 running on " + Thread.currentThread().getName());
            });
            // mono2 corresponds to the retries perforned in MailDispatcher
            Mono<Void> mono2 = Mono.fromRunnable(() -> {
                System.out.println("mono2 running on " + Thread.currentThread().getName());
                mono1.subscribeOn(Schedulers.elastic()).block();
            });
            // This is a spooler thread running LocalDelivery
            System.out.println("Current thread " + Thread.currentThread().getName());
            mono2.subscribeOn(Schedulers.elastic()).block();
        }
```

Output:

```
Current thread main
mono2 running on elastic-2
mono1 running on elastic-3
```

One thread doing the work and two waiting...

With the new paradigm (waiting for a fully reactive version)

```
        @test
        void testNestedBlockWithImmediateScheduler() {
            // mono1 corresponds to the append to the mailbox (blocking)
            Mono<Void> mono1 = Mono.fromRunnable(() -> {
                System.out.println("mono1 running on " + Thread.currentThread().getName());
            });
            // mono2 corresponds to the retries perforned in MailDispatcher
            Mono<Void> mono2 = Mono.fromRunnable(() -> {
                System.out.println("mono2 running on " + Thread.currentThread().getName());
                mono1.subscribeOn(Schedulers.immediate()).block();
            });
            // This is a spooler thread running LocalDelivery
            System.out.println("Current thread " + Thread.currentThread().getName());
            mono2.subscribeOn(Schedulers.immediate()).block();
        }
```

We get...

```
Current thread main
mono2 running on main
mono1 running on main
```

Way better! We do mobilize only one thread that would anyway be blocked.
@chibenwa
chibenwa force-pushed the various-improvments branch from d2b890c to cfdd144 Compare May 13, 2021 07:52
@chibenwa

Copy link
Copy Markdown
Contributor Author

Squashed in prevision of a merge...

@chibenwa
chibenwa merged commit 1554adb into apache:master May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants