Faster text input insertion and deletion #662

Merged
merged 4 commits into from Sep 30, 2012

Projects

None yet

2 participants

@akshayaurora
Member

Using the perf test from here #661. This is the difference I get.

With master branch

=====================
Test: load_large_text
=====================
loading uix/textinput.py....
putting text in textinput
mem usage after test
44 MB
------------------------------------------
Loaded 1602 lines 1.22284793854 secs
------------------------------------------
=====================
Test: stress_insert
=====================
[INFO   ] [Clipboard   ] using <pygame> as clipboard provider
Done!
pasted 1602 lines 9.0 times
mem usage after test
120 MB
total lines in text input: 17612
--------------------------------------
total time elapsed: 27.5229020119
--------------------------------------
=====================
Test: stress_del
=====================
Done!
deleted 210 characters 9 times
mem usage after test
159 MB
total lines in text input: 17553
--------------------------------------
total time elapsed: 75.9813170433
--------------------------------------
=====================
Test: stress_selection
=====================
Done!
mem usage after test
159 MB
--------------------------------------
total time elapsed: 0
--------------------------------------
===================
All Tests Completed
===================

with this branch

=====================
Test: load_large_text
=====================
loading uix/textinput.py....
putting text in textinput
mem usage after test
45 MB
------------------------------------------
Loaded 1699 lines 0.942888975143 secs
------------------------------------------
=====================
Test: stress_insert
=====================
[INFO   ] [Clipboard   ] using <pygame> as clipboard provider
Done!
pasted 1699 lines 8.0 times
mem usage after test
89 MB
total lines in text input: 16981
--------------------------------------
total time elapsed: 3.90121006966
--------------------------------------
=====================
Test: stress_del
=====================
Done!
deleted 210 characters 9 times
mem usage after test
91 MB
total lines in text input: 16922
--------------------------------------
total time elapsed: 2.36115598679
--------------------------------------
=====================
Test: stress_selection
=====================
Done!
mem usage after test
91 MB
--------------------------------------
total time elapsed: 0
--------------------------------------
===================
All Tests Completed
===================
@tito
Member
tito commented Sep 2, 2012

It look ok in the first read, but i'm hesitant about the side effect of some part.
Right now, i'll not merge it for 1.4.0, but we could do a 1.4.1 for it.

The side effect is when the font_size change, the textwidth cache is not cleared, and dunno about gl reloading for labels if they stay on the cache. If you work on window, doing a resizing is working ?

@tito
Member
tito commented Sep 2, 2012

If i understand, the main optim is to cache the text width calculation right? And i think that would be possible to cache directly within the core Label class. But then, the cache always need to check the font-name/size/bold/italic attribute etc, to see if we already have the size of the text.

Then, i imagine a new approach:

from kivy.core.text import Label
# create a fake and empty label with the right attribute as the textinput would use
ref_label = Label(...)
# then use the ext_extents()
ref_label.get_extents(...)

In the __init__ of the LabelBase, we could create the "identifiant" of the label (font_name, font_size, ...)
And then, add a cache decorator in the get_extents() implem in pygame and pil.

What do you think ?

@akshayaurora
Member

+1 for 1.4.1 or even 1.5, I'd rather get it right than rush it,
I haven't checked on windows, just about to do it and yes the label_width cache should be updated on font_size.
Text width calculation is a major part of optimisation but not the main optim, The main optimisation is in refreshing only the lines that are affected by the edit(insert/delete) instead of all the lines in refresh_text.

Your approach is better and would improve perf for text used elsewhere not just in TextInput.
I'll try to implement it.

@akshayaurora
Member

Removing the cache for text width all to-gather this is the result I get.

=====================
Test: stress_insert
=====================
[INFO   ] [Clipboard   ] using <pygame> as clipboard provider
Done!
pasted 1692 lines 8.0 times
mem usage after test
88 MB
total lines in text input: 16911
--------------------------------------
total time elapsed: 7.85320210457
--------------------------------------

Kind of expected doubling of time in some cases and reduced mem usage of roughly 1 M.
Moving the caching to get extents (we already use _label_cached in TextInput and self.font_id in core label so I only edited get_extents in text_pygame.py to test the difference)::

def get_extents(self, text):
        uid = ''.join((text, '_', self.fontid))
        width = Cache_get('core_label.width', uid)
        if not width:
            width = self._get_font().size(text)
            Cache_append('core_label.width', uid, width)
        return width

This is the difference

=====================
Test: stress_insert
=====================
[INFO   ] [Clipboard   ] using <pygame> as clipboard provider
Done!
pasted 1692 lines 8.0 times
mem usage after test
91 MB
total lines in text input: 16911
--------------------------------------
total time elapsed: 8.62396907806
--------------------------------------

Not only is there a increase in memory usage, the time used also increases, which negates the purpose of using the cache.

@akshayaurora akshayaurora was assigned Sep 4, 2012
@akshayaurora
Member

text_width Cache is now reloaded when font_size/name/padding... change. This has been now tested on Windows 7 (vm) and it works after resize, although the text in the textinput tends to scroll to top, but that's a septate issue.

@tito tito merged commit 6a0e9fa into master Sep 30, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment