purebred-mua / purebred

A terminal based mail user agent based on notmuch
GNU Affero General Public License v3.0
139 stars 19 forks source link

mail view: search term highlight is very costly #486

Closed frasertweedale closed 1 year ago

frasertweedale commented 1 year ago

Describe the bug The markup implementation copied from Brick is very slow, due to a lot of unpacking and repacking of text.

See attached profile, which records the following user session:

  1. open purebred with a fairly constrained search term
  2. open a long mail (test mail includes text of RFC 7030). This is in the initial search result.
  3. search within mail body for term "CMS". There are 15 results.
  4. press n 10 times to scroll through search hits in the mail body
  5. press q to return to thread index
  6. press q to quit purebred

Top cost centres:

COST CENTRE         MODULE                 SRC                                           %time %alloc

markupToList        Data.Text.Markup       src/Data/Text/Markup.hs:(73,1)-(83,72)         48.1   80.8
vBox                Brick.Widgets.Core     src/Brick/Widgets/Core.hs:(481,1)-(483,41)     31.2    2.7
@@                  Data.Text.Markup       src/Data/Text/Markup.hs:43:1-46                 2.3    5.9
safeWctwidth        Graphics.Text.Width    src/Graphics/Text/Width.hs:59:1-53              1.7    1.2
markup              Purebred.Brick.Markup  src/Purebred/Brick/Markup.hs:(49,1)-(62,55)     1.3    1.0
cropResultToContext Brick.Widgets.Internal src/Brick/Widgets/Internal.hs:(82,1)-(87,47)    1.2    1.2
renderMailView      Purebred.UI.Mail.Main  src/Purebred/UI/Mail/Main.hs:58:1-98            1.1    0.5

Expected behavior Heaps faster.

Additional context purebred-linux-x86_64.prof.txt

It probably makes sense to address this alongside #342 and move away from the Brick markup implementation.

frasertweedale commented 1 year ago

Note: the high costs of vBox are being addressed in brick PR https://github.com/jtdaugherty/brick/pull/398.

The top cost centres now look like:

COST CENTRE         MODULE                 SRC                                           %time %alloc

markupToList        Data.Text.Markup       src/Data/Text/Markup.hs:(73,1)-(83,72)         67.9   81.0
vBox                Brick.Widgets.Core     src/Brick/Widgets/Core.hs:(480,1)-(482,41)      3.6    2.4
@@                  Data.Text.Markup       src/Data/Text/Markup.hs:43:1-46                 3.6    6.0
safeWctwidth        Graphics.Text.Width    src/Graphics/Text/Width.hs:59:1-53              2.8    1.2
crop                Brick.BorderMap        src/Brick/BorderMap.hs:(189,1)-(195,9)          2.1    0.8
cropResultToContext Brick.Widgets.Internal src/Brick/Widgets/Internal.hs:(82,1)-(87,47)    2.0    1.2
renderMailView      Purebred.UI.Mail.Main  src/Purebred/UI/Mail/Main.hs:58:1-98            1.6    0.5
markup              Purebred.Brick.Markup  src/Purebred/Brick/Markup.hs:(49,1)-(62,55)     1.6    1.0
vLimit              Brick.Widgets.Core     src/Brick/Widgets/Core.hs:(844,1)-(846,70)      1.1    0.4
romanofski commented 1 year ago

If we move away from the markup implementation, how would we markup search highlights tho? Do you think we would be better of addressing this by implementing our own markup in events of searching?

I suppose one downside with the current situation is that we always render markup even tho we aren't searching.

frasertweedale commented 1 year ago

Yes, I think we can hand-roll a more efficient implementation.

frasertweedale commented 1 year ago

Addressed in https://github.com/purebred-mua/purebred/pull/491.

frasertweedale commented 1 year ago

491 was merged. Closing.