[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Avoid Scheme-computed Skyline_pair values to get collected before us
From: |
dak |
Subject: |
Re: Avoid Scheme-computed Skyline_pair values to get collected before use (issue 12708048) |
Date: |
Mon, 12 Aug 2013 07:13:45 +0000 |
On 2013/08/12 05:50:10, Keith wrote:
On 2013/08/11 21:07:03, dak wrote:
> It's conceivable that this change helped,
> though there don't seem to be a lot of opportunities
> for garbage collection between return of the SCM
> value and use of the Skyline_pair.
There is ample opportunity for garbage generation and garbage
collection. The
call to maybe_pure_coordinate() estimates positions for everything in
the score,
which causes a lot of computation and memory use.
Adding the two tracer printf()s below, I see a SCM object holding the
skyline
for the "Presto" is returned, then two hundred twenty three other
skylines are
created, side-positioned, and discarded,
Independent of this issue, this seems, uh, excessive?
then the pointer into the skyline for
the "Presto" is accessed.
I think that the C++ memory allocator is independent of the Scheme
allocator (ly:format uses scm_malloc so it may garbage collect when
memory is tight, but that seems about the only use of scm_malloc). So
as long as the skylines calculated in between are not smobbed (turned
into Scheme objects), the Scheme garbage collector has no incentive to
run.
Some Scheme objects, however, are allocated whenever a symbol is
encountered for the first time, so there are a lot of code paths that
seem safe from triggering garbage collection _except_ when they are
run for the first time. While most symbols used via get_property or
similar are already somewhere present in permanent Scheme code (and
thus the symbols themselves are already allocated), the memoizing
construct used internally by get_property enters them into a weak hash
table to make sure they have a reference. Maintenance of that weak
hash table can still cause a Scheme allocation on the first run
through.
In short: it's just not a good idea to rely on no Scheme allocations
in a non-trivial code passage. At any rate, the circumstances of this
patch seem sufficient for retesting current master.
https://codereview.appspot.com/12708048/