Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support shared in foreach lambdas #17910

Open
dlangBugzillaToGithub opened this issue Jun 19, 2019 · 5 comments
Open

Support shared in foreach lambdas #17910

dlangBugzillaToGithub opened this issue Jun 19, 2019 · 5 comments

Comments

@dlangBugzillaToGithub
Copy link

Manu reported this on 2019-06-19T03:30:57Z

Transferred from https://issues.dlang.org/show_bug.cgi?id=19984

CC List

  • Nicholas Wilson

Description

We want to arrive at a place where this code works:

struct S {
    int result;
    void inc(int i) shared { result.atomic!"+="(i); }
}
int sum(){
    S s;
    foreach(i; iota(1000).parallel){
        static assert(is(typeof(s) == shared(S)));
        s.inc(i);
    }
    static assert(is(typeof(s) == S));
    return s.result;
}



In this example, the opApply delegate would be `int delegate(ref int) shared`.
@dlangBugzillaToGithub
Copy link
Author

turkeyman commented on 2019-06-21T00:47:31Z

I will write the parallel() function (including the opApply), but the language needs to emit the correct lambda, and pass it to the opApply function.

@dlangBugzillaToGithub
Copy link
Author

iamthewilsonator commented on 2021-03-19T07:52:25Z

with explicit captures and no magic changing of type it is possible to do the below. It shouldn't be too hard to adapt it to multiple captured parameters. If this is acceptable to close the bug report with then it could be added to phobos.

```
import core.atomic;
import std.stdio;
import std.range;
struct S {
    int result;
    void inc(int i) shared { result.atomicOp!"+="(i); }
}
int main(){
    S s;
    
    foreach(i,_; iota(1000).parallel(capture!s)){
        _.s.inc(i);
    }
    int a;
    foreach (i; iota(1000))
    	a +=i;
    writeln(s.result); // 499500
    writeln(a);        // 499500
    return 0;
}

//alias capture(alias c) = Tuple!(shared typeof(c),__traits(identifier,c));
auto capture(alias c)()
{
    return Capture!c(c);
}

struct Capture(alias c)
{
    shared typeof(c)* ptr;
    this(ref typeof(c) _c)
    {
        ptr = cast(shared)&c;
    }
    ref opDispatch(string s)()
    {
        return *ptr;
    }
}
// a fake, minimal, not-parallel std.parallelism.parallel
auto parallel(R, C)(R r, scope C c)
{
    return ParallelCapture!(R, C)(r,c);
}
struct ParallelCapture(R, C)
{
    R range;
    C capture;
    this(R r, scope C c)
    {
        range = r;
        capture = c;
    }
    alias E = ElementType!R;
    alias NoIndexDg = int delegate(E, C);
    int opApply(scope NoIndexDg dg)
    {
        foreach(e; range)
        {
            if (dg(e,capture))
                return 1;
        }
        
        return 0;
    }
}
```

@dlangBugzillaToGithub
Copy link
Author

turkeyman commented on 2021-03-19T10:27:00Z

That's a pretty disappointing solution, because it leaves the massive un-safety that I'm trying to address in place.
In your example you can use `_.s` from the capture, but you can also still refer to `s`, which is the obvious thing to do, and it's a race waiting to be typed.
It's unreasonable for the interior of the for loop to have not-shared references to outer values.

Basically, I'm proposing that since the opApply implements the loop, and it defines what is safe to do from inside the loop body, an effective way to assert that function's specification of what is valid inner-loop code, is to infer the function attributed from the opApply function onto the lambda that it receives.

an `opApply(...) shared` method that inferred the `shared` attribute onto the loop body lambda feels like a really appropriate way to achieve this outcome.
It might also be reasonable that some loop body may not write to outer scope, and it could infer `const` the same way, etc.

@dlangBugzillaToGithub
Copy link
Author

iamthewilsonator commented on 2021-03-20T00:19:35Z

Hmm, point taken. It should be fairly easy to make opApply work with delegate that are shared, see https://issues.dlang.org/show_bug.cgi?id=21737

@dlangBugzillaToGithub
Copy link
Author

dfj1esp02 commented on 2021-06-10T19:55:16Z

Your example implicitly converts thread local data to shared:
---
struct S {
    int result;
    int[] data;
    void inc(int i) shared { result.atomic!"+="(i); }
}
shared int[] data;
int sum(){
    S s;
    s.data=[0,1,2];
    foreach(i; iota(1000).parallel){
        static assert(is(typeof(s) == shared(S)));
        s.inc(i);
        data=s.data;
    }
    static assert(is(typeof(s) == S));
    assert(s.data[0]==0,"now it's local and shared");
    return s.result;
}
---

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant