Hi,

ludovic.courtes@laas.fr (Ludovic Courtès) writes:

> Actually, I'm suspecting my gc-stats patch [0] to cause some GC problems
> (although this is quite unexpected since it touches only stat
> information).  For instance, consider the following program:
>
>   $ guile -e '(let loop ((a 0)) (loop a))'
>
> If `guile' is HEAD, then the memory consumption of that process (as
> shown by `top') continuously increases and at a pretty high rate (!).
> If `guile' is 1.8, then that does not happen.

I'm glad to announce that I just committed (in HEAD) a fix for this bug
(that I had introduced earlier, hmm...).

The issue is that in `scm_gc_for_newcell ()' (which is the main user of
GC sweeping), the global sweep statistics (`scm_gc_cells_collected' and
`scm_gc_cells_collected_1') did not get updated.  They only got updated
by full GCs (in `gc_end_stats ()') and consequently remained equal to
zero (since most of the collection took place in `scm_gc_for_newcell ()').
Thus, the GC was assuming that sweeping did not yield any collection and
consequently kept allocating new segments.

The patch (reproduced below) fixes this and further clarifies the flow
of GC sweep statistics.  In particular, it removes globals (most
importantly `scm_gc_cells_collected_1' and `scm_gc_cells_collected') so
that any function that needs to access this information must be
explicitly provided with it (see, for instance, `scm_i_adjust_min_yield'
and `scm_i_get_new_heap_segment').  As a result, I believe the code is
easier to audit now.

It all seems to work fine, but you may want to check it by yourself.  ;-)
As for merging it into 1.8, we should probably wait for some time (or
even not merge it)?

Thanks,
Ludovic.


2006-11-08  Ludovic Courtès  <ludovic.courtes@laas.fr>
	
	* libguile/gc-freelist.c (scm_i_adjust_min_yield): Take two
	"sweep_stats" arguments; use them instead of accessing the global
	variables `scm_gc_cells_collected' and `scm_gc_cells_collected_1'.

	* libguile/gc-segment.c (scm_i_sweep_some_cards): Reset SWEEP
	before each iteration of the loop.
	(scm_i_sweep_some_segments): Reset SWEEP at each iteration.
	(scm_i_get_new_heap_segment): Take an additional argument
	SWEEP_STATS.  Compute MIN_CELLS as a function of it.

	* libguile/gc.c (scm_gc_cells_collected,
	scm_gc_cells_collected_1): Removed.
	(scm_i_gc_sweep_stats, scm_i_gc_sweep_stats_1): New.
	(scm_gc_cells_marked_acc, scm_gc_cells_swept_acc,
	scm_gc_time_taken, scm_gc_mark_time_taken, scm_gc_times,
	scm_gc_cell_yield_percentage, protected_obj_count): Made `static'.
	(scm_gc_stats): Use `scm_i_gc_sweep_stats' instead of
	`scm_gc_cells_(collected|swept)'.
	(gc_update_stats): New.
	(gc_end_stats): Use `scm_i_gc_sweep_stats' and
	`scm_i_gc_sweep_stats_1' instead of the former globals.
	(scm_gc_for_newcell): Invoke `gc_update_stats ()' after each
	`scm_i_sweep_some_segments' call.  This fixes a bug where the GC
	would keep allocating new segments instead of re-using collected
	cells (because `scm_gc_cells_collected' would remain zero).

	* libguile/gc.h (scm_gc_cells_swept, scm_gc_cells_collected,
	scm_gc_cell_yield_percentage): Removed.

	* libguile/private-gc.h (scm_gc_cells_collected_1): Removed.
	(scm_i_adjust_min_yield): Updated.
	(scm_i_get_new_heap_segment): Updated.