View mode: basic / threaded / horizontal-split · Log in · Help
June 20, 2011
what to do with postblit on the heap?
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).

-Steve
June 20, 2011
Re: what to do with postblit on the heap?
Steven Schveighoffer:

> 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.

This seems fixable. Is it possible to rewrite _d_arraycopy?


> Anyone else have any thoughts?

I think the current situation is not acceptable. This is a problem quite worse than _d_arraycopy because here some information is missing. Isn't this is the same problem with struct destructors?

A solution is to add this information at runtime, a type tag to structs that have a postblit and/or destructor. But then structs aren't PODs any more. There are other places to store this information, like in some kind of associative array.

Another solution is to forbid what the compiler can't guarantee. If a struct is going to be used only where its type is known, then it's allowed to have postblit and destructor. Is it possible to enforce this? I think it is. Here an @annotation is useful to better manage this contract between programmer and compiler.

Bye,
bearophile
June 20, 2011
Re: what to do with postblit on the heap?
On Mon, 20 Jun 2011 11:03:27 -0400, bearophile <bearophileHUGS@lycos.com>  
wrote:

> Steven Schveighoffer:
>
>> 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.
>
> This seems fixable. Is it possible to rewrite _d_arraycopy?

The compiler is the one passing the parameters to _d_arraycopy, so even if  
I change _d_arraycopy to accept a TypeInfo, the compiler needs to be fixed  
to send the TypeInfo.

I think this is really a no-brainer, because currently what is passed is  
the element size, which is contained within the TypeInfo.  I will be  
filing a bug on that.  But currently, I can't fix it.

>
>
>> Anyone else have any thoughts?
>
> I think the current situation is not acceptable. This is a problem quite  
> worse than _d_arraycopy because here some information is missing. Isn't  
> this is the same problem with struct destructors?

This is an easy fix -- the typeinfo contains information of whether or not  
and how to run the postblit.  The larger problem is the GC not calling the  
destructor.

But my immediate question is -- is it better to half-fix the problem by  
committing my changes, or leave the issue alone?

> A solution is to add this information at runtime, a type tag to structs  
> that have a postblit and/or destructor. But then structs aren't PODs any  
> more. There are other places to store this information, like in some  
> kind of associative array.

Any solution that fixes the GC problem will have to store the typeinfo  
somehow associated with the block.  I think we may have more traction for  
this problem with a precise GC.

I don't think the right route is to store type info inside the struct  
itself.  This added overhead is not necessary for when the struct is  
stored on the stack.

> Another solution is to forbid what the compiler can't guarantee. If a  
> struct is going to be used only where its type is known, then it's  
> allowed to have postblit and destructor. Is it possible to enforce this?  
> I think it is. Here an @annotation is useful to better manage this  
> contract between programmer and compiler.

This is a possibility, making a struct only usable if it's inside another  
such struct or inside a class, or on the stack.

-Steve
June 20, 2011
Re: what to do with postblit on the heap?
On 2011-06-20 11:56, Jose Armando Garcia wrote:
> On Mon, Jun 20, 2011 at 12:03 PM, bearophile <bearophileHUGS@lycos.com> 
wrote:
> > Steven Schveighoffer:
> > A solution is to add this information at runtime, a type tag to structs
> > that have a postblit and/or destructor. But then structs aren't PODs any
> > more. There are other places to store this information, like in some
> > kind of associative array.
> 
> What are PODs?

Plain Old Datatype. It's a user-defined data type with member variables but no 
functions. It just holds data.

- Jonathan M Davis
June 20, 2011
Re: what to do with postblit on the heap?
On 2011-06-20 10:34:14 -0400, "Steven Schveighoffer" 
<schveiguy@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.

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?

-- 
Michel Fortin
michel.fortin@michelf.com
http://michelf.com/
June 20, 2011
Re: what to do with postblit on the heap?
On Mon, 20 Jun 2011 16:45:44 -0400, Michel Fortin  
<michel.fortin@michelf.com> wrote:

> On 2011-06-20 10:34:14 -0400, "Steven Schveighoffer"  
> <schveiguy@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
June 20, 2011
Re: what to do with postblit on the heap?
On 2011-06-20 15:12, Steven Schveighoffer wrote:
> On Mon, 20 Jun 2011 16:45:44 -0400, Michel Fortin
> 
> <michel.fortin@michelf.com> wrote:
> > On 2011-06-20 10:34:14 -0400, "Steven Schveighoffer"
> > 
> > <schveiguy@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 an object is moved, neither the postblit nor the destructor should be 
called. The object is moved, not copied and destroyed. I believe that TDPL is 
very specific on that.

- Jonathan M Davis
June 20, 2011
Re: what to do with postblit on the heap?
On Mon, 20 Jun 2011 18:43:30 -0400, Jonathan M Davis <jmdavisProg@gmx.com>
wrote:

> On 2011-06-20 15:12, Steven Schveighoffer wrote:
>> 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 an object is moved, neither the postblit nor the destructor should be
> called. The object is moved, not copied and destroyed. I believe that  
> TDPL is
> very specific on that.

Well, I think in this case it is being copied.  It's put on the stack, and
then copied to the heap inside the runtime function.  The runtime could be
passed a flag indicating the append is really a move, but I'm not sure
it's a good choice.  To me, not calling the postblit and dtor on a moved
struct is an optimization, no?  And you can't re-implement these semantics
for a normal function.  The one case I can think of is when an rvalue is
allowed to be passed by reference (which is exactly what's happening here).

Is there anything a postblit is allowed to do that would break a struct if
you disabled the postblit in this case?  I'm pretty sure internal pointers
are not supported, especially if move semantics do not call the postblit.

-Steve
June 21, 2011
Re: what to do with postblit on the heap?
On 2011-06-20 18:12:11 -0400, "Steven Schveighoffer" 
<schveiguy@yahoo.com> said:

> On Mon, 20 Jun 2011 16:45:44 -0400, Michel Fortin  
> <michel.fortin@michelf.com> wrote:
> 
>> 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.

Well, if

	a ~= S();

does result in a temporary which get copied and then destroyed, why 
have move semantics at all? Move semantics are not just an 
optimization, they actually change the semantics. If you have a struct 
with a @disabled postblit, should it still be appendable?


> 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.

That depends. I'm not too sure currently whether the S destructor is 
called for this code:

	a ~= S();

If the compiler currently calls the destructor on the temporary S 
struct, then your patch is actually a fix because it balances 
constructors and destructors correctly for the appending part (the bug 
is then that compiler should use move semantics but is using copy 
instead). If it doesn't call the destructor then your patch does 
introduce a bug for this case.

All in all, I don't think it's important enough to justify we waste 
hours debating in what order we should fix those bugs. Do what you 
think is right. If it becomes a problem or it introduces a bug here or 
there, we'll adjust, at worse that means a revert of your commit.


>> 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.

The GC calling the destructor doesn't require precise scanning. 
Although it's true that both problems require adding type information 
to memory blocks, beyond that requirement they're both independent. 
It'd be really nice if struct destructors were called correctly.


-- 
Michel Fortin
michel.fortin@michelf.com
http://michelf.com/
June 21, 2011
Re: what to do with postblit on the heap?
On 2011-06-20 16:07, Steven Schveighoffer wrote:
> On Mon, 20 Jun 2011 18:43:30 -0400, Jonathan M Davis <jmdavisProg@gmx.com>
> 
> wrote:
> > On 2011-06-20 15:12, Steven Schveighoffer wrote:
> >> 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 an object is moved, neither the postblit nor the destructor should be
> > called. The object is moved, not copied and destroyed. I believe that
> > TDPL is
> > very specific on that.
> 
> Well, I think in this case it is being copied.  It's put on the stack, and
> then copied to the heap inside the runtime function.  The runtime could be
> passed a flag indicating the append is really a move, but I'm not sure
> it's a good choice.  To me, not calling the postblit and dtor on a moved
> struct is an optimization, no?  And you can't re-implement these semantics
> for a normal function.  The one case I can think of is when an rvalue is
> allowed to be passed by reference (which is exactly what's happening here).

Well, going from the stack to the heap probably is a copy. But moves shouldn't 
be calling the postblit or the destructor, and you seemed to be saying that 
they should. The main place that a move would occur that I can think would be 
when returning a value from a function, which is very different. And I don't 
think that avoiding the postblit is necessarily just an optimization. If the 
postblit really is skipped, then it's probably possible to return an object 
which cannot legally be copied (presumably due to some combination of 
reference or pointer member variables and const or immutable), though that 
wouldn't exactly be a typical situation, even if it actually is possible. It 
_is_ primarily an optimization to move rather than copy and destroy, but I'm 
not sure that it's _just_ an optimization.

> Is there anything a postblit is allowed to do that would break a struct if
> you disabled the postblit in this case?  I'm pretty sure internal pointers
> are not supported, especially if move semantics do not call the postblit.

If the struct had a pointer to a local member variable which the postblit 
would have deep-copied, then sure, not calling the postblit would screw with 
the struct. But that would screw with a struct which was returned from a 
function as well, and that's the prime place for the move semantics. That sort 
of struct is just plain badly designed, so I don't think that it's really 
something to worry about. I can't think of any other cases where it would be a 
problem though. Structs don't usually care where they live (aside from the 
issue of structs being designed to live on the stack and then not getting 
their destructor called because they're on the heap).

- Jonathan M Davis
« First   ‹ Prev
1 2 3
Top | Discussion index | About this forum | D home