Skip to content
This repository has been archived by the owner on Oct 13, 2021. It is now read-only.

Avoid unnecessary recomputing of layouts #14

Closed
marcusolsson opened this issue May 4, 2017 · 2 comments
Closed

Avoid unnecessary recomputing of layouts #14

marcusolsson opened this issue May 4, 2017 · 2 comments

Comments

@marcusolsson
Copy link
Owner

Currently, SizeHints are calculated on every event which is expensive and makes the UI slow when you for example have labels with a lot of text.

Try visiting a web page in the http example. When the response finishes loading, typing becomes really slow. Profiling shows that a lot of time is spent in Label.SizeHint. A likely solution is to add dirty flags to layouts and widgets to indicate whether the call to Resize can be avoided.

@yml
Copy link
Contributor

yml commented May 24, 2017

I found this issue interesting and I started looking into go tool pprof. I played the scenario described above while taking a cpuprofile:

(pprof) peek SizeHint
19.01s of 19.69s total (96.55%)
Dropped 132 nodes (cum <= 0.10s)
----------------------------------------------------------+-------------
      flat  flat%   sum%        cum   cum%   calls calls% + context 	 	 
----------------------------------------------------------+-------------
                                            18.12s   100% |   github.com/marcusolsson/tui-go.(*Box).SizeHint
     0.03s  0.15%  0.15%     18.12s 92.03%                | github.com/marcusolsson/tui-go.(*Label).SizeHint
                                            10.92s 60.36% |   github.com/marcusolsson/tui-go.stringWidth
                                             6.18s 34.16% |   github.com/marcusolsson/tui-go.(*Label).heightForWidth
                                             0.99s  5.47% |   strings.Split
----------------------------------------------------------+-------------
                                            18.16s   100% |   github.com/marcusolsson/tui-go.doLayout
         0     0%  0.15%     18.16s 92.23%                | github.com/marcusolsson/tui-go.(*Box).SizeHint
                                            18.12s   100% |   github.com/marcusolsson/tui-go.(*Label).SizeHint
----------------------------------------------------------+-------------
(pprof) top
16300ms of 19690ms total (82.78%)
Dropped 132 nodes (cum <= 98.45ms)
Showing top 10 nodes out of 70 (cum >= 10880ms)
      flat  flat%   sum%        cum   cum%
    4130ms 20.98% 20.98%     4130ms 20.98%  github.com/mattn/go-runewidth.inTable
    3680ms 18.69% 39.66%     7810ms 39.66%  github.com/mattn/go-runewidth.inTables
    2030ms 10.31% 49.97%    10180ms 51.70%  github.com/mattn/go-runewidth.(*Condition).RuneWidth
    1730ms  8.79% 58.76%     2060ms 10.46%  strings.genSplit
    1290ms  6.55% 65.31%     2250ms 11.43%  bytes.(*Buffer).WriteByte
    1030ms  5.23% 70.54%     1040ms  5.28%  bytes.(*Buffer).grow
     930ms  4.72% 75.27%     5180ms 26.31%  github.com/mitchellh/go-wordwrap.WrapString
     620ms  3.15% 78.42%      620ms  3.15%  unicode.IsSpace
     460ms  2.34% 80.75%     2710ms 13.76%  bytes.(*Buffer).WriteRune
     400ms  2.03% 82.78%    10880ms 55.26%  github.com/mattn/go-runewidth.(*Condition).StringWidth
(pprof) 
(pprof) list SizeHint
Total: 19.69s
ROUTINE ======================== github.com/marcusolsson/tui-go.(*Box).SizeHint in /home/yml/gopath/src/github.com/marcusolsson/tui-go/box.go
         0     18.16s (flat, cum) 92.23% of Total
         .          .    124:// SizeHint returns the recommended size for the layout.
         .          .    125:func (b *Box) SizeHint() image.Point {
         .          .    126:	var sizeHint image.Point
         .          .    127:
         .          .    128:	for _, child := range b.children {
         .     18.16s    129:		size := child.SizeHint()
         .          .    130:		if b.Alignment() == Horizontal {
         .          .    131:			sizeHint.X += size.X
         .          .    132:			if size.Y > sizeHint.Y {
         .          .    133:				sizeHint.Y = size.Y
         .          .    134:			}
ROUTINE ======================== github.com/marcusolsson/tui-go.(*Label).SizeHint in /home/yml/gopath/src/github.com/marcusolsson/tui-go/label.go
      30ms     18.12s (flat, cum) 92.03% of Total
         .          .     42:}
         .          .     43:
         .          .     44:// SizeHint returns the recommended size for the label.
         .          .     45:func (l *Label) SizeHint() image.Point {
         .          .     46:	var max int
         .      990ms     47:	lines := strings.Split(l.text, "\n")
         .          .     48:	for _, line := range lines {
      20ms     10.94s     49:		if w := stringWidth(line); w > max {
         .          .     50:			max = w
         .          .     51:		}
         .          .     52:	}
      10ms      6.19s     53:	return image.Point{max, l.heightForWidth(max)}
         .          .     54:}
         .          .     55:
         .          .     56:func (l *Label) heightForWidth(w int) int {
         .          .     57:	return len(strings.Split(wordwrap.WrapString(l.text, uint(w)), "\n"))
         .          .     58:}
ROUTINE ======================== github.com/marcusolsson/tui-go.(*TextEdit).SizeHint in /home/yml/gopath/src/github.com/marcusolsson/tui-go/text_edit.go
         0       40ms (flat, cum)   0.2% of Total
         .          .     49:	for _, line := range lines {
         .          .     50:		if w := stringWidth(line); w > max {
         .          .     51:			max = w
         .          .     52:		}
         .          .     53:	}
         .       40ms     54:	return image.Point{max, e.heightForWidth(max)}
         .          .     55:}
         .          .     56:
         .          .     57:// OnEvent handles terminal events.
         .          .     58:func (e *TextEdit) OnEvent(ev Event) {
         .          .     59:	if !e.IsFocused() || ev.Type != EventKey {

yml added a commit to yml/tui-go that referenced this issue May 24, 2017
The cpuprofile of `example/http` shows that when resizing the app with
some large blob of text in Labels the function `label.SizeHint()` is the
bottle neck. The app becomes slow.
@cceckman
Copy link
Contributor

Looking over the change above - it's a little surprising (to me) that the cache isn't also reset on Resize() of a label, since the SizeHint is dependent on the label's size (in the word-wrap case). But I'm also not coming up with a test case where it doesn't work...

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants