#16 Maildir performance regression an hour ago

Comment by ~rjarry on ~rjarry/aerc

Hi Koni,

Koni Marti, Jan 26, 2022 at 12:06:

Hi Robin,

I'm writing with regard to the maildir performance regression issue that you are assigned to.

I believe that my recent commit (b30909 "dirlist: skip unnecessary change-folder action") added 1 second to the startup time. Hence, I'm partly to blame and would like to offer my support, if needed at all.

I have a attached a quick summary of my findings so far hoping that it can be useful in finding the root causes and that it aligns with your analysis.

Thanks a lot. I had not started working on it, I got caught up in something else yesterday.

I am CC'ing the ticket with your findings, I hope that is OK.

Approach

A sample maildir with 400'000 messages (total 3.1G) was used for the performance tests.

The performance was estimated by running time ./aerc and quitting when the message view was completely loaded. The performance metric is the total execution time, which is averaged over two runs. Individual timings are obviously machine dependent, but the overall trend is relevant.

Results

Tag	Time	Commit causing bulk of time increase
master	11.7s	cb3090956cfd
0.7.1	10.7s
0.7.0	10.7s
0.6.0	10.7s
0.5.0	11.0s
0.4.0	11.1s	01c96e78dfe8
0.3.0	5.3s	43435ba06cd0
0.2.0	1.5s

Great work!

Discussion

A performance regression over the recent releases can be demonstrated. The majority of the observed time increase is causes by release 0.4.0

The large increase in the startup time is due to the getDirectoryInfo() function in the maildir worker. This function loops over all messages and determines the number of seen and recent mails.

I guess this is required to update the folder view. However it could be made asynchronously after the folder has been opened to have a more responsive UI.

Another time increase of ~1s was introduced by my recent commit to avoid loading folders that are skipped over anyway. This can be reduced to 500ms, completely removed or have the user choose a suitable time himself.

I wonder if that could even be reduced to 300ms. Why not have an option for this but I don't think it is that important if the default value is suitable for most people.

Proof of principle

The following patch is to verify the claim that the largest time increase is caused by the getDirectoryInfo function and the skip-folder wait. This patch, applied to the master, reduces the startup time from 11.7s to 3.3s by only calculating the total mails in a folder and not the seen/recent messages:

diff --git a/widgets/dirlist.go b/widgets/dirlist.go
index 53afc1d..b7e8538 100644
--- a/widgets/dirlist.go
+++ b/widgets/dirlist.go
@@ -96,7 +96,7 @@ func (dirlist *DirectoryList) Select(name string) {

        go func() {
                select {
-               case <-time.After(1 * time.Second):
+               case <-time.After(500 * time.Millisecond):
                        dirlist.worker.PostAction(&types.OpenDirectory{Directory: name},
                                func(msg types.WorkerMessage) {
                                        switch msg.(type) {
diff --git a/worker/maildir/worker.go b/worker/maildir/worker.go
index 2f75f12..df8af4d 100644
--- a/worker/maildir/worker.go
+++ b/worker/maildir/worker.go
@@ -143,6 +143,9 @@ func (w *Worker) getDirectoryInfo(name string) *models.DirectoryInfo {
                return dirInfo
        }

+       dirInfo.Exists = len(uids)
+       return dirInfo
+
        for _, uid := range uids {
                message, err := w.c.Message(dir, uid)
                if err != nil {

Thoughts?

#16 Maildir performance regression a day ago

~rjarry assigned ~rjarry to #16 on ~rjarry/aerc

#16 Maildir performance regression a day ago

Comment by ~rjarry on ~rjarry/aerc

Thanks, I can take care of it. I'll have a quick look tonight.

#16 Maildir performance regression a day ago

maildir added by ~rjarry on ~rjarry/aerc

#16 Maildir performance regression a day ago

Comment by ~rjarry on ~rjarry/aerc

Do you recall a version with which the performance was better? That would help pinpointing the issue.

#15 crash when changing screen resolution 2 days ago

ui added by ~rjarry on ~rjarry/aerc

#15 crash when changing screen resolution 2 days ago

bug added by ~rjarry on ~rjarry/aerc

#15 crash when changing screen resolution 2 days ago

Ticket created by ~rjarry on ~rjarry/aerc

I have not diagnosed this properly. When changing screen resolution, I sometimes get this crash:

goroutine 1 [running]:
runtime/debug.Stack()
        /usr/lib/go-1.17/src/runtime/debug/stack.go:24 +0x65
runtime/debug.PrintStack()
        /usr/lib/go-1.17/src/runtime/debug/stack.go:16 +0x19
main.PanicTermFix(0x0)
        /home/robin/os/aerc/aerc.go:231 +0x45
panic({0x8e55c0, 0xc000020030})
        /usr/lib/go-1.17/src/runtime/panic.go:1047 +0x266
git.sr.ht/~rjarry/aerc/lib/ui.(*Context).Subcontext(0xc0000d64d0, 0xc0002fe0c0, 0xc0000d64d0, 0xa37718, 0xc0001c6d00)
        /home/robin/os/aerc/lib/ui/context.go:47 +0x16e
git.sr.ht/~rjarry/aerc/lib/ui.(*Grid).Draw(0xc0000d64d0, 0xc0002fe0c0)
        /home/robin/os/aerc/lib/ui/grid.go:143 +0x2d5
git.sr.ht/~rjarry/aerc/widgets.(*Aerc).Draw(0xc0000d6580, 0xc0002fe0c0)
        /home/robin/os/aerc/widgets/aerc.go:177 +0x2e
git.sr.ht/~rjarry/aerc/lib/ui.(*UI).Tick(0xc0004240f0)
        /home/robin/os/aerc/lib/ui/ui.go:113 +0x1f7
main.main()
        /home/robin/os/aerc/aerc.go:216 +0xa6c
aerc crashed: Attempted to create context with negative offset

I cannot reproduce this consistently.

#8 bindings: <C-h> = :prev-tab<Enter> broken in 0.6.0 5 days ago

Comment by ~rjarry on ~rjarry/aerc

Robin Jarry referenced this ticket in commit b963265.

#11 aerc crashes after piping git patch repeatedly 5 days ago

Comment by ~rjarry on ~rjarry/aerc

Koni Marti referenced this ticket in commit 7f34cab.