Migrating Agents to greener pastures #1051
Conversation
| @@ -116,7 +108,7 @@ class Agent[T](initialValue: T, refFactory: ActorRefFactory, system: ActorSystem | |||
| * Dispatch a function to update the internal state. | |||
| */ | |||
| def send(f: T ⇒ T): Unit = { | |||
| def dispatch = updater ! Update(f) | |||
| def dispatch = updater.execute(new Runnable { def run = ref.single.transformAndGet(f) }) | |||
There was a problem hiding this comment.
We don't use the return value, so could use transform instead of transformAndGet.
|
Regarding remState + attach that's a remnant from when remState didn't call attach, good call. Attach is a conditional construct required for guaranteeing invariants, so it should most definitely be in remState. remState is also private, so it is under control. Thanks for reviewing Rich! I'll add tests to both the EC impl and the java API of Agent :-) Docs for Agents also needs rewriting |
|
Rewrote Agent docs and added some more tests, will commence writing some tests for the new ExecutionContext. |
|
Added tests for the new EC |
|
|
||
| private final val state = new AtomicInteger(0) | ||
| @tailrec private final def addState(newState: Int): Boolean = { | ||
| val c = state.get; |
There was a problem hiding this comment.
Come on √ - a semicolon? For real?
There was a problem hiding this comment.
Lol – I confess, I experimented having it as a one-liner. Residue must go!
|
Addressed Rich's nice catches :-) |
| sec.isEmpty must be === true | ||
| } | ||
|
|
||
| "execute 'throughput' number of tasks per sweep" in { |
There was a problem hiding this comment.
how does suspend interact with throughtput?
There was a problem hiding this comment.
I see below that is supposed to interrupt batches, but a test would be nice
There was a problem hiding this comment.
The test below makes sure that the EC is loaded with all the tasks before starting to process, so that it will resubmit after "throughput" number of tasks processed. suspension aborts the current batch, otherwise it wouldn't suspend?
|
Yes, I like it now! (and you know what that means ;-) ) |
|
Added suspension test :-) |
|
ah, I can’t take it, the SUSPENSE now it only need migration docs |
|
As soon as everyone has OK:ed this change in Agents, I'll add migration docs :-) |
|
Started jenkins job akka-pr-validator at https://jenkins.akka.io/job/akka-pr-validator/347/ |
|
jenkins job akka-pr-validator: Success - https://jenkins.akka.io/job/akka-pr-validator/347/ |
| sec.suspend() | ||
| def perform(f: Int ⇒ Int) = sec execute new Runnable { def run = counter.set(f(counter.get)) } | ||
| perform(_ + 1) | ||
| 1 to 10 foreach { _ ⇒ perform(identity) } |
There was a problem hiding this comment.
It adds 10 tasks to do nothing. We do this to simulate that other stuff gets added in between.
|
+1 |
|
LGTM. Really like the |
|
Oh, I almost forgot. The doc doesn't build. KITTEH PLS COME BACK. |
|
Thanks Björn! Currently fixing the docs |
|
LGTM: merge after KITTEH has approved |
|
Started jenkins job akka-pr-validator at https://jenkins.akka.io/job/akka-pr-validator/347/ |
|
jenkins job akka-pr-validator: Success - https://jenkins.akka.io/job/akka-pr-validator/347/ |
Migrating Agents to greener pastures
A re-envisioning of Agents