--- Comment #38 from ---
(In reply to Walter Bright from comment #37)
> (In reply to timon.gehr from comment #36)
> > (In reply to Walter Bright from comment #35)
> > As I said, just do it if the delegate does not escape.
> Yes, I thought this was clear. This only applies if the delegate escapes.
> > > 2. Doing a heap alloc/free for every loop iteration is expensive, and 
> > > since
> > > the cost is hidden it will come as a nasty surprise.
> > > ...
> > 
> > I don't get how this is different from other implicit heap closure
> > allocations. A function that does such an implicit allocation not within a
> > loop in its own body might well be called in a loop and cause a performance
> > bottleneck. Profile.
> People have made it clear they don't particularly like hidden allocations in
> innocuous looking code.

Some people. Others just want obvious code to do the obvious thing.
Also, closures escaped from the loop body don't look innocuous.

> Hence the genesis of the @nogc attribute.

Exactly. I'm not proposing anything that would not be catched by @nogc.

> For this
> particular issue, it would be hard to look at a random loop and see if
> allocations are occurring

Not really. Annotate it with @nogc and you will see. This is really not a new
issue. This is about consistently applying an existing rule.

> - i.e. a nasty surprise if a small change suddenly
> made a big hit in performance. Profiling is not the answer, as very, very
> few people profile code.
> ...

A "big hit in performance" is noticeable.

> > > 3. Doing a gc allocation will generate an unbounded set of allocations, 
> > > also
> > > coming as a nasty surprise.
> > > ...
> > This is true for all implicit heap allocations. They are also often
> > convenient and there are already ways to flag them.
> I can hear the complaints about this already :-(
> ...

There will also be complaints about missing closure support.

> > > 4. Doing my delegate rewrite will cause a "by value" which changes the
> > > semantics, so that won't work.
> > > ...
> > That's not true.
> I believe it is:
>     int i = 0;
>     auto dg = { ++i; }
>     dg();
>     print(i);  // zero or one?
> By ref would make it one, with the rewrite it would be 0 because the ref
> would be to a copy of i, not the original.
> ...

No, the rewrite would wrap the loop body and nothing else. foreach loops have a
lowering similar to the following:

foreach(i; 0..n) delegates~=()=>i;


for(int __i=0;__i<n;__i++){
    int i=__i;

Here, the rewrite would be:

for(int __i=0;__i<n;__i++)(){
    int i=__i;
    delegates ~= ()=>i;

This has the same semantics as before, and every closure gets its own copy of
i, as this is a different variable for each of them (because it is declared in
the loop body).

If you just had the for loop:

for(int i=0;i<n;i++){

Then the (unnecessary!) rewrite would be:

for(int i=0;i<n;i++)(){

And all closures in the body get the same copy of i, as they all see the same
version. In this case, in the end, every closure would point to the same i and
the value of that i would be n. For this case, no additional implicit
allocations (gc or non-gc) would be necessary.

> > The delegate rewrite is a very hacky way
> > to implement it though, and probably not really the most convenient as you
> > need to thread through control flow and the generated code will not be as
> > good as it could be.
> Things get very sticky if you've got closures from functions nested several
> levels deep. It turns out to be pretty hard to get all this stuff right.
> Doing the rewrite means I know it will be correct, as it leverages all that
> complex, debugged, working code.
> ...

You know that that part will be correct. Wrapping the loop body into a function
does not in general result in valid code though. (E.g. break, continue, early
returns, interactions with scope.) Therefore, there need to be additional

> ...
> BTW, I belatedly realized that it wasn't just about loop variables.
> Capturing non-loop variables that get destructed when they go out of scope
> also cannot be done.

Yes, this is also an issue, though not related to the current one.


Reply via email to