1510325 - Date column slows nsTreeBodyFrame::PaintTreeBody().

Status: RESOLVED FIXED

(Thunderbird :: General, defect)

Description

[Eric Toombs]

User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:63.0) Gecko/20100101 Firefox/63.0

Steps to reproduce:

With a full screen of emails visible in nsTreeBodyFrame, I scroll quickly by pressing and dragging the scrollbar, or by pressing and holding pgup/pgdown.


Actual results:

The frame lags behind the scrolling action considerably.


Expected results:

No lag.

Furthermore, if I hide the Date column by right clicking on the headers and deselecting "Date", all of the lag instantly disappears.

I will try nuking the current formatting code and replacing it with a call to strftime(). I am expecting a massive performance improvement.

Comment 1

[Eric Toombs]

Attachment: MsgDBView-use-strftime.diff

This nukes the slow icu date rendering code and replaces it with one call to strftime(). Dates are still localised, but using glibc's locale support instead of icu's.

Comment 2

[Eric Toombs]

Just as I suspected, there is no more scrolling lag after switching to strftime().

Comment 3

[Jorg K (CEST = GMT+2)]

Very interesting. Thunderbird followed all the datetime formatting changes in Mozilla Core, which basically switched to new layer based on ICU. We can't just rip this out and go back to stdlib. What's the "current" locale? TB supports two locales, the application locale the program was built in and the OS locale used for the regional setting in the OS.

The date column can also have three three different formats depending on whether the date is today, this week or older. Your patch doesn't compile on Windows :-(

Zibi, would you like to comment here further. Surely FF displays very few dates, so ICU slowness doesn't matter.

Comment 4

[jc]

I am also affected by this bug. Ubuntu 18.04, Thunderbird 60.2.1 (Ubuntu) or official 60.3.1.

Comment 5

[lars]

Same problem here. Arch Linux (Plasma 5, X11 and Wayland), Thunderbird 60.3.1.

Removing the date column makes the lag disappear.

Comment 6

[Eric Toombs]

(In reply to Jorg K (GMT+1) from comment #3)
> Very interesting. Thunderbird followed all the datetime formatting changes
> in Mozilla Core, which basically switched to new layer based on ICU. We
> can't just rip this out and go back to stdlib.
Well, I just did, and I have never been happier with thunderbird's performance. I know ICU is newer and therefore better, but strftime() is just as cross-platform. Dates may not be identical between OSes, but using the same date formatting as the rest of the OS thunderbird is running on is just as defensible.
> What's the "current" locale?
In general, the one most recently set by setlocale(), which in thunderbird is the one set by the LC_TIME, LANG, or LC_ALL environment variables.
> TB supports two locales, the application locale the program was built in and
> the OS locale used for the regional setting in the OS.
> 
> The date column can also have three three different formats depending on
> whether the date is today, this week or older.
Yeah, Turing-complete people don't care about such features. If you don't know what day it is, stay away from my civilisation. You can keep doing it that way if you want. strftime() is flexible enough. But I'm satisfied with my fix.
> Your patch doesn't compile on
> Windows :-(
strftime() is not a UNIXey thing. It is part of the C standard library. I have no idea why it didn't work. Did it produce an error message? I don't have an instance of windows running anywhere to test it on.

Comment 7

[Eric Toombs]

You may need to do a non-zero amount of work to get it to work on operating systems and compilers that do not conform to the C standard.

Comment 8

[Eric Toombs]

Attachment: PKGBUILD.diff

Archlinux users may test the patch by svning thunderbird's ABS directory, downloading the patch to it, and making the following changes to the PKGBUILD.

Fair warning: it takes a really long time and at least 4 gigs of free RAM to compile.

I also have a binary prepared here: http://toombs.tk/thunderbird-60.3.1-2-x86_64.pkg.tar.xz

Comment 9

[Eric Toombs]

Scratch that last link. Use this one: https://toombs.tk/thunderbird-60.3.1-2-x86_64.pkg.tar.xz

Comment 10

[Zibi Braniecki [:zbraniecki][:gandalf]]

> Zibi, would you like to comment here further. Surely FF displays very few dates, so ICU slowness doesn't matter.

ICU slowness does matter, and I'm interested in investigating it. I'd like to see if we can find performance improvements without giving up the functionality before we evaluate alternatives on the platform level.

Keeping the NI? to profile the current codepaths more.

Comment 11

[malte.gross]

can confirm this bug in arch linux as well
leads to very choppy scrolling in the messages list when the date-column is enabled

Comment 12

[Zibi Braniecki [:zbraniecki][:gandalf]]

Can anyone test this behavior against latest nightly? 

Jonathan just landed a date time pattern cache in bug 1404666 and I hope it may positively affect this bug.

Comment 13

[malte.gross]

just had a look at the nightly version from the build server

File 	thunderbird-66.0a1.en-US.linux-x86_64.tar.bz2 	58M 	19-Dec-2018 10:53

Can confirm that it is much less choppy with the current daily build.

Comment 14

[Zibi Braniecki [:zbraniecki][:gandalf]]

Eric, can you verify this?

Comment 15

[Jorg K (CEST = GMT+2)]

Bug 1404666 will be included in TB 60.5 ESR (see bug 1404666 comment #15).

Comment 16

[Jorg K (CEST = GMT+2)]

If you want to try an unofficial TB 60.4.1 with this change:
Linux32: https://queue.taskcluster.net/v1/task/M1I1pUWgTtyzJrnwfS52xQ/runs/0/artifacts/public/build/target.tar.bz2
Linux64: https://queue.taskcluster.net/v1/task/PBZAiLHgRwS86ucOyA-36A/runs/0/artifacts/public/build/target.tar.bz2
Mac:     https://queue.taskcluster.net/v1/task/RPQQmm11QVCscueXGKLlrQ/runs/0/artifacts/public/build/target.dmg
Win32:   https://queue.taskcluster.net/v1/task/Hadsd7AbTnObU20UQqYsKw/runs/0/artifacts/public/build/install/sea/target.installer.exe
Win64:   https://queue.taskcluster.net/v1/task/ZgVxHmPCSSKlDCmWCqyHrw/runs/0/artifacts/public/build/install/sea/target.installer.exe

BTW, on Windows 10 with a Ryzen 2 2700 I don't actually see any lag to start with.

Comment 17

[Zibi Braniecki [:zbraniecki][:gandalf]]

I'm going to close this bug due to lack of activity. Reopen if needed.