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

["Clément Pit--Claudel" <clement.pitclaudel@live.com>]

[-- Attachment #1: Type: text/plain, Size: 3936 bytes --]

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.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]