Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-24 Thread jonas . hahnfeld
On 2020/04/24 08:15:09, Valentin Villenave wrote: > On 2020/04/24 07:20:55, hahnjo wrote: > > If there are no technical objections, I think we should move > > forward and let something as big sit around for too long. > > Just for clarification, did you miss a "not" in that sentence? Ehm yes.

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-24 Thread v . villenave
On 2020/04/24 07:20:55, hahnjo wrote: > If there are no technical objections, I think we should move > forward and let something as big sit around for too long. Just for clarification, did you miss a "not" in that sentence? Cheers, - V. https://codereview.appspot.com/573670043/

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-24 Thread jonas . hahnfeld
So I've largely kept out of the discussion for now, mostly because I'm not overly familiar with the code. However I fully agree with Dan that a macro pretending to be a member function is neither obvious nor C++ style. As such I'm in favor of doing the conversion as it better meets the

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-13 Thread Carl . D . Sorensen
I am in favor of this patch because of the following: 1) David K. has a long history of improving things through changes like this. 2) It does not change the user interface or any files that a user accesses 3) David has done the work of making all the code changes this syntax change requires

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-13 Thread dak
On 2020/04/13 23:07:57, Dan Eble wrote: > This change per se LGTM. I remember discussing this syntactic change briefly on > the list a few(?) months ago, so this is not surprising. > > I'm quite pleased with this change, actually. I remember how I felt the first > time I came across

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-13 Thread nine . fierce . ballads
This change per se LGTM. I remember discussing this syntactic change briefly on the list a few(?) months ago, so this is not surprising. I'm quite pleased with this change, actually. I remember how I felt the first time I came across klass->a_macro_actually(...) and couldn't find the method in

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-12 Thread dak
On 2020/04/12 10:35:24, hanwenn wrote: > On Sat, Apr 11, 2020 at 3:55 PM David Kastrup wrote: > > >> You haven't understood the scheme. A grob only contains the properties > > >> seminal to its type. That's why there is a double indirection. A full > > >> array of 416

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-12 Thread Han-Wen Nienhuys
On Sat, Apr 11, 2020 at 3:55 PM David Kastrup wrote: > >> You haven't understood the scheme. A grob only contains the properties > >> seminal to its type. That's why there is a double indirection. A full > >> array of 416 entries is needed per grob _type_, not per grob. > > > > I still don't

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-11 Thread David Kastrup
Han-Wen Nienhuys writes: > On Fri, Apr 10, 2020 at 1:07 PM David Kastrup wrote: >> > * memory use: each SCM value takes 8 bytes, and there are 416 grob >> > properties today, for a total of 3328 bytes. Morgenlied is single page >> > of music and has 3704 grobs. So storage for the vectors (which

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-11 Thread Han-Wen Nienhuys
On Sat, Apr 11, 2020 at 2:53 PM Jonas Hahnfeld wrote: > > Am Samstag, den 11.04.2020, 14:45 +0200 schrieb Han-Wen Nienhuys: > > I think it is not out of the ordinary to discuss plans invasive plans > > before implementing them. > > To be fair, David did ask about it two months ago: >

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-11 Thread Jonas Hahnfeld
Am Samstag, den 11.04.2020, 14:45 +0200 schrieb Han-Wen Nienhuys: > I think it is not out of the ordinary to discuss plans invasive plans > before implementing them. To be fair, David did ask about it two months ago: https://lists.gnu.org/archive/html/lilypond-devel/2020-02/msg00668.html (I

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-11 Thread Han-Wen Nienhuys
On Sat, Apr 11, 2020 at 2:45 PM Han-Wen Nienhuys wrote: > > What you call a "giant rewrite" is basically this patch. It's a purely > > syntactical patch only extending rather than reducing possible > > approaches and perfectly compatible with different implementations. The > > work for that

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-11 Thread Han-Wen Nienhuys
On Fri, Apr 10, 2020 at 1:07 PM David Kastrup wrote: > > * memory use: each SCM value takes 8 bytes, and there are 416 grob > > properties today, for a total of 3328 bytes. Morgenlied is single page > > of music and has 3704 grobs. So storage for the vectors (which will be > > mostly empty) will

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-10 Thread David Kastrup
Han-Wen Nienhuys writes: > On Thu, Apr 9, 2020 at 7:45 PM wrote: >> >> On 2020/04/09 17:07:46, hanwenn wrote: >> > I'm curious about these optimizations. Can you say more? >> >> Properties are currently stored in alists. They can be stored in >> vectors instead. Give all grob properties a

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-10 Thread Han-Wen Nienhuys
On Thu, Apr 9, 2020 at 7:45 PM wrote: > > On 2020/04/09 17:07:46, hanwenn wrote: > > I'm curious about these optimizations. Can you say more? > > Properties are currently stored in alists. They can be stored in > vectors instead. Give all grob properties a number, then have an array > per grob

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-09 Thread dak
On 2020/04/09 17:07:46, hanwenn wrote: > I'm curious about these optimizations. Can you say more? Properties are currently stored in alists. They can be stored in vectors instead. Give all grob properties a number, then have an array per grob type mapping this number to an index into an array.

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-09 Thread hanwenn
I'm curious about these optimizations. Can you say more? https://codereview.appspot.com/573670043/

Re: Refactor get/set_property to take the item as first argument (issue 573670043 by d...@gnu.org)

2020-04-09 Thread hanwenn
I'm curious about these optimizations. Can you say more? https://codereview.appspot.com/573670043/