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

Initial implementation of the Encore task library #125

Merged
merged 12 commits into from
Apr 21, 2015

Conversation

kikofernandez
Copy link
Contributor

Tasks are implemented as actors, where each scheduler has a specific task runner (actor for tasks). Tasks returns futures; at the moment, you cannot chain two tasks together to create dependencies between them, but you can create a task and chain it to a function (typical chain support).

there are some couple of examples for tasks:

  • global_async.enc, tests using async inside a global function
  • async_force_gc.enc, tests async creating many futures and forcing gc on the object
  • async_foreach.enc, tests async side effects, we don't block on the future produced by the task
  • async_chain.enc, simple example of chaining a task to an actor

@kikofernandez
Copy link
Contributor Author

@albertnetymk I have tested it under Vagrant but, could you test that it works on Linux?

@TheGrandmother
Copy link
Contributor

I ran the test on my machine(Ubuntu 14.10) and it works!

And don't forget to add tasks to the documentation :)

@kikofernandez
Copy link
Contributor Author

@TheGrandmother thanks! Doc added!

@TobiasWrigstad
Copy link
Contributor

Will tasks ever block explicitly on futures? I mean will there be a call to "block" inside the code of a task?

@kikofernandez
Copy link
Contributor Author

potentially, there could be. that case is not supported at the moment. what I mean is that there could be a case such that:

def process(a: Agent): Cat {
  let fut_cat = a.getCat() in
    get fut_cat
}

class Main
  def main(): void {
    ...
    let f = async(process(agent)) in 
      ...
  }

@albertnetymk
Copy link
Contributor

Works OK on my Linux. One comment on the syntax: async(f()), seems to suggest that async is one function, but it doesn't follow the call convention of other functions, call-by-value, I believe. One better way could be to view it as a keyword, like asyn f().

@kikofernandez
Copy link
Contributor Author

@albertnetymk it's a keyword, the parenthesis are placed there to improve readability! :)

@supercooldave
Copy link

I think the syntax should be
async { block }

@supercooldave
Copy link

@TobiasWrigstad regarding whether tasks will block. I see no reason why not.
But, get must have await semantics. That is, they must not block the actor that runs them.

@kikofernandez
Copy link
Contributor Author

yes, exactly (@supercooldave last comment), which means that I'll have to fix await

@kikofernandez
Copy link
Contributor Author

@supercooldave I have added documentation and an example with the requested syntax. E.g.

def square(x: int): int
  x * x

class Main
  def main(): void {
    repeat i <- 5
      async {
        let s = square(i) in
          print square(s)
      }
  }

@supercooldave
Copy link

Sweet!!
Next week we (with Daniel) can talk about compiling Party Types!

@kikofernandez kikofernandez force-pushed the features/active-tasks branch from bd0f4df to 5adb7d6 Compare April 18, 2015 17:19
@kikofernandez
Copy link
Contributor Author

I had to rebase on top of master and fix some conflicts. It would be great if someone with a Linux box could confirm that everything still works. For those who checked out this PR, you need to remove the local branch and pull again in order to get the new changes.

If someone is familiar with the new tests that @kaeluka created, it would be great if someone could confirm that what I see is normal:

await.out - differ: char 14, line 2
ERROR: test await failed with output:
vvvvvvvvvvvvvvvvvvvv
Before await
After await
17
While awaiting
^^^^^^^^^^^^^^^^^^^^
cmp: EOF on -
ERROR: test deadlock_yourself failed with output:
vvvvvvvvvvvvvvvvvvvv

^^^^^^^^^^^^^^^^^^^^
cmp: EOF on -
ERROR: test largestream failed with output:
vvvvvvvvvvvvvvvvvvvv

^^^^^^^^^^^^^^^^^^^^
parametricPrint_src/Pair.encore.c:26:111: warning: data argument not used by format string [-Wformat-extra-args]
  ..."(Expr.hs: type_to_printf_fstr not defined for a,Expr.hs: type_to_printf_fstr not defined for b)\n", _field...
     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~  ^
1 warning generated.
parametricPrint.out - differ: char 2, line 1
ERROR: test parametricPrint failed with output:
parametricPrint.out - differ: char 2, line 1                                                              [16/1998]
ERROR: test parametricPrint failed with output:
vvvvvvvvvvvvvvvvvvvv
(Expr.hs: type_to_printf_fstr not defined for a,Expr.hs: type_to_printf_fstr not defined for b)
^^^^^^^^^^^^^^^^^^^^
suspend.out - differ: char 16, line 2
ERROR: test suspend failed with output:
vvvvvvvvvvvvvvvvvvvv
Before suspend
After suspend
Interleaving!
^^^^^^^^^^^^^^^^^^^^
     Tests passed: 60/65
     Tests failed: 5/65
         await
         deadlock_yourself
         largestream
         parametricPrint
         suspend
make -C modules test
compiling 'Importer'
Importing module Lib from ./Lib.enc
compiling 'DeepImporter'
Importing module MidImport from ./MidImport.enc
Importing module DeepLib from ./DeepLib.enc
compiling 'FarImporter'
Importing module FarLib from lib/FarLib.enc
compiling 'DupImporter'
Warning: Module DupLib found in multiple places:
-- lib/DupLib.enc
-- lib/DupLib.enc
-- otherlib/DupLib.enc
compiling 'Qualified'
Importing module A.B.QLib from lib/A/B/QLib.enc

Module tests:
     Tests passed: 5/5
     Tests failed: 0/5

@albertnetymk
Copy link
Contributor

On my Linux, the result is the same except async_block. Because tasks are run in parallel, we can't specify the execution order.

@kikofernandez kikofernandez force-pushed the features/active-tasks branch from 5adb7d6 to 04f6c57 Compare April 20, 2015 09:51
@supercooldave supercooldave force-pushed the master branch 2 times, most recently from 70296c1 to c8d2a1f Compare April 20, 2015 20:57
@kikofernandez
Copy link
Contributor Author

@albertnetymk updated the test as requested :)

@supercooldave
Copy link

I've run this and it looks fine.
Any objections to merging it?

@albertnetymk
Copy link
Contributor

If I understand it correctly, the logic goes like this: for each iteration, thread local task_runner picks one task_msg from global task_q, and call handle_msg on it.

Possible improvement:
Uses dedicate class instead of Main class to construct task_runner, like cycle_detector
Uses dedicate task_handler instead of handle_msg to handle tasks, for some logic in handle_msg doesn't make sense for tasks.

Other than that, it looks good as the first implementation.

@kikofernandez
Copy link
Contributor Author

@albertnetymk yes, that's how it works.

First improvement: you are right, I took the main class because it's auto-generated and I need not change anything in the PonyRT (=> less conflicts when merging)

Second improvement: I do not agree, it's true that it could be optimized but I decided to treat every message in the same way. The handle_msg contains the logic for using the eager or lazy switch context and I would like the task runners to keep it in the same way. Otherwise we don't treat them as equal, when I believe the task runners do not need to concern with how to handle a message, they'll do whatever any other actor does (in the sense of lazy or eager).

Thanks for your feedback, it's always appreciated! 👍

@supercooldave
Copy link

Note that because this was open for so long, it now has conflicts with my recent commits -- I changed the AST a little.

Kiko Fernandez Reyes added 7 commits April 21, 2015 15:24
first implementation of the task library with futures.
tasks are run by a task runner actor who checks if there
are tasks to execute and, if so, picks up a task from the
mpmcq and runs the task. each `async` returns a future that's
fulfilled whenever the task runner runs its task. The program
doesn't finish until all the tasks are fulfilled!
there's high coupling between actors and tasks. refactor the code to alleviate
in some aspects this high coupling. this means moving around some header
files and extern variables / methods. Update the task library with const
qualifiers.
the current implementation works under mac and linux. however, there is
a problem when doing gc in the future. fulfilling a future fails by
accessing the gc->mark of an actor that seems to be gc'd. i have
disabled gc'ing futures for the time being until i can figure out what
is wrong. added test for tasks with side-effects and when no one is getting
on them.
there's no task trace function at the moment, which means that it's
needed to add each field in the task to be traced. otherwise, the actor
in which the task lives consideres that the task->env and task->dependencies
are garbage and should be collected.
update async_block test to omit order. it doesn't test that it doubles
the value but that the mod is 0. it tests that it uses `{`, `}` and the
return is valid, not really the execution (it's non-deterministic)
Kiko Fernandez Reyes added 3 commits April 21, 2015 15:25
this initial implementation does not support parallelism, since the task
is blocked as soon as it's defined.
the construct `finish` delays the blocking of the asyncs until the
end. Currently, the following form is supported:

  finish {
    async { e1, ... }
    async { e2, ... }
  }

and it's translated to:

  let x0 = async { e1, ... }
      x1 = async { e2, ... }
  in
    get x0;
    get x1
previously we allowed only `async` constructs inside the body of
`finish`. With this commit, you are allowed to enter other statements
and the compiler won't produce code to `get` (block) on them, since they
cannot be blocled.
@kikofernandez kikofernandez force-pushed the features/active-tasks branch from 78a8874 to 3f9bcb4 Compare April 21, 2015 13:27
@kikofernandez
Copy link
Contributor Author

@supercooldave thanks, I just fixed it.
I have pushed the finish language construct in this PR as well.

@supercooldave
Copy link

I will check over this tonight. It's be good if @albertnetymk gives his approval too.

@supercooldave
Copy link

(I hope you have squashed all that needs to be squashed.)

Kiko Fernandez Reyes added 2 commits April 21, 2015 17:32
the finish construct was not blocking on a task.
there is a current limitation, only top level asyncs declared
inside the body of the `finish` construct will be blocked.
on the other hand, it doesn't block on futures, just on Async AST nodes.
@kikofernandez kikofernandez force-pushed the features/active-tasks branch from 3f9bcb4 to 455ade3 Compare April 21, 2015 15:33
@supercooldave
Copy link

Looks good.

supercooldave pushed a commit that referenced this pull request Apr 21, 2015
Initial implementation of the Encore task library
@supercooldave supercooldave merged commit a4d74b5 into parapluu:master Apr 21, 2015
@supercooldave supercooldave deleted the features/active-tasks branch April 21, 2015 19:09
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.

5 participants