Comments? Like, how can this break things?

By changing line 129 of phobos/internal/gc/gc.d from:

_gc.setFinalizer(p, &new_finalizer);

to:

///_gc.setFinalizer(p, &new_finalizer);
///
    ClassInfo c = ci;
    do
    {
        if (c.destructor)
        {
            _gc.setFinalizer(p, &new_finalizer);
        }
        c = c.base;
    } while (c);
///

gives me about 3x performance in allocating class objects w/o a dtor using the following code.

Before:
D::~this
C::~this
C::~this
0.829

After:
D::~this
C::~this
C::~this
0.258

;---

import std.date, std.stdio;

void main()
{
    C c = new C;
    D d = new D;
    E e;
    F f;

    d_time st = getUTCtime();
    for(int i = 0; i < 1_000_000; i++)
    {
        e = new E;
        f = new F;
    }
    d_time et = getUTCtime();
    writefln((et - st) / cast(double)TicksPerSecond);
}

class C
{
    int i;
    ~this()
    {
        printf("C::~this\n");
    }
}

class D : C
{
    int i;
    ~this()
    {
        printf("D::~this\n");
    }
}

class E
{
    int i;
}

class F : E
{
    int i;
}

Thanks,

- Dave

Dave wrote:
> 
> Comments? Like, how can this break things?
> 
> By changing line 129 of phobos/internal/gc/gc.d from:
> 
> _gc.setFinalizer(p, &new_finalizer);
> 
> to:
> 
> ///_gc.setFinalizer(p, &new_finalizer);
> ///
>     ClassInfo c = ci;
>     do
>     {
>         if (c.destructor)
>         {
>             _gc.setFinalizer(p, &new_finalizer);
>         }
>         c = c.base;
>     } while (c);
> ///
> 
> gives me about 3x performance in allocating class objects w/o a dtor using the following code.
> 
> Before:
> D::~this
> C::~this
> C::~this
> 0.829
> 
> After:
> D::~this
> C::~this
> C::~this
> 0.258
> 


Heh heh heh ;-)

Then, to identify "leaking" resources, the collector only has to check if there's a finalizer set :)

Argh - I forgot about needing to release synchronization resources or zeroing the vptr. regardless of if there is a dtor or not...

Damn.

Dave wrote:
> 
> Comments? Like, how can this break things?
> 
> By changing line 129 of phobos/internal/gc/gc.d from:
> 
> _gc.setFinalizer(p, &new_finalizer);
> 
> to:
> 
> ///_gc.setFinalizer(p, &new_finalizer);
> ///
>     ClassInfo c = ci;
>     do
>     {
>         if (c.destructor)
>         {
>             _gc.setFinalizer(p, &new_finalizer);
>         }
>         c = c.base;
>     } while (c);
> ///
> 
> gives me about 3x performance in allocating class objects w/o a dtor using the following code.
> 
> Before:
> D::~this
> C::~this
> C::~this
> 0.829
> 
> After:
> D::~this
> C::~this
> C::~this
> 0.258
> 
> ;---
> 
> import std.date, std.stdio;
> 
> void main()
> {
>     C c = new C;
>     D d = new D;
>     E e;
>     F f;
> 
>     d_time st = getUTCtime();
>     for(int i = 0; i < 1_000_000; i++)
>     {
>         e = new E;
>         f = new F;
>     }
>     d_time et = getUTCtime();
>     writefln((et - st) / cast(double)TicksPerSecond);
> }
> 
> class C
> {
>     int i;
>     ~this()
>     {
>         printf("C::~this\n");
>     }
> }
> 
> class D : C
> {
>     int i;
>     ~this()
>     {
>         printf("D::~this\n");
>     }
> }
> 
> class E
> {
>     int i;
> }
> 
> class F : E
> {
>     int i;
> }
> 
> Thanks,
> 
> - Dave

Dave wrote:
> 
> Argh - I forgot about needing to release synchronization resources or zeroing the vptr. regardless of if there is a dtor or not...
> 
> Damn.
> 

Maybe all is not lost, change gcx.setFinalizer to:

    void setFinalizer(void *p, GC_FINALIZER pFn)
    {
	// should be thread-safe - Threads.nthreads is
	//  mutex'd in std/thread.d
        if(Thread.nthreads > 1)
        {
        synchronized (gcLock)
        {
            gcx.finalizer = pFn;
            gcx.doFinalize(p);
        }
        }
        else
        {
            gcx.finalizer = pFn;
            gcx.doFinalize(p);
        }
    }

Now you get:

Before:
D::~this
C::~this
C::~this
0.827

After:
D::~this
C::~this
C::~this
0.466

- Dave

> Dave wrote:
>>
>> Comments? Like, how can this break things?
>>
>> By changing line 129 of phobos/internal/gc/gc.d from:
>>
>> _gc.setFinalizer(p, &new_finalizer);
>>
>> to:
>>
>> ///_gc.setFinalizer(p, &new_finalizer);
>> ///
>>     ClassInfo c = ci;
>>     do
>>     {
>>         if (c.destructor)
>>         {
>>             _gc.setFinalizer(p, &new_finalizer);
>>         }
>>         c = c.base;
>>     } while (c);
>> ///
>>
>> gives me about 3x performance in allocating class objects w/o a dtor using the following code.
>>
>> Before:
>> D::~this
>> C::~this
>> C::~this
>> 0.829
>>
>> After:
>> D::~this
>> C::~this
>> C::~this
>> 0.258
>>
>> ;---
>>
>> import std.date, std.stdio;
>>
>> void main()
>> {
>>     C c = new C;
>>     D d = new D;
>>     E e;
>>     F f;
>>
>>     d_time st = getUTCtime();
>>     for(int i = 0; i < 1_000_000; i++)
>>     {
>>         e = new E;
>>         f = new F;
>>     }
>>     d_time et = getUTCtime();
>>     writefln((et - st) / cast(double)TicksPerSecond);
>> }
>>
>> class C
>> {
>>     int i;
>>     ~this()
>>     {
>>         printf("C::~this\n");
>>     }
>> }
>>
>> class D : C
>> {
>>     int i;
>>     ~this()
>>     {
>>         printf("D::~this\n");
>>     }
>> }
>>
>> class E
>> {
>>     int i;
>> }
>>
>> class F : E
>> {
>>     int i;
>> }
>>
>> Thanks,
>>
>> - Dave

Dave wrote:
> Dave wrote:
>>
>> Argh - I forgot about needing to release synchronization resources or zeroing the vptr. regardless of if there is a dtor or not...
>>
>> Damn.
>>
> 
> Maybe all is not lost, change gcx.setFinalizer to:
> 
>     void setFinalizer(void *p, GC_FINALIZER pFn)
>     {
>     // should be thread-safe - Threads.nthreads is
>     //  mutex'd in std/thread.d
>         if(Thread.nthreads > 1)
>         {
>         synchronized (gcLock)
>         {
>             gcx.finalizer = pFn;
>             gcx.doFinalize(p);
>         }
>         }
>         else
>         {
>             gcx.finalizer = pFn;
>             gcx.doFinalize(p);
>         }
>     }

This will optimize the code path for single-threaded programs, but I would advise against ever calling setFinalizer from user code.  So far as I can tell, the setFinalizer call is an artifact of the days before D classes could have dtors.  While you'd think setFinalizer sets a per-object finalizer pointer, it actually sets a bit flag indicating that p should be finalized and then sets a global pointer to the finalizer function.  Thus:

    gc_setFinalizer( p, null );
    gc_fullCollect();

will actually cause all orphaned objects to not be finalized during the collection, as the GC's finalizer pointer will be null.

I think the GC should be restructured so that setting the global finalizer function is available as a separate option from the function that indicates p should be finalized.  In fact, the finalizer should probably either be an established extern (C) function or set via gc_init, assuming it doesn't live inside the GC code.

For Ares, I've decided to make the finalizer a named C function (rt_finalize), and GC allocator functions now accept a parameter to indicate whether the block should be finalized on deletion/collection.


Sean