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

[Quan Phan]

Quoting Zoltan Somogyi <zs at csse.unimelb.edu.au>:

 On 06-Mar-2008, Quan Phan <quan.phan at cs.kuleuven.be> wrote:
 > Normally by setting the GC_PRINT_STATS environment variable to a
 > value different from 0, the Boehm collector will print out some statistics
 > of its behaviour each time it is invoked. I modified it a bit so that it
 > reports on the total time spent on garbage collection.  
 > 
 > boehm_gc/alloc.c:
 > 	Accumulate the collection time into a global variable. I took the
 > 	calls to GET_TIME out of the if statements so that we can always
 > 	report on collection time with report_stats predicate. I tested it
 > 	with some programs and it seemed not affect the overall performance.
 
 I looked at the code, and since GET_TIME expands to a system call
 (getrusage),
 it can lead to a nontrivial slowdown. I therefore think that this change
 needs to be modified to record the time taken for gc only if the user asks
 for it.
 
 Unless you object, I will modify the diff tomorrow to accomplish this.
 Don't commit the parts of the diff related this.
 
There is no objection (at least from me). Please go ahead with the necessary
changes.

 > Index: runtime/mercury_region.h
 > ===================================================================
 > RCS file:
 /home/mercury/mercury1/repository/mercury/runtime/mercury_region.h,v
 > retrieving revision 1.11
 > diff -u -r1.11 mercury_region.h
 > --- runtime/mercury_region.h	23 Jan 2008 11:44:48 -0000	1.11
 > +++ runtime/mercury_region.h	6 Mar 2008 13:58:27 -0000
 > @@ -37,7 +37,7 @@
 >  
 >  /* XXX This should be made configurable when compiling a program. */
 >  #define     MR_REGION_NUM_PAGES_TO_REQUEST                  100
 > -#define     MR_REGION_PAGE_SPACE_SIZE                       255
 > +#define     MR_REGION_PAGE_SPACE_SIZE                       2048
 
 This looks like a bug: MR_REGION_PAGE_SPACE_SIZE should have a value
 that makes the size of struct MR_RegionPage_Struct a power of 2. With
 this definition, it won't be, because of the MR_regionpage_next field.
 The line you want is
 
 #define     MR_REGION_PAGE_SPACE_SIZE                       2047

You are right, if possible, please include your fix to the new diff.

Regards,
Quan.
 
 Zoltan.
 --------------------------------------------------------------------------
 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
 --------------------------------------------------------------------------
 



--------------------------------------------------------------------------
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
--------------------------------------------------------------------------