Re: Copy alist instead of deep copy on Grob clone (issue 561640045 by hanw...@gmail.com)

commit a456a5946c21cb63c64640a758e7faf829d98fa2 Author: Han-Wen Nienhuys Date: Mon Apr 13 20:57:51 2020 +0200 Copy alist instead of deep copy on Grob clone https://codereview.appspot.com/561640045/

Re: Copy alist instead of deep copy on Grob clone (issue 561640045 by hanw...@gmail.com)

On Thu, Apr 16, 2020 at 7:16 AM Werner LEMBERG wrote: > > > > I did some better structured measurements, with interleaved runs on > > MSDM: [...] > > Have you ever tried valgrind's `callgrind` tool for profiling (and > using `kcachegrind` for displaying the results)? While very slow it > would

Re: Copy alist instead of deep copy on Grob clone (issue 561640045 by hanw...@gmail.com)

>> Have you ever tried valgrind's `callgrind` tool for profiling (and >> using `kcachegrind` for displaying the results)? While very slow >> it would avoid temperature issues and the like – no need to call it >> multiple times to get reliable values. > > What magic does callgrind do to prevent

Re: Copy alist instead of deep copy on Grob clone (issue 561640045 by hanw...@gmail.com)

Am Donnerstag, den 16.04.2020, 07:16 +0200 schrieb Werner LEMBERG: > > I did some better structured measurements, with interleaved runs on > > MSDM: [...] > > Have you ever tried valgrind's `callgrind` tool for profiling (and > using `kcachegrind` for displaying the results)? While very slow it

Re: Copy alist instead of deep copy on Grob clone (issue 561640045 by hanw...@gmail.com)

On Apr 16, 2020, at 01:16, Werner LEMBERG wrote: > > Have you ever tried valgrind's `callgrind` tool for profiling (and > using `kcachegrind` for displaying the results)? While very slow it > would avoid temperature issues and the like – no need to call it > multiple times to get reliable

Re: Copy alist instead of deep copy on Grob clone (issue 561640045 by hanw...@gmail.com)

On 2020/04/15 22:15:04, hanwenn wrote: > On 2020/04/14 20:07:17, hahnjo wrote: > > On 2020/04/14 19:14:29, hanwenn wrote: > > > On 2020/04/14 10:22:01, hahnjo wrote: > > > > On 2020/04/14 09:39:18, hanwenn wrote: > > > > > it reads from the immutable_list_ if there is no override in the > > > > >

Re: Copy alist instead of deep copy on Grob clone (issue 561640045 by hanw...@gmail.com)

> I did some better structured measurements, with interleaved runs on > MSDM: [...] Have you ever tried valgrind's `callgrind` tool for profiling (and using `kcachegrind` for displaying the results)? While very slow it would avoid temperature issues and the like – no need to call it multiple

Re: Copy alist instead of deep copy on Grob clone (issue 561640045 by hanw...@gmail.com)

On 2020/04/15 22:23:44, hanwenn wrote: > description now I remember why the deep copy was there. I've updated the description. https://codereview.appspot.com/561640045/

Re: Copy alist instead of deep copy on Grob clone (issue 561640045 by hanw...@gmail.com)

On 2020/04/14 20:07:17, hahnjo wrote: > On 2020/04/14 19:14:29, hanwenn wrote: > > On 2020/04/14 10:22:01, hahnjo wrote: > > > On 2020/04/14 09:39:18, hanwenn wrote: > > > > it reads from the immutable_list_ if there is no override in the > > > > mutable property list. > > > > > > Ack, that's

Re: Copy alist instead of deep copy on Grob clone (issue 561640045 by hanw...@gmail.com)

On 2020/04/14 19:14:29, hanwenn wrote: > On 2020/04/14 10:22:01, hahnjo wrote: > > On 2020/04/14 09:39:18, hanwenn wrote: > > > 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

Re: Copy alist instead of deep copy on Grob clone (issue 561640045 by hanw...@gmail.com)

On 2020/04/14 10:22:01, hahnjo wrote: > On 2020/04/14 09:39:18, hanwenn wrote: > > On Tue, Apr 14, 2020 at 9:28 AM 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

Re: Copy alist instead of deep copy on Grob clone (issue 561640045 by hanw...@gmail.com)

On 2020/04/14 09:39:18, hanwenn wrote: > On Tue, Apr 14, 2020 at 9:28 AM 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

Re: Copy alist instead of deep copy on Grob clone (issue 561640045 by hanw...@gmail.com)

On Tue, Apr 14, 2020 at 9:28 AM 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?

Re: Copy alist instead of deep copy on Grob clone (issue 561640045 by hanw...@gmail.com)

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

Re: Copy alist instead of deep copy on Grob clone (issue 561640045 by hanw...@gmail.com)

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

Re: Copy alist instead of deep copy on Grob clone (issue 561640045 by hanw...@gmail.com)

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

Re: Copy alist instead of deep copy on Grob clone (issue 561640045 by hanw...@gmail.com)

it's hard to say if this makes a measurable difference: $ grep -C1 User.time timing-deepcopy.txt Command being timed: "lilypond -I carver MSDM" User time (seconds): 56.16 System time (seconds): 0.73 -- Command being timed: "./out/bin/lilypond.baseline -I carver

Re: Copy alist instead of deep copy on Grob clone (issue 561640045 by hanw...@gmail.com)

On 2020/04/13 20:44:47, hahnjo wrote: > Please give reasons for changes, ie why isn't it necessary to do a deep copy? Is > it currently not failing a test, are the shallow copies immutable anyway, is > there some other guarantee? Done. https://codereview.appspot.com/561640045/

Copy alist instead of deep copy on Grob clone (issue 561640045 by hanw...@gmail.com)

Please give reasons for changes, ie why isn't it necessary to do a deep copy? Is it currently not failing a test, are the shallow copies immutable anyway, is there some other guarantee? IMO pushing less changes with strictly better descriptions will get us further in the long run.