Page MenuHomeFreeBSD

Cache the most recently drawn text on vt and don't re-draw it

Authored by cperciva on Aug 15 2018, 3:42 PM.



Keep a record of the most recently drawn character and the foreground and background colours. In bitblt_text functions, compare values to this cache and don't re-draw the characters if they haven't changed.

In particular, when scrolling the display (which is where most time is spent within the vt driver) there can be a significant performance improvement when most lines are less than the width of the terminal, since this avoids re-drawing blanks on top of blanks.

Note that "re-drawing" here includes writing to the VGA text mode buffer; on virtualized systems this can be extremely slow (since it triggers a glyph being rendered onto a 640x480 screen).

Test Plan

Tested on my laptop (FB) and in EC2 (VGA).

On a c5.4xlarge EC2 instance, the time spent in _vprintf drops from ~1200 ms to ~700 ms.

Diff Detail

rS FreeBSD src repository
Automatic diff as part of commit; lint not applicable.
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

cperciva created this revision.Aug 15 2018, 3:42 PM

On my laptop, this cuts the time spent in _vprintf during the kernel boot from 970 ms down to 155 ms.

imp accepted this revision.Aug 21 2018, 3:44 PM

These optimizations look sane to me, but I've not spent a lot of time studying it for subtle effects.

This revision is now accepted and ready to land.Aug 21 2018, 3:44 PM
cem accepted this revision.EditedAug 21 2018, 5:18 PM
cem added a subscriber: cem.

Probably ok; context (i.e., using arc or diff -U9999) would be helpful. Some nits below.

887 ↗(On Diff #46710)

This seems to only cache screen contents in text mode; the same optimization could be applied to vga_bitblt_text_gfxmode.

933–940 ↗(On Diff #46710)

This is identical to the FB variant — why is this a "method" instead of just a single implementation?

cperciva added inline comments.Aug 21 2018, 6:06 PM
887 ↗(On Diff #46710)

Are you able to test such a change? I didn't want to touch something I couldn't test, this close to the release.

933–940 ↗(On Diff #46710)

I wasn't sure if there might be other hardware backends -- I figured it was better to play it safe by having the default behaviour be "this method doesn't exist so nothing happens".

cem added inline comments.Aug 21 2018, 7:27 PM
887 ↗(On Diff #46710)

I probably cannot guarantee I can test that change — I'm ok with this as-is.

933–940 ↗(On Diff #46710)

I don't really see how this could be harmful for other backends. It's just manipulating a generic vt data structure. I guess the screen width needs to be a variable somewhere (for 'z' calculation) and we should bail if a backend does not define it (zero) but otherwise I think this routine should be shared.

This revision was automatically updated to reflect the committed changes.