Refresh context only after thread switch

[EliasC]

This commit tries to keep the number of calls to pony_ctx to a minimum
by only updating it after a (potential) switch of threads, but then doing it
"across the stack" by using a pointer to a stack variable in the run
function of the scheduler. Most functions have been changed to pass
around this double pointer to the context (rather than just a pointer), and
the most important line is line 236 of encore.c:

*ctx = pony_ctx(); // Context might have gone stale, update it

No other part of the code should need to write to *ctx.

This commit fixes #434.

[kikofernandez]

BTW, this PR runs GCBench 11% faster than the current development branch when compiled with -O3!

[EliasC]

@albertnetymk: since me and @kikofernandez wrote this, could you review it?

[EliasC]

BTW, this PR runs GCBench 11% faster than the current development branch!

I could only confirm 3%, but that's still better!

[kikofernandez]

You should not use the Makefile from that benchmark but encorec -O3 GCBench.enc

[EliasC]

bash-3.2$ encorec -O3 GCBench.enc
bash-3.2$ time for i in `seq 10`; do ./GCBench; done
...
real    1m3.897s
user    1m58.835s
sys 0m3.335s

(6.39s average)

vs.

bash-3.2$ encorec -O3 GCBench.enc
bash-3.2$ time for i in `seq 10`; do ./GCBench; done
...
real    1m5.865s
user    2m4.810s
sys 0m3.342s

(6.59 average)

[kikofernandez]

in my machine, using your same loop:

-- Running GCBench from this PR
Elapsed (wall clock) time (h:mm:ss or m:ss): 0:49.53

-- Running GCBench in `development`
Elapsed (wall clock) time (h:mm:ss or m:ss): 0:53.83

This suggests a speedup of 8.6% (53.83 / 49.53)

[EliasC]

Nice!

[albertnetymk]

I originally thought suspend, await and get all need to taken care of in order to do this double pointer trick, but apparently, they use the same code for context manipulation. The final ctx update in future_block_actor looks redundant.

I was expecting clang to cache *ctx, but looking at the assembly, it seems to be doing deref all the time. I suggest to put pony_ctx_t **cctx as the first argument, and pony_ctx_t *ctx = *cctx as the first statement if ctx is needed in the current function.

  pony_gc_send(*ctx);
  pony_trace(*ctx, entry);
  pony_traceactor(*ctx, (pony_actor_t *)entry->actor);
  pony_send_done(*ctx);

I suggest using (void)var to surpass the unused var warning, instead of placing __attribute__((unused)) in the signature, so that the declaration in the header and the definition are consistent and clean.

The call to pony_ctx() in actor_run seems redundant.

[EliasC]

The final ctx update in future_block_actor looks redundant.

I thought we removed that! Fixed now

I was expecting clang to cache *ctx, but looking at the assembly, it seems to be doing deref all the time.

We were expecting the same! Fixed as per your suggestion now.

The call to pony_ctx() in actor_run seems redundant.

Agreed. Fixed.

[albertnetymk]

I think this can be merged. Would do so in an hour if no objections.

[kikofernandez]

To everyone:
I think it was great to have an open discussion in #517 which lead us to try a new and more complex implementation (this one) that fixes #434, allows us to re-introduce the ParT extract combinator and cleans most of the calls to encore_alloc and pony_alloc. Great team effort!