[Mono-dev] [PATCH] Two patches to make SGen work on Darwin/x86

[Paolo Molaro]

On 03/24/10 Mark Probst wrote:
> Here's an updated patch that fixes this problem, both for mono-ehash
> as well as for mono-hash.


> --- a/mono/metadata/boehm-gc.c
> +++ b/mono/metadata/boehm-gc.c
> @@ -932,5 +932,11 @@ mono_gc_get_write_barrier (void)
>  
>  #endif
>  
> +void
> +mono_gc_invoke_without_moving (void (*func) (void*), void *data)
> +{
> +	func (data);
> +}
> +

Please introduce a typedef for the function pointer type.
I think the naming here is misleading, since the issue is not just about
moving the objects, but also about them being collected.
Moreover, I think the feature should be actually implemented in Boehm
since it's provided by the library and it can be useful for other cases,
too.

> --- a/mono/metadata/sgen-gc.c
> +++ b/mono/metadata/sgen-gc.c
> @@ -597,6 +597,7 @@ safe_object_get_size (MonoObject* o)
>   * ######################################################################
>   */
>  static LOCK_DECLARE (gc_mutex);
> +static LOCK_DECLARE (interruption_mutex);

It may be better to put the declaration of the mutexes far from each
other so that there are more chances they'll end up in separate
cachelines and hence provide an effective advantage wrt using a single
mutex.

>  static int gc_disabled = 0;
>  static int num_minor_gcs = 0;
>  static int num_major_gcs = 0;
> @@ -3337,6 +3338,8 @@ collect_nursery (size_t requested_size)
>  	TV_DECLARE (atv);
>  	TV_DECLARE (btv);
>  
> +	LOCK_INTERRUPTION;

You need to put the lock acquisition at the start of
stop_world(), here it can cause a deadlock, since a thread may be
stopped while holding interruption_mutex and this call here
will wait indefinitely.
Also, to prepare for the future changes, introduce two functions:
void acquire_gc_locks (void) and void release_gc_locks (void) and
call them in stop_world()/restart().

The rest looks ok to me.

lupus

-- 
-----------------------------------------------------------------
lupus at debian.org                                     debian/rules
lupus at ximian.com                             Monkeys do it better