On 2020/04/14 09:39:18, hanwenn wrote: > On Tue, Apr 14, 2020 at 9:28 AM <mailto:jonas.hahnf...@gmail.com> wrote: > > > > On 2020/04/14 07:24:10, hanwenn wrote: > > > On 2020/04/14 07:00:56, hahnjo wrote: > > > > On 2020/04/13 21:01:11, hanwenn wrote: > > > > > it's hard to say if this makes a measurable difference: > > > > > > > > > > [...] > > > > > > > > So then ... why do it at all? From a code perspective, a deep copy > > looks saner > > > > in terms of "encapsulation" (one principle of object-oriented > > programming). If > > > > there is no provable gain, I'm for leaving the current code alone > > until there > > > is > > > > good reason to change - "premature optimization is the root of all > > evil" > > > > > > As I described in the commit message, this code doesn't protect > > anything, > > > because get_property can also return values from the immutable list. > > > > get_property uses data backed by mutable_property_alist_, right? > > Use the Source, Jonas!
I find this statement inappropriate: I'm not going to read through 100k LOC to find the one thing that is possibly different from what I remember. That is why I ask if I'm missing something. > https://github.com/lilypond/lilypond/blob/0c00cd98e81b27325bed5891b950fe7f0f0ebe5d/lily/grob-property.cc#L140 > > it reads from the immutable_list_ if there is no override in the > mutable property list. Ack, that's also what the two names suggested. The code in Grob's copy constructor does a copy of mutable_property_alist_. As far as I understand it contains those elements that have been changed for the Grob? So I'm thinking about the following situation (not real code, just conceptually): Grob g1; g1.set_property("key", "value"); Grob g2(g1); g2.set_property("key", "value2"); set_property will change mutable_property_alist_, and with this change both Grobs will share some of it. So I can't help, but this looks like the g1.get_property("key") will probably return "value2" at the end, no? https://codereview.appspot.com/561640045/
