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:jonas.hahnf...@gmail.com> 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/
