www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - Remove closure allocation

reply Malte <no valid.mail> writes:
I was trying to get a function that has a closure allocation to 
compile with  nogc.

Assuming this function:

int identity(immutable int q) pure nothrow  safe
{
    import std.algorithm;

    static immutable auto arr = [42];
    int getSecondArgument(int a, int b)
    {
        return b;
    }

    return arr.map!(a => getSecondArgument(a,q))[0];
}


When I add  nogc it complains about using q.
My idea was to change the closure to have another parameter with 
default initializer:

int identity(immutable int q) pure nothrow  safe  nogc
{
    import std.algorithm;

    static immutable auto arr = [42];
    int getSecondArgument(int a, int b)
    {
        return b;
    }

    return arr.map!((int a, int b = q) => getSecondArgument(a, 
 b))[0];
}


This compiles with DMD, however it returns random numbers instead 
of the value I passed in. Looks like a bug to me. Should that 
work or is there any other pattern I could use for that?
May 26 2018
parent reply Neia Neutuladh <neia ikeran.org> writes:
On Saturday, 26 May 2018 at 15:00:40 UTC, Malte wrote:

 This compiles with DMD, however it returns random numbers 
 instead of the value I passed in. Looks like a bug to me. 
 Should that work or is there any other pattern I could use for 
 that?


Filed as https://issues.dlang.org/show_bug.cgi?id=18910

As for the larger issue, you have to store `q` somewhere.

The compiler could store it on the stack, but the compiler would 
have to do a lot of work to prove that that's safe -- that the 
return value from `arr.map` doesn't escape the current function, 
that `map` doesn't save the thing you passed to it anywhere, that 
sort of thing.

And that sort of analysis is flaky. It's a recipe for code that 
compiles on one version of a compiler and not the next, a lot of 
bug reports that are hard to track down, that sort of thing. And 
it means that it has to assume the worst for functions that it 
doesn't have the source for -- when you pass a delegate, when you 
call an extern(D) function, when you call a virtual method.

So the compiler does the safe thing, and it has the GC allocate a 
bit of memory to hold `q`.

The way around that is the `scope` keyword. The compiler can do 
that complex analysis one function at a time; that's sufficiently 
simple. So if you wrote a `map` function, you could define it as 
taking a `scope U delegate(T)` and the compiler wouldn't need to 
use the GC there.

That obviously adds restrictions on how you can write that 
function.
May 26 2018
parent Malte <no valid.mail> writes:
On Saturday, 26 May 2018 at 18:10:30 UTC, Neia Neutuladh wrote:

 On Saturday, 26 May 2018 at 15:00:40 UTC, Malte wrote:

 This compiles with DMD, however it returns random numbers 
 instead of the value I passed in. Looks like a bug to me. 
 Should that work or is there any other pattern I could use for 
 that?


 Filed as https://issues.dlang.org/show_bug.cgi?id=18910

 As for the larger issue, you have to store `q` somewhere.

 The compiler could store it on the stack, but the compiler 
 would have to do a lot of work to prove that that's safe -- 
 that the return value from `arr.map` doesn't escape the current 
 function, that `map` doesn't save the thing you passed to it 
 anywhere, that sort of thing.

 And that sort of analysis is flaky. It's a recipe for code that 
 compiles on one version of a compiler and not the next, a lot 
 of bug reports that are hard to track down, that sort of thing. 
 And it means that it has to assume the worst for functions that 
 it doesn't have the source for -- when you pass a delegate, 
 when you call an extern(D) function, when you call a virtual 
 method.

 So the compiler does the safe thing, and it has the GC allocate 
 a bit of memory to hold `q`.

 The way around that is the `scope` keyword. The compiler can do 
 that complex analysis one function at a time; that's 
 sufficiently simple. So if you wrote a `map` function, you 
 could define it as taking a `scope U delegate(T)` and the 
 compiler wouldn't need to use the GC there.

 That obviously adds restrictions on how you can write that 
 function.


I had filed a bugreport already, but this is only slightly 
related. It might be the solution to my problem and just not 
working because it's a compiler bug, but could also be supposed 
to give me a compile error instead for some reasons I don't know 
yet.

I understand the general issue why a closure can be necessary, 
but it shouldn't be here, because all I have are value types 
(int) and strongly pure function calls. In fact even that 
immutable on q should be merely an optimization hint that the 
compiler could use the same register when inlining it instead of 
having to make a copy.
Adding scope and immutable at will wouldn't be an issue, but it 
doesn't solve the problem and wouldn't make sense to me if it did.

In fact, my actual issue is not that I need a  nogc function. I 
have a working code that is designed with that principle to have 
data transformation using pure functions and I want to 
parallelise it by just replacing map with taskPool.amap.
I had exactly that problem before and was more or less randomly 
trying to make changes to get rid of the "cannot access frame of 
function" error messages. However, if I try to make it a nogc 
function first, the compiler gives me useful information like 
"onlineapp.identity.__lambda4 closes over variable q at 
onlineapp.d(24)", so I can have a closer look at q instead of 
blindly guessing why the compiler thinks it needs another frame 
pointer.

What I did last time is rewrite the whole function to fit into a 
parallel foreach and I guess I have to do the same here too.

It's not that I can't rewrite it to something like

int identity(immutable int q) pure nothrow  safe  nogc
{
    static immutable auto arr = [42];
    int getSecondArgument(int a, int b)
    {
        return b;
    }

    foreach(i, a; arr) {
        auto tmp = getSecondArgument(a, q);
        if (i==0) return tmp;
    }
 
    assert(0);
}


, I just don't think I should have to rewrite it that much. But I 
currently see no other way to use taskPool and in general I see 
very little possibilities to utilize taskPool.amap even though it 
looks like a handy tool at first sight, but is just too limited 
when you actually try to do something with it.
May 27 2018