[m-rev.] For review: Change the Boehm collector to report on collection time.

[Quan Phan]

On Mon, Mar 17, 2008 at 06:14:26PM +1100, Zoltan Somogyi wrote:
> On 12-Mar-2008, Quan Phan <Quan.Phan at cs.kuleuven.be> wrote:
> > There is no objection (at least from me). Please go ahead with the necessary
> > changes.
> 
> The new diff follows, for review by anyone. It works on my test case,
> but I am just bootchecking it now. (Peter will need to look at the rename
> of --munmap.)

This change looks fine to me. There seem to be some style alignments
errors causing by cvs diff itself when not using expandtab in vim. I see
that the code in boehm_gc does not follow our coding standard but just
point out anyway. See belows.

> 
> >  #define     MR_REGION_PAGE_SPACE_SIZE                       2047
> > 
> > You are right, if possible, please include your fix to the new diff.
> 
> I committed that small change separately.

Thanks for doing this.

> 
> Zoltan.
> 
> Add a new MERCURY_OPTIONS option, --boehm-gc-calc-time. If given, we record
> the time taken by garbage collections so far, and make report_stats in
> library/benchmarking.m report this information.
> 
> boehm_gc/alloc.c:
> 	Gather this information.
> 
> boehm_gc/include/gc.h:
> 	Declare the variable that contains this information as well the
> 	variable that controls whether this information is gathered at all.
> 
> doc/user_guide.texi:
> 	Document the new option.
> 
> 	Document the renaming of another option that also applies only when
> 	using the Boehm collector.
> 
> library/benchmarking.m:
> 	Print the time taken by gc so far if we gather that information.
> 
> runtime/mercury_wrapper.c:
> 	Enable the facility in boehm_gc/alloc.c if --boehm-gc-calc-time is
> 	given. Rename the other Boehm-specific option.
> 
> 
> cvs diff: Diffing .
> cvs diff: Diffing analysis
> cvs diff: Diffing bindist
> cvs diff: Diffing boehm_gc
> Index: boehm_gc/alloc.c
> ===================================================================
> RCS file: /home/mercury/mercury1/repository/mercury/boehm_gc/alloc.c,v
> retrieving revision 1.17
> diff -u -b -r1.17 alloc.c
> --- boehm_gc/alloc.c	6 Feb 2008 01:45:48 -0000	1.17
> +++ boehm_gc/alloc.c	17 Mar 2008 06:57:41 -0000
> @@ -59,6 +59,12 @@
>  
>  word GC_gc_no = 0;
>  
> +GC_bool		GC_mercury_calc_gc_time = 0;
> +			   /* Accumulate total gc time  in      */
> +			   /* GC_total_gc_time if set to true.  */
> +unsigned long 	GC_total_gc_time = 0;
> +			   /* Measured in milliseconds.         */
> +
>  #ifndef SMALL_CONFIG
>    int GC_incremental = 0;  /* By default, stop the world.	*/
>  #endif
> @@ -318,8 +324,9 @@
>      	}
>      }
>      if (stop_func == GC_never_stop_func) GC_notify_full_gc();
> -    if (GC_print_stats) {
> +    if (GC_print_stats || GC_mercury_calc_gc_time) {
>          GET_TIME(start_time);
> +	if (GC_print_stats)
>  	GC_log_printf(
>  	   "Initiating full world-stop collection %lu after %ld allocd bytes\n",
>  	   (unsigned long)GC_gc_no+1, (long)GC_bytes_allocd);
> @@ -356,10 +363,17 @@
>        return(FALSE);

Here ...
>      }
>      GC_finish_collection();
> -    if (GC_print_stats) {
> +    if (GC_print_stats || GC_mercury_calc_gc_time) {
> +	unsigned long cur_gc_time;
>          GET_TIME(current_time);
> +        cur_gc_time = MS_TIME_DIFF(current_time,start_time);
> +        if (GC_print_stats) {
>          GC_log_printf("Complete collection took %lu msecs\n",
> -                  MS_TIME_DIFF(current_time,start_time));
> +                cur_gc_time);
> +	}
> +    	if (GC_mercury_calc_gc_time) {
> +            GC_total_gc_time += cur_gc_time;
> +	}
>      }
>      return(TRUE);
>  }

and here also.

Quan.
--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------