--- Begin Message ---
|
Subject: |
Re: [PATCH] GC code cleanup #2 |
|
Date: |
Wed, 04 Jan 2006 18:16:31 +0100 |
|
User-agent: |
Gnus/5.110004 (No Gnus v0.4) Emacs/21.4 (gnu/linux) |
Hi,
address@hidden (Han-Wen Nienhuys) writes:
>> void
>>-scm_i_adjust_min_yield (scm_t_cell_type_statistics *freelist)
>>+scm_i_adjust_min_yield (scm_t_cell_type_statistics *freelist,
>>+ unsigned cells_collected,
>>+ unsigned cells_swept)
>
> you're passing this as a combination everywhere. I think it would be
> cleaner to create a
>
> scm_t_sweep_statistics
Right, good idea. ;-)
I followed you advice and below is a patch that does this.
Could someone please review it and apply it if it's acceptable?
Thanks,
Ludovic.
2006-01-04 Ludovic Courtès <address@hidden>
* libguile/gc-segment.c (scm_i_sweep_some_cards): Take a SWEEP_STATS
argument. Don't refer to SCM_GC_CELLS_COLLECTED and
SCM_CELLS_ALLOCATED. If SEG->FIRST_TIME, let CELLS_COLLECTED as zero.
Take into account SEG->SPAN when computing CELLS_SWEPT.
(scm_i_sweep_segment): Take one more argument, similarly.
(scm_i_sweep_all_segments): Likewise.
(scm_i_sweep_some_segments): Likewise.
(scm_i_adjust_min_yield): Change the way MIN_CELLS is computed: do not
refer to SCM_GC_CELLS_COLLECTED.
* libguile/gc-freelist.c (scm_i_adjust_min_yield): Take one more
argument, an `scm_i_sweep_statistics' object.
Change the way DELTA is collected: don't take into account
SCM_GC_CELLS_COLLECTED_1, only SWEEP_STATS.COLLECTED.
* libguile/gc-malloc.c (scm_realloc): Pass an extra argument
to `scm_i_sweep_all_segments ()'.
* libguile/gc.c (gc_start_stats): Updated accordingly.
(gc_end_stats): Take an additional SWEEP_STATS argument.
Decrement SCM_CELLS_ALLOCATED after calls to `scm_i_sweep_* ()'.
(scm_gc_for_newcell): Updated callers of `scm_i_sweep_*'.
Decrement SCM_CELLS_ALLOCATED.
(scm_i_gc): Likewise.
* libguile/private-gc.h (scm_i_sweep_*): Updated function
prototypes accordingly.
(scm_t_sweep_statistics): New type.
(scm_i_sweep_statistics_init): New macro.
(scm_i_sweep_statistics_sum): New macro
--- orig/libguile/gc-freelist.c
+++ mod/libguile/gc-freelist.c
@@ -78,7 +78,8 @@
*/
void
-scm_i_adjust_min_yield (scm_t_cell_type_statistics *freelist)
+scm_i_adjust_min_yield (scm_t_cell_type_statistics *freelist,
+ scm_t_sweep_statistics sweep_stats)
{
/* min yield is adjusted upwards so that next predicted total yield
* (allocated cells actually freed by GC) becomes
@@ -98,7 +99,7 @@
{
/* Pick largest of last two yields. */
long delta = ((SCM_HEAP_SIZE * freelist->min_yield_fraction / 100)
- - (long) SCM_MAX (scm_gc_cells_collected_1,
scm_gc_cells_collected));
+ - (long) sweep_stats.collected);
#ifdef DEBUGINFO
fprintf (stderr, " after GC = %lu, delta = %ld\n",
(unsigned long) scm_cells_allocated,
--- orig/libguile/gc-malloc.c
+++ mod/libguile/gc-malloc.c
@@ -105,6 +105,7 @@
scm_realloc (void *mem, size_t size)
{
void *ptr;
+ scm_t_sweep_statistics sweep_stats;
SCM_SYSCALL (ptr = realloc (mem, size));
if (ptr)
@@ -113,7 +114,7 @@
scm_i_scm_pthread_mutex_lock (&scm_i_sweep_mutex);
scm_gc_running_p = 1;
- scm_i_sweep_all_segments ("realloc");
+ scm_i_sweep_all_segments ("realloc", &sweep_stats);
SCM_SYSCALL (ptr = realloc (mem, size));
if (ptr)
@@ -124,7 +125,7 @@
}
scm_i_gc ("realloc");
- scm_i_sweep_all_segments ("realloc");
+ scm_i_sweep_all_segments ("realloc", &sweep_stats);
scm_gc_running_p = 0;
scm_i_pthread_mutex_unlock (&scm_i_sweep_mutex);
@@ -220,13 +221,14 @@
{
unsigned long prev_alloced;
float yield;
-
+ scm_t_sweep_statistics sweep_stats;
+
scm_i_scm_pthread_mutex_lock (&scm_i_sweep_mutex);
scm_gc_running_p = 1;
prev_alloced = mallocated;
scm_i_gc (what);
- scm_i_sweep_all_segments ("mtrigger");
+ scm_i_sweep_all_segments ("mtrigger", &sweep_stats);
yield = (((float) prev_alloced - (float) scm_mallocated)
/ (float) prev_alloced);
--- orig/libguile/gc-segment.c
+++ mod/libguile/gc-segment.c
@@ -140,15 +140,13 @@
scm_i_segment_card_count (seg) * SCM_GC_CARD_BVEC_SIZE_IN_LONGS *
SCM_SIZEOF_LONG);
}
-/*
- Sweep cards from SEG until we've gathered THRESHOLD cells
-
- RETURN:
-
- Freelist.
-*/
+/* Sweep cards from SEG until we've gathered THRESHOLD cells. On return,
+ *CELLS_SWEPT contains the number of cells that have been visited and
+ *CELLS_COLLECTED contains the number of cells actually collected. A
+ freelist is returned, potentially empty. */
SCM
-scm_i_sweep_some_cards (scm_t_heap_segment *seg)
+scm_i_sweep_some_cards (scm_t_heap_segment *seg,
+ scm_t_sweep_statistics *sweep_stats)
{
SCM cells = SCM_EOL;
int threshold = 512;
@@ -158,7 +156,7 @@
scm_t_cell * next_free = seg->next_free_card;
int cards_swept = 0;
-
+
while (collected < threshold && next_free < seg->bounds[1])
{
collected += (*sweeper) (next_free, &cells, seg);
@@ -166,14 +164,18 @@
cards_swept ++;
}
- scm_gc_cells_swept += cards_swept * (SCM_GC_CARD_N_CELLS -
SCM_GC_CARD_N_HEADER_CELLS);
- scm_gc_cells_collected += collected * seg->span;
+ sweep_stats->swept = cards_swept * seg->span
+ * (SCM_GC_CARD_N_CELLS - SCM_GC_CARD_N_HEADER_CELLS);
if (!seg->first_time)
- scm_cells_allocated -= collected * seg->span;
-
- seg->freelist->collected += collected * seg->span;
-
+ {
+ /* scm_cells_allocated -= collected * seg->span; */
+ sweep_stats->collected = collected * seg->span;
+ }
+ else
+ sweep_stats->collected = 0;
+
+ seg->freelist->collected += collected * seg->span;
if(next_free == seg->bounds[1])
{
@@ -196,31 +198,33 @@
segment again, the statistics are off.
*/
void
-scm_i_sweep_segment (scm_t_heap_segment * seg)
+scm_i_sweep_segment (scm_t_heap_segment *seg,
+ scm_t_sweep_statistics *sweep_stats)
{
+ scm_t_sweep_statistics sweep;
scm_t_cell * p = seg->next_free_card;
- int yield = scm_gc_cells_collected;
- int coll = seg->freelist->collected;
- unsigned long alloc = scm_cells_allocated ;
-
- while (scm_i_sweep_some_cards (seg) != SCM_EOL)
- ;
- scm_gc_cells_collected = yield;
- scm_cells_allocated = alloc;
- seg->freelist->collected = coll;
-
+ scm_i_sweep_statistics_init (sweep_stats);
+
+ while (scm_i_sweep_some_cards (seg, &sweep) != SCM_EOL)
+ scm_i_sweep_statistics_sum (sweep_stats, sweep);
+
seg->next_free_card =p;
}
void
-scm_i_sweep_all_segments (char const *reason)
+scm_i_sweep_all_segments (char const *reason,
+ scm_t_sweep_statistics *sweep_stats)
{
- int i= 0;
+ unsigned i= 0;
+ scm_i_sweep_statistics_init (sweep_stats);
for (i = 0; i < scm_i_heap_segment_table_size; i++)
{
- scm_i_sweep_segment (scm_i_heap_segment_table[i]);
+ scm_t_sweep_statistics sweep;
+
+ scm_i_sweep_segment (scm_i_heap_segment_table[i], &sweep);
+ scm_i_sweep_statistics_sum (sweep_stats, sweep);
}
}
@@ -317,29 +321,35 @@
}
SCM
-scm_i_sweep_some_segments (scm_t_cell_type_statistics * fl)
+scm_i_sweep_some_segments (scm_t_cell_type_statistics *fl,
+ scm_t_sweep_statistics *sweep_stats)
{
int i = fl->heap_segment_idx;
SCM collected = SCM_EOL;
-
+
+ scm_i_sweep_statistics_init (sweep_stats);
if (i == -1)
i++;
-
+
for (;
i < scm_i_heap_segment_table_size; i++)
{
+ scm_t_sweep_statistics sweep;
+
if (scm_i_heap_segment_table[i]->freelist != fl)
continue;
-
- collected = scm_i_sweep_some_cards (scm_i_heap_segment_table[i]);
+ collected = scm_i_sweep_some_cards (scm_i_heap_segment_table[i],
+ &sweep);
+
+ scm_i_sweep_statistics_sum (sweep_stats, sweep);
if (collected != SCM_EOL) /* Don't increment i */
break;
}
fl->heap_segment_idx = i;
-
+
return collected;
}
@@ -479,8 +489,7 @@
*/
float f = freelist->min_yield_fraction / 100.0;
float h = SCM_HEAP_SIZE;
- float min_cells
- = (f * h - scm_gc_cells_collected) / (1.0 - f);
+ float min_cells = (f * h - scm_gc_cells_collected) / (1.0 - f);
/* Make heap grow with factor 1.5 */
len = freelist->heap_size / 2;
--- orig/libguile/gc.c
+++ mod/libguile/gc.c
@@ -403,31 +403,32 @@
{
t_before_gc = scm_c_get_internal_run_time ();
- scm_gc_cells_marked_acc += (double) scm_gc_cells_swept
- - (double) scm_gc_cells_collected;
- scm_gc_cells_swept_acc += (double) scm_gc_cells_swept;
-
- scm_gc_cell_yield_percentage = ( scm_gc_cells_collected * 100 ) /
SCM_HEAP_SIZE;
-
- scm_gc_cells_swept = 0;
- scm_gc_cells_collected_1 = scm_gc_cells_collected;
-
- /*
- CELLS SWEPT is another word for the number of cells that were
- examined during GC. YIELD is the number that we cleaned
- out. MARKED is the number that weren't cleaned.
- */
- scm_gc_cells_collected = 0;
scm_gc_malloc_collected = 0;
scm_gc_ports_collected = 0;
}
static void
-gc_end_stats ()
+gc_end_stats (scm_t_sweep_statistics sweep_stats)
{
unsigned long t = scm_c_get_internal_run_time ();
scm_gc_time_taken += (t - t_before_gc);
+ /*
+ CELLS SWEPT is another word for the number of cells that were
+ examined during GC. YIELD is the number that we cleaned
+ out. MARKED is the number that weren't cleaned.
+ */
+ scm_gc_cells_marked_acc += (double) sweep_stats.swept
+ - (double) scm_gc_cells_collected;
+ scm_gc_cells_swept_acc += (double) sweep_stats.swept;
+
+ scm_gc_cell_yield_percentage = (sweep_stats.collected * 100) / SCM_HEAP_SIZE;
+
+ scm_gc_cells_swept = sweep_stats.swept;
+ scm_gc_cells_collected_1 = scm_gc_cells_collected;
+ scm_gc_cells_collected = sweep_stats.collected;
+ scm_cells_allocated -= sweep_stats.collected;
+
++scm_gc_times;
}
@@ -478,15 +479,19 @@
{
SCM cell;
int did_gc = 0;
-
+ scm_t_sweep_statistics sweep_stats;
+
scm_i_scm_pthread_mutex_lock (&scm_i_sweep_mutex);
scm_gc_running_p = 1;
- *free_cells = scm_i_sweep_some_segments (freelist);
+ *free_cells = scm_i_sweep_some_segments (freelist, &sweep_stats);
+ scm_cells_allocated -= sweep_stats.collected;
+
if (*free_cells == SCM_EOL && scm_i_gc_grow_heap_p (freelist))
{
freelist->heap_segment_idx = scm_i_get_new_heap_segment (freelist,
abort_on_error);
- *free_cells = scm_i_sweep_some_segments (freelist);
+ *free_cells = scm_i_sweep_some_segments (freelist, &sweep_stats);
+ scm_cells_allocated -= sweep_stats.collected;
}
if (*free_cells == SCM_EOL)
@@ -495,7 +500,7 @@
with the advent of lazy sweep, GC yield is only known just
before doing the GC.
*/
- scm_i_adjust_min_yield (freelist);
+ scm_i_adjust_min_yield (freelist, sweep_stats);
/*
out of fresh cells. Try to get some new ones.
@@ -504,7 +509,8 @@
did_gc = 1;
scm_i_gc ("cells");
- *free_cells = scm_i_sweep_some_segments (freelist);
+ *free_cells = scm_i_sweep_some_segments (freelist, &sweep_stats);
+ scm_cells_allocated -= sweep_stats.collected;
}
if (*free_cells == SCM_EOL)
@@ -513,7 +519,8 @@
failed getting new cells. Get new juice or die.
*/
freelist->heap_segment_idx = scm_i_get_new_heap_segment (freelist,
abort_on_error);
- *free_cells = scm_i_sweep_some_segments (freelist);
+ *free_cells = scm_i_sweep_some_segments (freelist, &sweep_stats);
+ scm_cells_allocated -= sweep_stats.collected;
}
if (*free_cells == SCM_EOL)
@@ -545,6 +552,8 @@
void
scm_i_gc (const char *what)
{
+ scm_t_sweep_statistics sweep_stats;
+
scm_i_thread_put_to_sleep ();
scm_c_hook_run (&scm_before_gc_c_hook, 0);
@@ -571,7 +580,12 @@
Let's finish the sweep. The conservative GC might point into the
garbage, and marking that would create a mess.
*/
- scm_i_sweep_all_segments("GC");
+ scm_i_sweep_all_segments ("GC", &sweep_stats);
+
+ /* Invariant: the number of cells collected (i.e., freed) must always be
+ lower than or equal to the number of cells "swept" (i.e., visited). */
+ assert (sweep_stats.collected <= sweep_stats.swept);
+
if (scm_mallocated < scm_i_deprecated_memory_return)
{
/* The byte count of allocated objects has underflowed. This is
@@ -624,7 +638,7 @@
scm_gc_sweep ();
scm_c_hook_run (&scm_after_sweep_c_hook, 0);
- gc_end_stats ();
+ gc_end_stats (sweep_stats);
scm_i_thread_wake_up ();
@@ -635,6 +649,7 @@
*/
}
+
/* {GC Protection Helper Functions}
*/
--- orig/libguile/private-gc.h
+++ mod/libguile/private-gc.h
@@ -118,11 +118,40 @@
} scm_t_cell_type_statistics;
+/* Sweep statistics. */
+typedef struct scm_sweep_statistics
+{
+ /* Number of cells "swept", i.e., visited during the sweep operation. */
+ unsigned swept;
+
+ /* Number of cells collected during the sweep operation. This number must
+ alsways be lower than or equal to SWEPT. */
+ unsigned collected;
+} scm_t_sweep_statistics;
+
+#define scm_i_sweep_statistics_init(_stats) \
+ do \
+ { \
+ (_stats)->swept = (_stats)->collected = 0; \
+ } \
+ while (0)
+
+#define scm_i_sweep_statistics_sum(_sum, _addition) \
+ do \
+ { \
+ (_sum)->swept += (_addition).swept; \
+ (_sum)->collected += (_addition).collected; \
+ } \
+ while (0)
+
+
+
extern scm_t_cell_type_statistics scm_i_master_freelist;
extern scm_t_cell_type_statistics scm_i_master_freelist2;
extern unsigned long scm_gc_cells_collected_1;
-void scm_i_adjust_min_yield (scm_t_cell_type_statistics *freelist);
+void scm_i_adjust_min_yield (scm_t_cell_type_statistics *freelist,
+ scm_t_sweep_statistics sweep_stats);
void scm_i_gc_sweep_freelist_reset (scm_t_cell_type_statistics *freelist);
int scm_i_gc_grow_heap_p (scm_t_cell_type_statistics * freelist);
@@ -221,8 +250,10 @@
void scm_i_clear_segment_mark_space (scm_t_heap_segment *seg);
scm_t_heap_segment * scm_i_make_empty_heap_segment
(scm_t_cell_type_statistics*);
-SCM scm_i_sweep_some_cards (scm_t_heap_segment *seg);
-void scm_i_sweep_segment (scm_t_heap_segment * seg);
+SCM scm_i_sweep_some_cards (scm_t_heap_segment *seg,
+ scm_t_sweep_statistics *sweep_stats);
+void scm_i_sweep_segment (scm_t_heap_segment *seg,
+ scm_t_sweep_statistics *sweep_stats);
void scm_i_heap_segment_statistics (scm_t_heap_segment *seg, SCM tab);
@@ -232,9 +263,11 @@
int scm_i_get_new_heap_segment (scm_t_cell_type_statistics *, policy_on_error);
void scm_i_clear_mark_space (void);
void scm_i_sweep_segments (void);
-SCM scm_i_sweep_some_segments (scm_t_cell_type_statistics * fl);
+SCM scm_i_sweep_some_segments (scm_t_cell_type_statistics *fl,
+ scm_t_sweep_statistics *sweep_stats);
void scm_i_reset_segments (void);
-void scm_i_sweep_all_segments (char const *reason);
+void scm_i_sweep_all_segments (char const *reason,
+ scm_t_sweep_statistics *sweep_stats);
SCM scm_i_all_segments_statistics (SCM hashtab);
void scm_i_make_initial_segment (int init_heap_size,
scm_t_cell_type_statistics *freelist);
_______________________________________________
Guile-devel mailing list
address@hidden
http://lists.gnu.org/mailman/listinfo/guile-devel
--- End Message ---