On Mon, 20 Jun 2011 16:45:44 -0400, Michel Fortin
<[email protected]> wrote:
On 2011-06-20 10:34:14 -0400, "Steven Schveighoffer"
<[email protected]> 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
