bug#21028: Performance regression in revision af1a69f4d17a482c359d98c00ef86fac835b5fac (Apr 2014).

[Clément Pit--Claudel]

On 07/10/2015 05:41 AM, Eli Zaretskii wrote:
>> Date: Fri, 10 Jul 2015 03:34:23 -0700
>> From: Clément Pit--Claudel <clement.pitclaudel@live.com>
>>
>> I tried to pinpoint the commit that introduced the bug, and found revision 
>> af1a69f4d17a482c359d98c00ef86fac835b5fac by bisecting between 24.3 and 
>> master. Here's the commit in question:
>>
>>   af1a69f4d17a482c359d98c00ef86fac835b5fac is the first bad commit
>>   commit af1a69f4d17a482c359d98c00ef86fac835b5fac
>>   Author: Dmitry Antipov <dmantipov@yandex.ru>
>>   Date:   Wed Apr 2 17:24:19 2014 +0400
>>
>>       * font.c (font_list_entities): Do not add empty vector to font cache.
>>       (font_matching_entity): Likewise.  If matching entity is found, insert
>>       1-item vector with this entity instead of entity itself (Bug#17125).
> 
> Maybe I'm blind, but I don't see anything in that changeset that could
> explain a performance hit.  The modified code seems to do
> approximately the same amount of work, and create the same data
> structures, as the original one.  Are you sure that backing out those
> changes fixes your problem?

Thanks for looking into this. Yes, reverting these changes on master fixes the 
problem. I ran a number of experiments:

* I timed the sample code that I sent, before and after 
af1a69f4d17a482c359d98c00ef86fac835b5fac:

** On af1a69f4d17a482c359d98c00ef86fac835b5fac (the commit that I identified as 
bad):

   $ time src/emacs -Q --eval "(progn (dotimes (_ 3) (set-fontset-font t 
'unicode (font-spec :name \"Arial\") nil 'append)) (redisplay t) (kill-emacs))"
   real 0m2.249s
   user 0m1.259s
   sys  0m0.100s

   $ time src/emacs -Q --eval "(progn (dotimes (_ 3) (set-fontset-font t 
'unicode (font-spec :name \"Arial\") nil 'append)) (dotimes (_ 50) (insert 
\"日本国\n\")) (redisplay t) (kill-emacs))"
   real 0m5.749s
   user 0m2.717s
   sys  0m0.857s

** On 200c532bd04a67a89db602462d74706051c61178 (the previous commit)

   $ time src/emacs -Q --eval "(progn (dotimes (_ 3) (set-fontset-font t 
'unicode (font-spec :name \"Arial\") nil 'append)) (redisplay t) (kill-emacs))"
   real 0m2.214s
   user 0m1.247s
   sys  0m0.077s

   $ time src/emacs -Q --eval "(progn (dotimes (_ 3) (set-fontset-font t 
'unicode (font-spec :name \"Arial\") nil 'append)) (dotimes (_ 50) (insert 
\"日本国\n\")) (redisplay t) (kill-emacs))"
   real 0m2.226s
   user 0m1.222s
   sys  0m0.106s

Thus the operation of inserting these 50 lines takes about 3.5 seconds on the 
bad commit, while it's instantaneous in the previous commit.

* On master, timed the same code before and after reverting 
af1a69f4d17a482c359d98c00ef86fac835b5fac:

** Before reverting

   $ time emacs -Q --eval "(progn (dotimes (_ 3) (set-fontset-font t 'unicode 
(font-spec :name \"Arial\") nil 'append)) (redisplay t) (kill-emacs))"
   real 0m2.441s
   user 0m1.363s
   sys  0m0.142s

   $ time emacs -Q --eval "(progn (dotimes (_ 3) (set-fontset-font t 'unicode 
(font-spec :name \"Arial\") nil 'append)) (dotimes (_ 50) (insert \"日本国\n\")) 
(redisplay t) (kill-emacs))"
   real 0m5.149s
   user 0m2.569s
   sys  0m0.742s

** After reverting

   $ time src/emacs -Q --eval "(progn (dotimes (_ 3) (set-fontset-font t 
'unicode (font-spec :name \"Arial\") nil 'append)) (redisplay t) (kill-emacs))"
   real 0m2.413s
   user 0m1.351s
   sys  0m0.147s

   $ time src/emacs -Q --eval "(progn (dotimes (_ 3) (set-fontset-font t 
'unicode (font-spec :name \"Arial\") nil 'append)) (dotimes (_ 50) (insert 
\"日本国\n\")) (redisplay t) (kill-emacs))"
   real 0m2.570s
   user 0m1.608s
   sys  0m0.083s

The figures are very similar to the tests above: with that patch inserting 50 
lines takes 3 seconds, and without it it's instantaneous. Thus I think it's 
safe to say that this particular patch is indeed responsible for the 
performance regression. But maybe I'm missing something?

Cheers,
Clément.

signature.asc
Description: OpenPGP digital signature