Re: Grow heap aggressively during music interpretation (issue 561390043 by address@hidden)

[hanwenn]

On 2020/02/09 09:55:12, hahnjo wrote:
> On 2020/02/09 09:36:40, hanwenn wrote:
> > On Sun, Feb 9, 2020 at 10:25 AM <mailto:address@hidden>
wrote:
> > > FWIW I tried to research on the internals of GC. I found the
following
> > > statement that decides whether to collect or just expand the heap:
> > > https://github.com/ivmai/bdwgc/blob/v8.0.4/alloc.c#L1435 We are
hit by
> > > the case
> > > (GC_fo_entries > (last_fo_entries + 500) && (last_bytes_finalized
|
> > > GC_bytes_finalized) != 0)
> > > Removing this case gets me to a state very close to this patch,
both in
> > > terms of performance and memory usage.
> > >
> > > If I understand the approach correctly, GC_fo_entries counts the
number
> > > of outstanding finalizations. It seems like Guile is making good
use of
> > > this functionality, so GC tries to reclaim that memory first
instead of
> > > just expanding the heap. Please note that the number 500 is
hard-coded,
> > > so we have no means to influence it via environment variables or
APIs. I
> > > guess something like that could be added, but it would again take
some
> > > time to bubble through all distributions that we care about.
> > 
> > Aha! That explains why we see such poor performance, because we rely
on
> > finalizers a lot. If we were to move away from finalizers and let
bdwgc
> > handle all of the memory management, then that it would also become
a
> > non-issue?
> 
> Disclaimer: I'm not yet very familiar with LilyPond's Scheme code. If
you refer
> to scm_set_smob_free / Smob::free_smob, then I think you are right. Is
there an
> easy way to get rid of this, maybe just for testing?

You could define operator new/delete in the smob class (using
scm_gc_malloc), and then let 
BDWGC handle memory for the smobs. The problem is that this will not
reclaim any
STL containers (ie. vectors) contained in the smobs, and especially
engravers are full
of them.

A way around that is to change all instances of vectors holding SCM
values to a new 
gc_vector type that has a custom allocator.  That will be significant
work, but probably
desirable in the long term. 

https://codereview.appspot.com/561390043/