Re: Rewrite Mutable_properties based on vector<SCM> (issue 575990043 by address@hidden)

[hanwenn]

Reviewers: hahnjo,

Message:
On 2020/04/13 16:40:34, hahnjo wrote:
> I gave this patch a try on my system, time lilypond MSDM.ly, 'real'
time, 3
> repetitions
> 
> baseline (master, 0e7c26d40f):
> 0m42,533s; 0m42,547s; 0m42,430s
> 
> Abstract Grob property storage into Mutable_properties.
> (https://codereview.appspot.com/561640043)
> 0m41,102s; 0m41,036s; 0m40,861s
> 
> Rewrite Mutable_properties based on vector<SCM>
> (https://codereview.appspot.com/575990043/)
> 0m42,461s; 0m42,632s; 0m42,212s
> 
> (I did not apply any of the other optimization patches posted in the
last days.)
> If at all, Issue 561640043 seems to show some benefit. This _could_ be
because
> Grob::Grob (Grob const &s) doesn't call ly_deep_copy anymore, but I've
only
> looked into this very briefly because I think the introduction of
> Mutable_properties makes sense in any case. However I clearly don't
see 7%
> improvement for this patch. Is there anything I missed?

No, I'll have a look at some other systems. You're right about the deep
copy, and it's an explanation.

What machine are you trying this with? For the record, I see the
difference with GUILE 1.8 on Fedora 31 on a T460p (i5 6440HQ 2.6Ghz).
I'll try with the other machines that I have for comparison.

Description:
Rewrite Mutable_properties based on vector<SCM>

On the Carver MDSM score, this yields a 7% speedup.

Alists use are based on linked lists. They have poor locality, which
introduces more CPU cache misses. Since LilyPond is not multithreaded,
the CPU sits idle during these misses.

This is the culmination of a series of experiments rearchitecting
property storage. For completeness, here are the full results of all
experiments:

Baseline (master, at commit 0e7c26d4):

        User time (seconds): 41.42
        System time (seconds): 0.58

Storing mutable properties in a custom written hash table with linear
probing:

        User time (seconds): 40.44
        System time (seconds): 0.84

Globally memozing symbols to uint16 (this reduces the storage needs of
the hashtable, potentially increasing cache efficiency)

        User time (seconds): 40.61
        System time (seconds): 0.74

Copying immutable properties to the hash table (based on previous
change, ie.  memoized uint16 symbol IDs)

        User time (seconds): 40.21
        System time (seconds): 0.80

Array-based alists (this change):

        User time (seconds): 38.43
        System time (seconds): 0.56

Please review this at https://codereview.appspot.com/575990043/

Affected files (+90, -35 lines):
  M lily/grob.cc
  M lily/include/mutable-properties.hh
  M lily/mutable-properties.cc