what to do with postblit on the heap?

Steven Schveighoffer schveiguy at yahoo.com
Mon Jun 20 15:12:11 PDT 2011


On Mon, 20 Jun 2011 16:45:44 -0400, Michel Fortin  
<michel.fortin at michelf.com> wrote:

> On 2011-06-20 10:34:14 -0400, "Steven Schveighoffer"  
> <schveiguy at yahoo.com> said:
>
>> I have submitted a fix for bug 5272,   
>> http://d.puremagic.com/issues/show_bug.cgi?id=5272 "Postblit not called  
>> on  copying due to array append"
>>  However, I am starting to realize that one of the major reasons for   
>> postblit is to match it with an equivalent dtor.
>>  This works well when the struct is on the stack -- the posblit for   
>> instance increments a reference counter, then the dtor decrements the  
>> ref  counter.
>>  But when the data is on the heap, the destructor is *not* called.  So  
>> what  happens to any ref-counted data that is on the heap?  It's never   
>> decremented.  Currently though, it might still work, because postblit   
>> isn't called when the data is on the heap!  So no increment, no  
>> decrement.
>>  I think this is an artificial "success".  However, if the pull request  
>> I  initiated is accepted, then postblit *will* be called on heap  
>> allocation,  for instance if you append data.  This will further  
>> highlight the fact  that the destructor is not being called.
>>  So is it worth adding calls to postblit, knowing that the complement   
>> destructor is not going to be called?  I can see in some cases where it  
>>  would be expected, and I can see other cases where it will be  
>> difficult to  deal with.  IMO, the difficult cases are already broken  
>> anyways, but it  just seems like they are not.
>>  The other part of this puzzle that is missing is array assignment,  
>> for  example a[] = b[] does not call postblits.  I cannot fix this  
>> because  _d_arraycopy does not give me the typeinfo.
>>  Anyone else have any thoughts?  I'm mixed as to whether this patch  
>> should  be accepted without more comprehensive GC/compiler reform.  I  
>> feel its a  step in the right direction, but that it will upset the  
>> balance in a few  places (particularly ref-counting).
>
> My feeling is that array appending and array assignment should be  
> considered a compiler issue first and foremost. The compiler needs to be  
> fixed, and once that's done the runtime will need to be updated anyway  
> to match the changes in the compiler. Your proposed fix for array  
> assignment is a good start for when the compiler will provide the  
> necessary info to the runtime, but applying it at this time will just  
> fix some cases by breaking a few others: net improvement zero.

BTW, I now feel that your request to make a distinction between move and  
copy is not required.  The compiler currently calls the destructor of  
temporaries, so it should also call postblit.  I don't think it can make  
the distinction between array appending and simply calling some other  
function.

If the issue of array assignment is fixed, do you think it's worth putting  
the change in, and then filing a bug against the GC?  I still think the  
current cases that "work" are fundamentally broken anyways.

For instance, in a ref-counted struct, if you appended it to an array,  
then removed all the stack-based references, the ref count goes to zero,  
even though the array still has a reference (I think someone filed a bug  
against std.stdio.File for this).

> As for the issue that destructors aren't called for arrays on the heap,  
> it's a serious problem. But it's also a separate problem that concerns  
> purely the runtime, as far as I am aware of. Is there someone working on  
> it?

I think we need precise scanning to get a complete solution.  Another  
option is to increase the information the array runtime stores in the  
memory block (currently it only stores the "used" length) and then hook  
the GC to call the dtors.  This might be a quick fix that doesn't require  
precise scanning, but it also fixes the most common case of allocating a  
single struct or an array of structs on the heap.

-Steve


More information about the Digitalmars-d mailing list