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

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

Details

Reviewers

dumbbell
emaste
imp
cem

Commits

rS338316: Speed up vt(4) by keeping a record of the most recently drawn character and

Summary

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

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

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

Harbormaster completed remote builds in B18810: Diff 46710.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.

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

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

sys/dev/vt/hw/vga/vt_vga.c
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

sys/dev/vt/hw/vga/vt_vga.c
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

sys/dev/vt/hw/vga/vt_vga.c
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.