Re: [Help-smalltalk] Big objects and GC

[Paolo Bonzini]

> Here is the patch, all the tests are green ;-)

Nice patch!

I have a few comments.

> diff --git a/libgst/oop.c b/libgst/oop.c
> index ad264d9..018ec95 100644
> --- a/libgst/oop.c
> +++ b/libgst/oop.c
> @@ -66,6 +66,7 @@
>
>  /* Define this flag to turn on debugging code for OOP table management */
>  /* #define GC_DEBUGGING */
> +#define GC_DEBUGGING

Spurious change.

> @@ -701,7 +702,7 @@ _gst_make_oop_fixed (OOP oop)
>  {
>    gst_object newObj;
>    int size;
> -  if (oop->flags & F_FIXED)
> +  if ((oop->flags & F_FIXED) || (oop->flags & F_LARGE))
>      return;

Can you try representing large objects with F_FIXED but not F_OLD?  At 
the same time, makeFixed would not cause objects to become old, which is 
useful for objects passed to C.

Apparently, I even had some code to handle this in _gst_tenure_oop:

  if (oop->flags & F_OLD)
    return;

  if (!(oop->flags & F_FIXED))
    {
      int size = SIZE_TO_BYTES (TO_INT(oop->object->objSize));
      newObj = (gst_object) _gst_mem_alloc (_gst_mem.old, size);
      if (!newObj)
        abort ();

      memcpy (newObj, oop->object, size);
      _gst_mem.numOldOOPs++;

      oop->object = newObj;
    }

where the increment of numOldOOPs should be moved outside the if.

> @@ -780,6 +781,38 @@ _gst_alloc_obj (size_t size,
>  }
>
>  gst_object
> +_gst_alloc_large_obj (size_t size,
> +                      OOP *p_oop)
> +{
> +  gst_object p_instance;
> +
> +  size = ROUNDED_BYTES (size);
> +
> +  /* If the object is big enough, we put it directly in fixedspace.  */
> +  p_instance = (gst_object) _gst_mem_alloc (_gst_mem.fixed, size);
> +  if COMMON (p_instance)
> +    goto ok;
> +
> +  _gst_global_gc (size);
> +  p_instance = (gst_object) _gst_mem_alloc (_gst_mem.fixed, size);
> +  if COMMON (p_instance)
> +    goto ok;
> +
> +  _gst_compact (0);

Compacting is not going to be helpful, for fixed objects.

> @@ -1452,7 +1485,7 @@ _gst_sweep_oop (OOP oop)
>      _gst_make_oop_non_weak (oop);
>
>    /* Free unreachable oldspace objects.  */
> -  if UNCOMMON (oop->flags & F_FIXED)
> +  if UNCOMMON ((oop->flags & F_FIXED) || (oop->flags & F_LARGE))
>      {
>        _gst_mem.numOldOOPs--;

This makes stats wrong.  With the idea above, the code would be like:

  if ((oop->flags & F_LOADED) == 0)
    {
      if UNCOMMON (oop->flags & F_FIXED)
        _gst_mem_free (_gst_mem.fixed, oop->object);
      else if UNCOMMON (oop->flags & F_OLD)
        _gst_mem_free (_gst_mem.old, oop->object);
    }

  if UNCOMMON (oop->flags & F_OLD)
    {
      _gst_mem.numOldOOPs--;
      stats.reclaimedOldSpaceBytesSinceLastGlobalGC +=
        SIZE_TO_BYTES (TO_INT (OOP_TO_OBJ (oop)->objSize));
    }

It is a very good start though, thanks!

Paolo