Skip to content

Nit Actor Model, with some examples#2361

Merged
privat merged 16 commits intonitlang:masterfrom
BlackMinou:actors
Feb 28, 2017
Merged

Nit Actor Model, with some examples#2361
privat merged 16 commits intonitlang:masterfrom
BlackMinou:actors

Conversation

@BlackMinou
Copy link
Copy Markdown
Contributor

A better version than the previous PR !

The model injection and support module generation are now two phases in the frontend.

Needs #2357 to be able to compile with nitc, for now you have to use it as nitc module.nit -m actors_module.nit for compiling.

Copy link
Copy Markdown
Member

@privat privat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good job

Comment thread lib/actors/actors.nit Outdated
# Abstraction of an actor
# It has a mailbox, can receive and process messages asynchronously
abstract class Actor
super Thread
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indent

Comment thread lib/pthreads/pthreads.nit Outdated
fun wait(mutex: NativePthreadMutex) `{ pthread_cond_wait(self, mutex); `}
end


Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why a blank line?

Comment thread lib/actors/README.md Outdated
blocking, since it's direcly canceling the native pthread associated to the actor.

For now, there isn't any mecanism to recreate and actor after it was terminated.
Sending messages after terminating it results in unknown behaviour.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/unknown/unspecified/

@BlackMinou
Copy link
Copy Markdown
Contributor Author

test osx please

1 similar comment
@xymus
Copy link
Copy Markdown
Contributor

xymus commented Feb 16, 2017

test osx please

Copy link
Copy Markdown
Contributor

@xymus xymus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expect the use of pthreads::extra to break compatibility with macOS and maybe Android. The module doc is Offers some POSIX threads services that are not available on all platforms... While Android is not a priority you may have to add your tests to the darwin.skip blacklist file.

Comment thread lib/actors/README.md Outdated

## Waiting for all actors to finish processing

Let's imagine you create a whole bunch of actors and make them do things asynchronously from the main thread. Your don't want your program to exit right after giving work to your actors.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/Your/You/

The two sentences on the same line look like an oversight too.

Comment thread lib/actors/README.md Outdated
## Examples

You can find example of differents small programs implemented with Nit actors in the `/examples`
directory. For a really simple example, you can check `/examples/simple`.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these relative paths? Beginning with a / usually means the root of the FS or maybe of the depository.

Comment thread lib/actors/actors.nit Outdated
end
end

# Ends `self`, not responsible if work was ongoing
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"not responsible" does not say much, it looks like it cancels ongoing work. Or do you mean that it's dangerous?

Comment thread lib/actors/actors.nit
end

# A Blocking queue implemented from a `ConcurrentList`
# Corresponds to the mailbox of an actor
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should list which (3?) services are blocking. Other services like [], first and insert are not blocking nor do they unblock shift.

If you want to make this a public service, you should probably also implement pop, it's the opposite to push.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't really want people to use pop on a BlockingQueue, and this one is specific to my actors.

Ultimately, a future PR would be needed to abstract the concept of a BlockingQueue in the pthreads module I think. But there is enough things in this one to report this to a future one.

Comment thread lib/actors/actors.nit Outdated
mutex.unlock
end

# If the real collection is empty, we wait
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or you could precise here what stops the wait, like:

If empty, blocks until an item is inserted with `push` or `unshift`

super Phase

# List of actor classes
var actors = new Array[String]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm nitpicking now but the doc pretty much repeat the line below, may I suggest a tweak adding an important detail "Source code of the actor classes to generate"?

# Return signature
var ret = ""
# Return statement
var ret2 = ""
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comments are so clear, yet the variable names are not...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha, I must have inherited some habits from @privat

""")


# The actual proxy call
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation


var module_path = "{mmodule_path}/{module_name}.nit"

var nit_module = new NitModule(module_name + " is no_warning(\"missing-doc\")")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should set the modules annotations with nit_module.annotations.add "no_warning...".

Comment thread src/phase.nit Outdated
# @toimplement
fun process_nmodule(nmodule: AModule) do end


Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

@xymus
Copy link
Copy Markdown
Contributor

xymus commented Feb 16, 2017

test osx please

Signed-off-by: BlackMinou <romain.chanoir@viacesi.fr>
Signed-off-by: BlackMinou <romain.chanoir@viacesi.fr>
Signed-off-by: BlackMinou <romain.chanoir@viacesi.fr>
Signed-off-by: BlackMinou <romain.chanoir@viacesi.fr>
Signed-off-by: BlackMinou <romain.chanoir@viacesi.fr>
Signed-off-by: BlackMinou <romain.chanoir@viacesi.fr>
Signed-off-by: BlackMinou <romain.chanoir@viacesi.fr>
Signed-off-by: BlackMinou <romain.chanoir@viacesi.fr>
Copy link
Copy Markdown
Contributor

@xymus xymus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 and it looks OK on macOS after all.

Signed-off-by: BlackMinou <romain.chanoir@viacesi.fr>
Signed-off-by: BlackMinou <romain.chanoir@viacesi.fr>
Signed-off-by: BlackMinou <romain.chanoir@viacesi.fr>
Signed-off-by: BlackMinou <romain.chanoir@viacesi.fr>
Signed-off-by: BlackMinou <romain.chanoir@viacesi.fr>
Signed-off-by: BlackMinou <romain.chanoir@viacesi.fr>
Signed-off-by: BlackMinou <romain.chanoir@viacesi.fr>
Signed-off-by: BlackMinou <romain.chanoir@viacesi.fr>
privat added a commit that referenced this pull request Feb 28, 2017
A better version than the previous PR !

The model injection and support module generation are now two phases in the frontend.

Needs #2357 to be able to compile with `nitc`, for now you have to use it as `nitc module.nit -m actors_module.nit` for compiling.

Pull-Request: #2361
Reviewed-by: Alexis Laferrière <alexis.laf@xymus.net>
Reviewed-by: Jean Privat <jean@pryen.org>
@privat privat merged commit 17cd19a into nitlang:master Feb 28, 2017
for i in [0..p.length[ do p[i] = i

var i = count.length - 1
while i > 0 do
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Est-ce possible de faire une boucle à l'envers en nit -- au lieu d'un while:

for i in [count.length-1..0].step(-1) do

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oui, avec un reverse_iterator enfait

var first = p[0]
if p[first] != 0 then
p.copy_to(0, pp.length, pp, 0)
loop
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Est-ce qu'un simple while ne serait pas suffisant ici (avec la condition plus haut p[first] != 0 (donc pas besoin du loop et du break):

while p[first] != 0 do
...
end

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

les deux conditions sont différentes, l'une est sur p, et l'autre sur pp. Mais je peux quand même changer la boucle :)

if p[0] != 0 then
var flips = count_flips
maxflips = maxflips.max(flips)
if i % 2 == 0 then
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chk_sum += i % 2 == 0 ? +flips : -flips

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on a pas exactement d'opérateur ternaire comme cela en Nit, mais il y a une solution qui ressemble, je vais la faire !

var chk_sum = 0

var i = idx_min
loop
Copy link
Copy Markdown

@tremblay-guy tremblay-guy Mar 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Je n'aime pas les loop/break quand un while ou for suffit:

for i in [idx_min..idx_max] do
if i != idx_min then next_permutation

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok ok

for i in [0..n[ do count.add(0)

var task = 0
loop
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while (task = task_id.get_and_increment) < n_tasks do
run_task(task)
end

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

on peut pas faire d'assignation dans un while en Nit malheureusement !

p.copy_to(0, i+1, pp, 0)

for j in [0..i] do
if j + d <= i then
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

p[j] = (j+d <= i) ? pp[j+d] : pp[j+d-i-1]

chk_sums = new Array[Int].with_capacity(n_tasks)
for i in [0..n_tasks[ do chk_sums.add(0)

var actors = new Array[FannkuchRedux].with_capacity(8)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C'est quoi cette constante magique 8? => Constante symbolique!!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

un oubli de ma part, c'est nb_actors

var zi2 = cib[y]

var b = 0
for j in [0..49[ do
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Constante magique 49 => constante symbolique!

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c'est le nombre d'itérations qu'on effectue pour déterminer si le nombre appartient à l'ensemble de mandelbrot, je vais le mettre en constante symbolique

zr2 = nzr2
zi2 = nzi2

if zr1 * zr1 + zi1 * zi1 > 4.0 then b = b | 2
Copy link
Copy Markdown

@tremblay-guy tremblay-guy Mar 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ceci me semble plus simple et clair:

if zr1 * zr1 + zi1 * zi1 > 4.0 && zr2 * zr2 + zi2 * zi2 > 4.0 then
break
end

Serait encore plus simple avec une méthode auxiliaire...

Par contre, je constate que b est utilisé pour r... mais d'une façon que je ne comprends pas trop :(

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'avoue que je ne comprends pas trop non plus cette partie de l'algo, je préfère ne pas y toucher ...


fun work do
var line = 0
loop
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while (line = atomic.get_and_increment) < n do
put_line( line, data[line] )
end

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idem que pour fannkuch, on ne peut pas faire d'assignation dans un while en Nit, je vais quand même remettre le if sur une ligne par contre

var nb_threads = 8
end

if args.is_empty then
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

n = args.is_empty ? 200 : args[0].to_i

print chk.to_s + "\nPfannfuchen(" + n.to_s + ") = " + res.to_s

end

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

n = args.is_empty ? 7 : args[0].to_i

else
n = args[0].to_i
end

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pourquoi le +7?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bonne question, ça a l'air de fonctionner bien sans !

var samecount = 0

fun run do
loop
Copy link
Copy Markdown

@tremblay-guy tremblay-guy Mar 17, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

var p
while (p = place.meet(id, color)) != null do

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

toujours pas d'assignation dans les while :p

var colors: Array[String] = ["blue", "red", "yellow"]
end

fun complement(c, other: Int): Int do
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ouille. Pas moyen de faire ça avec une table?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

j'ait fait un Array[Array[Int]] à la place, mais c'est pas plus perf !

fun get_number(n: Int): String do
var str = ""
var nstr = n.to_s
for c in nstr do
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Est-ce que "+" en Nit crée une nouvelle chaine? (Comme en Java et Ruby?)

Est-ce qu'il existe un append comme en Ruby (<<), qui modifie la chaine sans en créer une nouvelle?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oui, le + crée une nouvelle chaine, faudrait utiliser un buffer pour faire du append, mais dans le cas là je vois pas trop l'interêt c'est pas comme si je le faisais souvent !


for c in creatures do c.async.run

active_actors.is_empty
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Que fait "is_empty"? Avec un tel nom, ça devrait être un prédicat, donc une méthode qui retourne un booléen... ce qui ne semble pas approprié ici puisque le résultat n'est pas utilisé.

Donc ça doit être une commande... mais avec un drôle de nom

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HAHA c'est une particularité de ma ReverseBlockingQueue, il faut que je la change mais je ferais ça plus tard, enfait le is_empty est bloquant sur cette liste tant qu'il reste des éléments dedans. C'est mon point de synchronisation pour savoir quand mes acteurs ont plus de message a traiter !


# Receive a token, then send it to `next` unless its value is `0`
fun send_token(message: Int) do
if message == 0 then
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if message == 0 then print id # Une seule ligne?

if message >= 1 then next.async... # Idem

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah oui, sûrement des restes de quand il y avait du code de debug a cet endroit !

var first = new ThreadRing(1)
var current = new ThreadRing(2)
first.next = current
for i in [3..503] do
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Constante magique 503 => Constante symbolique!!

privat added a commit that referenced this pull request Apr 5, 2017
Tried to follow @tremblay-guy commentaries on #2361  to improve the code of the actors examples

Pull-Request: #2401
Reviewed-by: Alexis Laferrière <alexis.laf@xymus.net>
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.

4 participants