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

Features/foreach parallel construct #146

Merged
merged 2 commits into from
Apr 27, 2015

Conversation

kikofernandez
Copy link
Contributor

Add foreach parallel construct that iterates over an array and executes the iterations in parallel (by using tasks behind the scenes). You cannot assume that the side-effects of the foreach construct have been applied (finished) by the end of foreach construct.

E.g.

class Main
  def main(): void {
    let master = new Master()
        a = [1, 2, 3, 4, 5]
    in 
      foreach item in a {
        master.add(item)
      }
  }

It's tested under Mac and Linux (vagrant) and has documentation. Feedback is welcome! :)

@supercooldave
Copy link

There seems to be two tests, but only output for one of them.
Have you tested cases such as: array has size 0, nested foreach loops?
Can you assign back into the array?

@kikofernandez
Copy link
Contributor Author

@supercooldave:
one test is edited since it uses the keyword foreach as the name of the function and foreach is now a reserved keyword.

  1. at the moment, I was assuming that the array had all the elements. the size of an array is known at compile time, and I am not sure that we have a construct for checking how many slots of the array are filled. Good catch.
  2. No nested loops as of now either, I can check on that if you think it's important.
  3. You cannot assign back at the moment, but if you would like that to be the default behaviour, I can fix it so that it returns either the array with all the items fulfilled (no futures) or the array with futures or get creative!

@supercooldave
Copy link

Re 1: I didn't mean that the array had uninitialised entries -- that's quite challenging to check, so let it be the programmer's problem.

Re 2: Nested loops can come later, but it will be important.

Re 3: What is the default behaviour (I haven't read the manual)?

@supercooldave
Copy link

It also seems that in now becomes a keyword.

@kikofernandez
Copy link
Contributor Author

in was already a keyword, wasn't it? We have the let ... in

@kikofernandez
Copy link
Contributor Author

Re Re 3: the body of the foreach is transformed into an async { body }

@supercooldave
Copy link

Re re re 3: D'oh!

@kikofernandez
Copy link
Contributor Author

Re Re 3: the body of the foreach is transformed into an async { body }

@supercooldave
Copy link

Re Re 3 (2): So you are saying, indirectly, that there is an array of futures returned, or what?

@kikofernandez
Copy link
Contributor Author

Re Re 3 (3) there's nothing returned at the moment. it's similar to using just the async for each iteration and not keeping the future. A foreach construct does not usually return anything, that would be a map, that's why I thought that we should be consistent with its semantics.

@supercooldave
Copy link

The semantics is fine. I just wanted to know what it is.

@kikofernandez
Copy link
Contributor Author

Not sure where we stand right now... 1 is no issue, 2 will come later and 3 means that a foreach construct returns unit type. Is there something that I should fix? (I'll create 2 as an issue as soon as this gets merged)

@supercooldave
Copy link

Nothing to fix. Maybe add a test case for empty arrays.

@supercooldave
Copy link

I get the following warning
foreach_construct_src/Main.encore.c:30:19: warning: incompatible pointer types passing '_enc__active_Master_t *' (aka 'struct _enc__active_Master_t *') to parameter of type 'pony_actor_t *' (aka 'struct pony_actor_t *') [-Wincompatible-pointer-types] pony_traceactor((*((struct _enc__task0_env_task*) p)).master);

I will merge as it passed all the tests (except the three pesky ones), but perhaps you should check that wraning

supercooldave pushed a commit that referenced this pull request Apr 27, 2015
Features/foreach parallel construct
@supercooldave supercooldave merged commit 7fce82f into parapluu:master Apr 27, 2015
@supercooldave supercooldave deleted the features/foreach branch April 27, 2015 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants