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

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

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 1 month ago by hanwenn
Modified:
4 years, 1 month 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 ...
4 years, 1 month ago (2020-02-01 06:58:32 UTC) #1
hanwenn
nit
4 years, 1 month 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 ...
4 years, 1 month 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 ...
4 years, 1 month 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 ...
4 years, 1 month 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 > ...
4 years, 1 month ago (2020-02-01 20:49:09 UTC) #6
hanwenn
move to lily-guile.cc
4 years, 1 month 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: > ...
4 years, 1 month 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 ...
4 years, 1 month 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 ...
4 years, 1 month 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 ...
4 years, 1 month 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 ...
4 years, 1 month 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 ...
4 years, 1 month 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 ...
4 years, 1 month 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 > ...
4 years, 1 month ago (2020-02-02 13:14:15 UTC) #15
hanwenn
autoconf
4 years, 1 month 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 ...
4 years, 1 month ago (2020-02-02 13:21:09 UTC) #17
hanwenn
jonas' comments
4 years, 1 month 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 ...
4 years, 1 month 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, ...
4 years, 1 month ago (2020-02-02 13:45:34 UTC) #20
hanwenn
rebase
4 years, 1 month 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 ...
4 years, 1 month 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. ...
4 years, 1 month 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 ...
4 years, 1 month 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 ...
4 years, 1 month 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 ...
4 years, 1 month 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, ...
4 years, 1 month 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 ...
4 years, 1 month 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: > > > ...
4 years, 1 month 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 ...
4 years, 1 month 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: ...
4 years, 1 month 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 ...
4 years, 1 month 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 ...
4 years, 1 month 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: ...
4 years, 1 month 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 ...
4 years, 1 month 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 ...
4 years, 1 month 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 ...
4 years, 1 month 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> ...
4 years, 1 month 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 * ...
4 years, 1 month 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 ...
4 years, 1 month 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: >> >> ...
4 years, 1 month 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 ...
4 years, 1 month 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 ...
4 years, 1 month 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: ...
4 years, 1 month 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 ...
4 years, 1 month 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 ...
4 years, 1 month 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 ...
4 years, 1 month ago (2020-02-07 17:27:35 UTC) #47
dak
Han-Wen Nienhuys <hanwenn@gmail.com> writes: > Single threaded, the numbers make more sense: > > > ...
4 years, 1 month ago (2020-02-07 18:27:29 UTC) #48
hanwenn
hook into GC event
4 years, 1 month 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 ...
4 years, 1 month ago (2020-02-07 22:57:52 UTC) #50
hanwenn
Use smob count as memory proxy
4 years, 1 month 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 ...
4 years, 1 month 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 ...
4 years, 1 month 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 ...
4 years, 1 month 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 ...
4 years, 1 month 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> ...
4 years, 1 month 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, ...
4 years, 1 month ago (2020-02-09 10:08:20 UTC) #57
hanwenn
Jonas' comments
4 years, 1 month 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: ...
4 years, 1 month ago (2020-02-09 10:17:38 UTC) #59
hanwenn
comment
4 years, 1 month ago (2020-02-09 10:18:59 UTC) #60
hahnjo
LGTM, thanks for this clearly documented change!
4 years, 1 month 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: ...
4 years, 1 month 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 ...
4 years, 1 month 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 ...
4 years, 1 month ago (2020-02-09 23:20:36 UTC) #64
hanwenn
4 years, 1 month 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