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

Issue 1817045: Optimizations for pure-height approximations. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
13 years, 8 months ago by joeneeman
Modified:
12 years, 9 months ago
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

Optimizations for pure-height approximations. Since we end up querying the height of each VerticalAxisGroup multiple times for each line, cache the intermediate results.

Patch Set 1 #

Total comments: 2

Patch Set 2 : Change hash-table size and fix alphabetization. #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+45 lines, -14 lines) Patch
M lily/axis-group-interface.cc View 1 4 chunks +38 lines, -13 lines 1 comment Download
M lily/include/axis-group-interface.hh View 1 chunk +2 lines, -1 line 0 comments Download
M scm/define-grob-properties.scm View 1 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 14
lemzwerg
http://codereview.appspot.com/1817045/diff/1/4 File scm/define-grob-properties.scm (right): http://codereview.appspot.com/1817045/diff/1/4#newcode1059 scm/define-grob-properties.scm:1059: Since the properties in this file are sorted alphabetically, ...
13 years, 8 months ago (2010-07-14 23:15:13 UTC) #1
Neil Puttock
http://codereview.appspot.com/1817045/diff/1/2 File lily/axis-group-interface.cc (right): http://codereview.appspot.com/1817045/diff/1/2#newcode125 lily/axis-group-interface.cc:125: pure_height_cache = scm_c_make_hash_table (1000); This default size appears to ...
13 years, 8 months ago (2010-07-15 22:04:58 UTC) #2
joeneeman
On 2010/07/15 22:04:58, Neil Puttock wrote: > http://codereview.appspot.com/1817045/diff/1/2 > File lily/axis-group-interface.cc (right): > > http://codereview.appspot.com/1817045/diff/1/2#newcode125 ...
13 years, 8 months ago (2010-07-21 20:16:56 UTC) #3
joeneeman
On 2010/07/14 23:15:13, lemzwerg wrote: > http://codereview.appspot.com/1817045/diff/1/4 > File scm/define-grob-properties.scm (right): > > http://codereview.appspot.com/1817045/diff/1/4#newcode1059 > ...
13 years, 8 months ago (2010-07-21 20:17:20 UTC) #4
Neil Puttock
On 2010/07/21 20:16:56, joeneeman wrote: > I can change it to 100, but it's still ...
13 years, 8 months ago (2010-07-21 21:04:38 UTC) #5
Neil Puttock
On 2010/07/21 21:04:38, Neil Puttock wrote: > Ah, don't worry, I think I see what's ...
13 years, 8 months ago (2010-07-21 22:09:03 UTC) #6
joeneeman
On Wed, Jul 21, 2010 at 3:09 PM, <n.puttock@gmail.com> wrote: > On 2010/07/21 21:04:38, Neil ...
13 years, 8 months ago (2010-07-22 20:45:12 UTC) #7
Neil Puttock
On 2010/07/22 20:45:12, joeneeman wrote: > Wait, you mean that you're running it with 1000 ...
13 years, 8 months ago (2010-07-23 19:53:51 UTC) #8
Neil Puttock
Hi Joe, I haven't been able to replicate the problems I had running `make check', ...
13 years, 8 months ago (2010-07-23 21:49:50 UTC) #9
arnonokia6230
On 2010/07/23 21:49:50, Neil Puttock wrote: > Hi Joe, > > I haven't been able ...
13 years, 8 months ago (2010-07-31 10:59:28 UTC) #10
Neil Puttock
On 2010/07/31 10:59:28, arnonokia6230 wrote: > Hm, i am seeing huge memory problems here. What ...
13 years, 8 months ago (2010-07-31 16:36:31 UTC) #11
Graham Percival (old account)
Has this patch been pushed, or definitely rejected, or is it waiting for when Joe ...
13 years, 6 months ago (2010-10-02 16:01:10 UTC) #12
joeneeman
On Sat, Oct 2, 2010 at 9:01 AM, <percival.music.ca@gmail.com> wrote: > Has this patch been ...
13 years, 6 months ago (2010-10-03 04:52:35 UTC) #13
Neil Puttock
13 years, 5 months ago (2010-10-24 14:53:35 UTC) #14
On 2010/10/03 04:52:35, joeneeman wrote:

> I was just waiting to see if the memory problems are still there. I can't
> reproduce them (and it seems that Neil can't either any more) so I'm not
> sure what to do. I could just push it and then revert if people howl...

I'm still not sure what was going on when I first tested this, but it's never
come back.  FWIW, I've had no problems with this patch; I've tested GUB builds
for mingw, linux-x86 and linux-64 with the patch applied, and experienced no
problems (just great improvements in
compile times).

I think we should go ahead and push.  I'll pop a rebased diff on the tracker
soon.

Cheers,
Neil
Sign in to reply to this message.

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