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? So if we do a deep copy there, a copied grob can return different data than the original one. Whereas a copy of the list returns the same references which can be modified from both instances of the object. At least that's the way it works in C++, is there a difference with SCM values? > Due to timing variability, we can't quantify performance changes below 1% of > impact, but if we add 10 of them in a row, we do get measurable impact. > > For the Mutable_properties change, we saw seemingly consistent performance > differences, which we could tease apart from deep copy change by separating out > the commit. We did see a measurable difference with the Mutable_properties change, so this cannot be the reason. I never said it was, it was just a guess. https://codereview.appspot.com/561640045/
