Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(275)

Issue 579570043: Optimize get_path_list. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 11 months ago by hanwenn
Modified:
3 years, 11 months ago
Reviewers:
dak
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Optimize get_path_list. It constructed a list just call memv() on it. Memv does the hare&tortoise algorithm, so it's also unnecessarily slow.

Patch Set 1 #

Total comments: 4

Patch Set 2 : dak #

Unified diffs Side-by-side diffs Delta from patch set Stats (+15 lines, -13 lines) Patch
M lily/stencil-integral.cc View 1 1 chunk +15 lines, -13 lines 0 comments Download

Messages

Total messages: 4
dak
There is scm_c_memq for avoiding hare&tortoise, but for this use case, the explicit code seems ...
3 years, 11 months ago (2020-04-13 17:50:34 UTC) #1
hanwenn
dak
3 years, 11 months ago (2020-04-13 19:10:35 UTC) #2
hanwenn
https://codereview.appspot.com/579570043/diff/577750045/lily/stencil-integral.cc File lily/stencil-integral.cc (right): https://codereview.appspot.com/579570043/diff/577750045/lily/stencil-integral.cc#newcode115 lily/stencil-integral.cc:115: && (SCM_EQ_P (head, ly_symbol2scm ("moveto")) On 2020/04/13 17:50:33, dak ...
3 years, 11 months ago (2020-04-13 19:11:22 UTC) #3
hanwenn
3 years, 11 months ago (2020-04-24 14:02:14 UTC) #4
On 2020/04/13 19:11:22, hanwenn wrote:
>
https://codereview.appspot.com/579570043/diff/577750045/lily/stencil-integral.cc
> File lily/stencil-integral.cc (right):
> 
>
https://codereview.appspot.com/579570043/diff/577750045/lily/stencil-integral...
> lily/stencil-integral.cc:115: && (SCM_EQ_P (head, ly_symbol2scm ("moveto"))
> On 2020/04/13 17:50:33, dak wrote:
> > SCM_EQ_P is an old deprecated macro.  Use scm_is_eq instead.  Same
> performance.
> 
> Done.
> 
>
https://codereview.appspot.com/579570043/diff/577750045/lily/stencil-integral...
> lily/stencil-integral.cc:127: return get_path_list (scm_cdr (l));
> On 2020/04/13 17:50:33, dak wrote:
> > Not caused by this patch, but this does look like something nicer addressed
by
> a
> > loop rather than tail recursion with regard to C++ idioms.
> 
> I'm gonna pass on this for now.

commit d48d79b52c28cf9c82d088b2fdf8edbf452d6f94
Author: Han-Wen Nienhuys <hanwen@lilypond.org>
Date:   Mon Apr 13 18:36:46 2020 +0200

    Optimize get_path_list.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b