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

Issue 561390043: GUILE2: Scale GC heap with the number of smobs (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 weeks ago by hanwenn
Modified:
2 weeks, 1 day ago
Reviewers:
dak, dan, lemzwerg, Dan Eble, hahnjo, pkx166h, hahnjo
CC:
lilypond-devel_gnu.org
Visibility:
Public.

Description

On every GC cycle, increase the heap so we have at least 2000 bytes per non-simple Smob. This increases the size of the heap relative to the live set of data. The number 2000 is based on benchmarking the Robert Carver's "Missa Dum sacrum mysterium" Edited by Vaughan McAlley (2013), a 104 page choir score. By setting GC_INITIAL_HEAP_SIZE and the max heap size, we can see how GC time and heap size relate. On a Lenovo T460p (i5 6440HQ 2.6Ghz), this yields the following timings: * 2G heap: 49.4s (1.8s GC) * 1G heap: 51.7s (3.3s GC) * 800M heap: 54.6s (5.8s GC) * 500M heap: 68.2s (17.9s GC) We see that 800M - 1G is a sweet spot, where around 10% of time is spent on GC. Running the same score with 2000 bytes per smob, we get a final heap size of 1.2G, and a GC time of 3.4s. The reason this is necessary, is because during the interpretation stage, we build up a large set of Grobs. Without tuning, GC is ineffective, because the Grobs are live data and can't be collected. We can observe this problem by running with GC_PRINT_STATS=1, which prints In-use heap: 85% (370824 KiB pointers + 60065 KiB other) In-use heap: 85% (370725 KiB pointers + 59737 KiB other) ..

Patch Set 1 #

Total comments: 1

Patch Set 2 : nit #

Total comments: 8

Patch Set 3 : move to lily-guile.cc #

Patch Set 4 : autoconf #

Total comments: 4

Patch Set 5 : jonas' comments #

Patch Set 6 : rebase #

Patch Set 7 : hook into GC event #

Patch Set 8 : Use smob count as memory proxy #

Total comments: 8

Patch Set 9 : Jonas' comments #

Patch Set 10 : comment #

Unified diffs Side-by-side diffs Delta from patch set Stats (+62 lines, -1 line) Patch
M configure.ac View 1 2 3 4 5 6 8 1 chunk +6 lines, -0 lines 0 comments Download
M lily/include/smobs.hh View 1 2 3 4 5 6 7 1 chunk +7 lines, -1 line 0 comments Download
M lily/smobs.cc View 1 2 3 4 5 6 7 8 9 1 chunk +49 lines, -0 lines 0 comments Download

Messages

Total messages: 65
lemzwerg
LGTM https://codereview.appspot.com/561390043/diff/563450046/lily/score-engraver.cc File lily/score-engraver.cc (right): https://codereview.appspot.com/561390043/diff/563450046/lily/score-engraver.cc#newcode200 lily/score-engraver.cc:200: // This double the heap. TODO: don't do ...
3 weeks, 6 days ago (2020-02-01 06:58:32 UTC) #1
hanwenn
nit
3 weeks, 6 days ago (2020-02-01 08:48:03 UTC) #2
Dan Eble
https://codereview.appspot.com/561390043/diff/557260051/lily/include/score-engraver.hh File lily/include/score-engraver.hh (right): https://codereview.appspot.com/561390043/diff/557260051/lily/include/score-engraver.hh#newcode33 lily/include/score-engraver.hh:33: GC_word last_gc_count_; FYI: Because we're using C++11 now, you ...
3 weeks, 6 days ago (2020-02-01 10:55:50 UTC) #3
hahnjo
https://codereview.appspot.com/561390043/diff/557260051/lily/include/score-engraver.hh File lily/include/score-engraver.hh (right): https://codereview.appspot.com/561390043/diff/557260051/lily/include/score-engraver.hh#newcode26 lily/include/score-engraver.hh:26: #include <gc.h> This effectively adds a dependency on libgc ...
3 weeks, 6 days ago (2020-02-01 11:00:41 UTC) #4
hahnjo
https://codereview.appspot.com/561390043/diff/557260051/lily/include/score-engraver.hh File lily/include/score-engraver.hh (right): https://codereview.appspot.com/561390043/diff/557260051/lily/include/score-engraver.hh#newcode26 lily/include/score-engraver.hh:26: #include <gc.h> On 2020/02/01 11:00:40, hahnjo wrote: > This ...
3 weeks, 6 days ago (2020-02-01 11:26:43 UTC) #5
hanwenn
On 2020/02/01 11:00:41, hahnjo wrote: > https://codereview.appspot.com/561390043/diff/557260051/lily/include/score-engraver.hh > File lily/include/score-engraver.hh (right): > > https://codereview.appspot.com/561390043/diff/557260051/lily/include/score-engraver.hh#newcode26 > ...
3 weeks, 6 days ago (2020-02-01 20:49:09 UTC) #6
hanwenn
move to lily-guile.cc
3 weeks, 6 days ago (2020-02-01 20:58:14 UTC) #7
hanwenn
https://codereview.appspot.com/561390043/diff/557260051/lily/include/score-engraver.hh File lily/include/score-engraver.hh (right): https://codereview.appspot.com/561390043/diff/557260051/lily/include/score-engraver.hh#newcode33 lily/include/score-engraver.hh:33: GC_word last_gc_count_; On 2020/02/01 10:55:50, Dan Eble wrote: > ...
3 weeks, 6 days ago (2020-02-01 20:58:24 UTC) #8
dak
On 2020/02/01 20:49:09, hanwenn wrote: > On 2020/02/01 11:00:41, hahnjo wrote: > > > https://codereview.appspot.com/561390043/diff/557260051/lily/include/score-engraver.hh ...
3 weeks, 6 days ago (2020-02-01 20:59:06 UTC) #9
hanwenn
On 2020/02/01 20:59:06, dak wrote: > On 2020/02/01 20:49:09, hanwenn wrote: > > On 2020/02/01 ...
3 weeks, 6 days ago (2020-02-01 21:23:51 UTC) #10
dak
On 2020/02/01 21:23:51, hanwenn wrote: > On 2020/02/01 20:59:06, dak wrote: > > On 2020/02/01 ...
3 weeks, 6 days ago (2020-02-01 21:59:14 UTC) #11
hahnjo
On 2020/02/01 20:49:09, hanwenn wrote: > On 2020/02/01 11:00:41, hahnjo wrote: > > > https://codereview.appspot.com/561390043/diff/557260051/lily/include/score-engraver.hh ...
3 weeks, 5 days ago (2020-02-02 09:49:50 UTC) #12
hanwenn
On 2020/02/02 09:49:50, hahnjo wrote: > On 2020/02/01 20:49:09, hanwenn wrote: > > On 2020/02/01 ...
3 weeks, 5 days ago (2020-02-02 12:41:59 UTC) #13
hanwenn
On 2020/02/01 21:59:14, dak wrote: > On 2020/02/01 21:23:51, hanwenn wrote: > > On 2020/02/01 ...
3 weeks, 5 days ago (2020-02-02 12:56:44 UTC) #14
hanwenn
On 2020/02/01 11:00:41, hahnjo wrote: > https://codereview.appspot.com/561390043/diff/557260051/lily/include/score-engraver.hh > File lily/include/score-engraver.hh (right): > > https://codereview.appspot.com/561390043/diff/557260051/lily/include/score-engraver.hh#newcode26 > ...
3 weeks, 5 days ago (2020-02-02 13:14:15 UTC) #15
hanwenn
autoconf
3 weeks, 5 days ago (2020-02-02 13:14:31 UTC) #16
hahnjo
https://codereview.appspot.com/561390043/diff/565600046/configure.ac File configure.ac (right): https://codereview.appspot.com/561390043/diff/565600046/configure.ac#newcode270 configure.ac:270: PKG_CHECK_MODULES(BDWGC, bdw-gc) This should fail if not found, you're ...
3 weeks, 5 days ago (2020-02-02 13:21:09 UTC) #17
hanwenn
jonas' comments
3 weeks, 5 days ago (2020-02-02 13:43:25 UTC) #18
hanwenn
https://codereview.appspot.com/561390043/diff/565600046/configure.ac File configure.ac (right): https://codereview.appspot.com/561390043/diff/565600046/configure.ac#newcode270 configure.ac:270: PKG_CHECK_MODULES(BDWGC, bdw-gc) On 2020/02/02 13:21:09, hahnjo wrote: > This ...
3 weeks, 5 days ago (2020-02-02 13:44:06 UTC) #19
hahnjo
On 2020/02/02 13:43:25, hanwenn wrote: > jonas' comments The uploaded diff has the wrong base, ...
3 weeks, 5 days ago (2020-02-02 13:45:34 UTC) #20
hanwenn
rebase
3 weeks, 5 days ago (2020-02-02 13:51:26 UTC) #21
hanwenn
On 2020/02/02 13:45:34, hahnjo wrote: > On 2020/02/02 13:43:25, hanwenn wrote: > > jonas' comments ...
3 weeks, 5 days ago (2020-02-02 13:52:04 UTC) #22
hahnjo
I just tried to reproduce the timings for commits already in master and this patch. ...
3 weeks, 5 days ago (2020-02-02 20:32:37 UTC) #23
dak
jonas.hahnfeld@gmail.com writes: > I just tried to reproduce the timings for commits already in master ...
3 weeks, 5 days ago (2020-02-02 20:51:38 UTC) #24
hanwenn
For testing, use https://github.com/hanwen/lilypond/tree/guile22-experiment in particular, you need https://github.com/hanwen/lilypond/commit/b696550379831ecec1519be6d59cd8a3003e5329 for the UTF-8 parsing. I fixed ...
3 weeks, 5 days ago (2020-02-02 22:33:50 UTC) #25
pkx166h_posteo.net
On 02/02/2020 22:33, Han-Wen Nienhuys wrote: > For me, juggling 15 different outstanding code reviews ...
3 weeks, 5 days ago (2020-02-02 22:47:27 UTC) #26
hanwenn
On Sun, Feb 2, 2020 at 11:47 PM <pkx166h@posteo.net> wrote: > > On 02/02/2020 22:33, ...
3 weeks, 4 days ago (2020-02-03 08:36:09 UTC) #27
hanwenn
On Mon, Feb 3, 2020 at 9:35 AM Han-Wen Nienhuys <hanwenn@gmail.com> wrote: > > On ...
3 weeks, 4 days ago (2020-02-03 09:21:10 UTC) #28
hanwenn
On Mon, Feb 3, 2020 at 9:35 AM Han-Wen Nienhuys <hanwenn@gmail.com> wrote: > > > ...
3 weeks, 4 days ago (2020-02-03 09:31:10 UTC) #29
hanwenn
On Mon, Feb 3, 2020 at 9:35 AM Han-Wen Nienhuys <hanwenn@gmail.com> wrote: > > On ...
3 weeks, 4 days ago (2020-02-03 09:41:12 UTC) #30
dak
Han-Wen Nienhuys <hanwenn@gmail.com> writes: > On Sun, Feb 2, 2020 at 11:47 PM <pkx166h@posteo.net> wrote: ...
3 weeks, 4 days ago (2020-02-03 10:34:22 UTC) #31
dak
Han-Wen Nienhuys <hanwenn@gmail.com> writes: > On Mon, Feb 3, 2020 at 9:35 AM Han-Wen Nienhuys ...
3 weeks, 4 days ago (2020-02-03 10:57:30 UTC) #32
dan_faithful.be
On Feb 3, 2020, at 04:30, Han-Wen Nienhuys <hanwenn@gmail.com> wrote: > > Instead of waiting ...
3 weeks, 4 days ago (2020-02-03 14:58:11 UTC) #33
dak
Dan Eble <dan@faithful.be> writes: > On Feb 3, 2020, at 04:30, Han-Wen Nienhuys <hanwenn@gmail.com> wrote: ...
3 weeks, 4 days ago (2020-02-03 15:06:05 UTC) #34
hahnjo
Am Montag, den 03.02.2020, 09:58 -0500 schrieb Dan Eble: > On Feb 3, 2020, at ...
3 weeks, 4 days ago (2020-02-03 15:09:04 UTC) #35
hahnjo
On 2020/02/02 22:33:50, hanwenn wrote: > For testing, use > > https://github.com/hanwen/lilypond/tree/guile22-experiment So I ran ...
3 weeks, 3 days ago (2020-02-04 18:50:51 UTC) #36
hanwenn
On Tue, Feb 4, 2020 at 7:50 PM <jonas.hahnfeld@gmail.com> wrote: > On 2020/02/02 22:33:50, hanwenn ...
3 weeks, 3 days ago (2020-02-04 22:23:45 UTC) #37
hahnjo
On 2020/02/04 22:23:45, hanwenn wrote: > On Tue, Feb 4, 2020 at 7:50 PM <mailto:jonas.hahnfeld@gmail.com> ...
3 weeks, 2 days ago (2020-02-05 11:33:31 UTC) #38
hahnjo
Some more numbers: I took guile22-experiment and removed the following: * GC_set_free_space_divisor, * GC_INITIAL_HEAP_SIZE=40M * ...
3 weeks, 2 days ago (2020-02-05 12:59:37 UTC) #39
dan_faithful.be
On Feb 5, 2020, at 07:59, jonas.hahnfeld@gmail.com wrote: > > and it seems stable, but ...
3 weeks, 2 days ago (2020-02-05 13:38:02 UTC) #40
dak
Dan Eble <dan@faithful.be> writes: > On Feb 5, 2020, at 07:59, jonas.hahnfeld@gmail.com wrote: >> >> ...
3 weeks, 2 days ago (2020-02-05 13:43:52 UTC) #41
hanwenn
On Wed, Feb 5, 2020 at 12:33 PM <jonas.hahnfeld@gmail.com> wrote: > > Can you do ...
3 weeks, 2 days ago (2020-02-05 23:19:22 UTC) #42
hanwenn
On Thu, Feb 6, 2020 at 12:19 AM Han-Wen Nienhuys <hanwenn@gmail.com> wrote: > Um, that ...
3 weeks, 2 days ago (2020-02-05 23:20:32 UTC) #43
dak
Han-Wen Nienhuys <hanwenn@gmail.com> writes: > On Wed, Feb 5, 2020 at 12:33 PM <jonas.hahnfeld@gmail.com> wrote: ...
3 weeks, 2 days ago (2020-02-05 23:27:25 UTC) #44
hahnjo_hahnjo.de
Am Donnerstag, den 06.02.2020, 00:27 +0100 schrieb David Kastrup: > Han-Wen Nienhuys < > hanwenn@gmail.com ...
3 weeks, 1 day ago (2020-02-06 12:45:02 UTC) #45
hanwenn
Thanks, I ran the carver score successfully now. I took some pauses between runs so ...
3 weeks ago (2020-02-07 16:53:24 UTC) #46
hanwenn
On Fri, Feb 7, 2020 at 5:53 PM Han-Wen Nienhuys <hanwenn@gmail.com> wrote: > Thanks, I ...
3 weeks ago (2020-02-07 17:27:35 UTC) #47
dak
Han-Wen Nienhuys <hanwenn@gmail.com> writes: > Single threaded, the numbers make more sense: > > > ...
3 weeks ago (2020-02-07 18:27:29 UTC) #48
hanwenn
hook into GC event
3 weeks ago (2020-02-07 22:48:41 UTC) #49
hanwenn
On Fri, Feb 7, 2020 at 7:27 PM David Kastrup <dak@gnu.org> wrote: > Han-Wen Nienhuys ...
3 weeks ago (2020-02-07 22:57:52 UTC) #50
hanwenn
Use smob count as memory proxy
2 weeks, 6 days ago (2020-02-08 20:05:53 UTC) #51
hanwenn
On 2020/02/08 20:05:53, hanwenn wrote: > Use smob count as memory proxy This looks good ...
2 weeks, 6 days ago (2020-02-08 20:09:27 UTC) #52
Dan Eble
https://codereview.appspot.com/561390043/diff/567180043/lily/include/smobs.hh File lily/include/smobs.hh (right): https://codereview.appspot.com/561390043/diff/567180043/lily/include/smobs.hh#newcode312 lily/include/smobs.hh:312: static size_t count; It seems that this is initialized ...
2 weeks, 5 days ago (2020-02-09 03:29:27 UTC) #53
hahnjo
The approach sounds good to me. I can confirm that this works on my system ...
2 weeks, 5 days ago (2020-02-09 09:25:21 UTC) #54
hanwenn
On Sun, Feb 9, 2020 at 10:25 AM <jonas.hahnfeld@gmail.com> wrote: > The approach sounds good ...
2 weeks, 5 days ago (2020-02-09 09:36:40 UTC) #55
hahnjo
On 2020/02/09 09:36:40, hanwenn wrote: > On Sun, Feb 9, 2020 at 10:25 AM <mailto:jonas.hahnfeld@gmail.com> ...
2 weeks, 5 days ago (2020-02-09 09:55:12 UTC) #56
hanwenn
On 2020/02/09 09:55:12, hahnjo wrote: > On 2020/02/09 09:36:40, hanwenn wrote: > > On Sun, ...
2 weeks, 5 days ago (2020-02-09 10:08:20 UTC) #57
hanwenn
Jonas' comments
2 weeks, 5 days ago (2020-02-09 10:08:36 UTC) #58
hanwenn
https://codereview.appspot.com/561390043/diff/567180043/lily/include/smobs.hh File lily/include/smobs.hh (right): https://codereview.appspot.com/561390043/diff/567180043/lily/include/smobs.hh#newcode312 lily/include/smobs.hh:312: static size_t count; On 2020/02/09 03:29:26, Dan Eble wrote: ...
2 weeks, 5 days ago (2020-02-09 10:17:38 UTC) #59
hanwenn
comment
2 weeks, 5 days ago (2020-02-09 10:18:59 UTC) #60
hahnjo
LGTM, thanks for this clearly documented change!
2 weeks, 5 days ago (2020-02-09 11:46:36 UTC) #61
dak
https://codereview.appspot.com/561390043/diff/567180043/lily/include/smobs.hh File lily/include/smobs.hh (right): https://codereview.appspot.com/561390043/diff/567180043/lily/include/smobs.hh#newcode312 lily/include/smobs.hh:312: static size_t count; On 2020/02/09 03:29:26, Dan Eble wrote: ...
2 weeks, 5 days ago (2020-02-09 12:13:26 UTC) #62
dan_faithful.be
On Feb 9, 2020, at 04:25, jonas.hahnfeld@gmail.com wrote: > > Anyway I think adding a ...
2 weeks, 5 days ago (2020-02-09 12:58:56 UTC) #63
dak
On 2020/02/09 10:08:20, hanwenn wrote: > A way around that is to change all instances ...
2 weeks, 5 days ago (2020-02-09 23:20:36 UTC) #64
hanwenn
2 weeks, 1 day ago (2020-02-13 11:41:09 UTC) #65
commit a19aed147bf1605b21cbe7b1909ff6cbf519fb64
Author: Han-Wen Nienhuys <hanwen@lilypond.org>
Date:   Sat Feb 8 21:02:12 2020 +0100

    GUILE2: Scale GC heap with the number of smobs
Sign in to reply to this message.

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