After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 354061 - Excessive use of strlen by _vte_termcap_create
Excessive use of strlen by _vte_termcap_create
Status: RESOLVED FIXED
Product: vte
Classification: Core
Component: general
unspecified
Other Linux
: Normal minor
: ---
Assigned To: VTE Maintainers
VTE Maintainers
Depends on:
Blocks:
 
 
Reported: 2006-09-03 01:13 UTC by Chris Wilson
Modified: 2006-12-29 09:53 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Use getline if available (5.75 KB, patch)
2006-09-03 01:16 UTC, Chris Wilson
needs-work Details | Review
a new patch. (36.23 KB, patch)
2006-11-17 04:48 UTC, Allison Karlitskaya (desrt)
none Details | Review
new patch (34.75 KB, patch)
2006-11-17 06:19 UTC, Allison Karlitskaya (desrt)
committed Details | Review

Description Chris Wilson 2006-09-03 01:13:22 UTC
Profiling 'gnome-terminal -e exit' results in >30% of the time spent below _vte_termcap_create with >25% of the time spent calling strlen().
Comment 1 Chris Wilson 2006-09-03 01:16:41 UTC
Created attachment 72112 [details] [review]
Use getline if available

Avoid using strlen in loading termcap by using getline if available and propagate string length instead of recalculating.
Comment 2 Chris Wilson 2006-09-03 01:18:54 UTC
On my system this patch reduced the CPU time for 'gnome-terminal -e exit' from 0.6s to 0.4s.
Comment 3 Allison Karlitskaya (desrt) 2006-11-16 17:37:10 UTC
This patch won't be accepted in its current state:  specifically, getline() use is not acceptable.
Comment 4 Allison Karlitskaya (desrt) 2006-11-17 04:48:53 UTC
Created attachment 76743 [details] [review]
a new patch.

New patch based on GMappedFile and using GTree.  

 ChangeLog     |   10
 src/matcher.c |   16
 src/vtetc.c   | 1330 +++++++++++++++++---------------------
 3 files changed, 621 insertions(+), 735 deletions(-)

I've tested it on esr's "Big Termcap".

It runs more than 5 times as fast as the old implementation and uses somewhat less memory.  It does absolutely no reallocing (no malloc at all, for that matter, except on the API, for a function that is supposed to return a copy of a string).

Searching should also be faster.  Significant amounts of memory (almost all of it) could be saved by removing the search-tree functionality at which point searching would be no slower than the old implementation.

The patch essentially works by having the entire file mapped into memory and building a tree of terminal emulations.  Each emulation points at another tree which contains the attributes for that emualtion.

plz review :)
Comment 5 Allison Karlitskaya (desrt) 2006-11-17 06:19:28 UTC
Created attachment 76746 [details] [review]
new patch

Here's a new patch implementing the suggested change to the above patch.  The result is a further increase in load speed (now about 10-12x faster than the original code) and a massive decrease in memory use (something like 15x improvement on the original).

Essentially, I don't bother building a tree for the individual capabilities, but rather just do a simple text search.  This makes lookups slightly slower but I don't think very many lookups happen anyway (since the matcher basically reads the values and then never looks at the termcap again).  I haven't tested this theory, though. :)
Comment 6 Allison Karlitskaya (desrt) 2006-11-17 06:22:10 UTC
Of course, we should really also look at bug 169295.
Comment 7 Allison Karlitskaya (desrt) 2006-12-27 08:15:34 UTC
i just instrumented the termcap stuff.  i added some requesting/creating stuff to also check up on the caches (they're working fine...)


on opening the first gnome-terminal:

requesting new tc for /opt/gnome/share/vte/termcap/xterm
creating new tc for /opt/gnome/share/vte/termcap/xterm
find string: xterm/ae
find string: xterm/al
find string: xterm/AL
find string: xterm/as
find string: xterm/bc
find string: xterm/bl
find string: xterm/bt
find string: xterm/cb
find string: xterm/cc
find string: xterm/cd
find string: xterm/ce
find string: xterm/ch
find string: xterm/cl
find string: xterm/cm
find string: xterm/CM
find string: xterm/cr
find string: xterm/cs
find string: xterm/ct
find string: xterm/cv
find string: xterm/dc
find string: xterm/DC
find string: xterm/dl
find string: xterm/DL
find string: xterm/dm
find string: xterm/do
find string: xterm/DO
find string: xterm/ds
find string: xterm/eA
find string: xterm/ec
find string: xterm/ed
find string: xterm/ei
find string: xterm/ff
find string: xterm/fs
find string: xterm/hd
find string: xterm/ho
find string: xterm/hu
find string: xterm/i1
find string: xterm/i3
find string: xterm/ic
find string: xterm/IC
find string: xterm/if
find string: xterm/im
find string: xterm/ip
find string: xterm/iP
find string: xterm/is
find string: xterm/l0
find string: xterm/l1
find string: xterm/l2
find string: xterm/l3
find string: xterm/l4
find string: xterm/l5
find string: xterm/l6
find string: xterm/l7
find string: xterm/l8
find string: xterm/l9
find string: xterm/la
find string: xterm/le
find string: xterm/LE
find string: xterm/LF
find string: xterm/ll
find string: xterm/LO
find string: xterm/mb
find string: xterm/MC
find string: xterm/md
find string: xterm/me
find string: xterm/mh
find string: xterm/mk
find string: xterm/ml
find string: xterm/ML
find string: xterm/mm
find string: xterm/mo
find string: xterm/mp
find string: xterm/mr
find string: xterm/MR
find string: xterm/mu
find string: xterm/nd
find string: xterm/nl
find string: xterm/nw
find string: xterm/pc
find string: xterm/pf
find string: xterm/pk
find string: xterm/pl
find string: xterm/pn
find string: xterm/po
find string: xterm/pO
find string: xterm/ps
find string: xterm/px
find string: xterm/r1
find string: xterm/r2
find string: xterm/r3
find string: xterm/RA
find string: xterm/rc
find string: xterm/rf
find string: xterm/RF
find string: xterm/RI
find string: xterm/rp
find string: xterm/rP
find string: xterm/rs
find string: xterm/RX
find string: xterm/s0
find string: xterm/s1
find string: xterm/s2
find string: xterm/s3
find string: xterm/sa
find string: xterm/SA
find string: xterm/sc
find string: xterm/se
find string: xterm/sf
find string: xterm/SF
find string: xterm/sr
find string: xterm/SR
find string: xterm/st
find string: xterm/SX
find string: xterm/ta
find string: xterm/ts
find string: xterm/uc
find string: xterm/ue
find string: xterm/up
find string: xterm/UP
find string: xterm/us
find string: xterm/vb
find string: xterm/vi
find string: xterm/wi
find string: xterm/XF
find string: xterm/XN
find bool: xterm/am
find bool: xterm/bw
find bool: xterm/LP
find bool: xterm/ul
find bool: xterm/xn
find numeric: xterm/co
find numeric: xterm/li
find numeric: xterm/it


on opening new windows:

requesting new tc for /opt/gnome/share/vte/termcap/xterm
find bool: xterm/am
find bool: xterm/bw
find bool: xterm/LP
find bool: xterm/ul
find bool: xterm/xn
find numeric: xterm/co
find numeric: xterm/li
find numeric: xterm/it


i tried a few ls's (with colours) and opening vim and stuff but there never seems to be additional termcap requests after the first few.
Comment 8 Behdad Esfahbod 2006-12-27 18:18:31 UTC
Cool.
Comment 9 Behdad Esfahbod 2006-12-27 23:46:21 UTC
Do we need to bump the glib dependency?
Comment 10 Allison Karlitskaya (desrt) 2006-12-27 23:58:42 UTC
GMappedFile

"Since 2.8" -- http://developer.gnome.org/doc/API/2.2/glib/glib-File-Utilities.html#GMappedFile

configure.in: PKG_CHECK_MODULES(GLIB,glib-2.0 > 2.9.0)

should be fine.
Comment 11 James Andrewartha 2006-12-29 09:43:12 UTC
You broke the build: http://jhbuild.bxlug.be/builds/2006-12-29-0002/logs/vte/#build - int is not the same as gssize.

vtetc.c:
char *
_vte_termcap_find_string_length (VteTermcap *termcap,
                                 const char *tname,
                                 const char *cap,
                                 int *length)
{

vtetc.h:
char *_vte_termcap_find_string_length(struct _vte_termcap *termcap,
              const char *tname,
              const char *cap, gssize *length);
Comment 12 Allison Karlitskaya (desrt) 2006-12-29 09:53:20 UTC
fixed, thanks :)