Native code effects

[kayceesrk]

Native code effects (mostly) work. See examples in native-tests [0]. make test in this directory runs the tests. PR exists only for code review.

[0] https://github.com/ocamllabs/ocaml-effects/tree/effects-native/native-tests

[lpw25]

I've had a quick skim read through and it looks good to me. I notice that it creates a new stack for running the initial OCaml fiber. I think it would be better if it stayed on the C stack for the initial fiber (and for any callbacks into OCaml). This will make the behaviour of programs that don't use effects much closer to their current behaviour and improve the interaction with external tools like gdb. It would also make callbacks into OCaml cheaper, and may make C calls from the initial OCaml fiber cheaper as well.

[kayceesrk]

I had initially wanted to run the initial OCaml fiber on the C stack. But the specialization makes the code non-uniform at several places such including switching between OCaml stacks, C calls, callbacks and GC walking the OCaml stacks. This led to lot of duplication/specialization of code and runtime checks to see if we are on the main OCaml stack or a fiber. Hence, I'd ditched the idea for the initial prototype. I found it much nicer to assume that OCaml stacks are always on the heap.

That said, as you'd mentioned in the email conversation, I would be curious to benchmark the performance overhead of main OCaml stack as fiber before attempting this refactor. If it turns out that the performance overhead is non-trivial, we should definitely do the refactor.

@mshinwell mentioned that DWARF should be able to represent non-contiguous stacks. If that is the case, we should be able to provide gdb support for fibers. It would be quite useful to see the dynamic parent of fibers.

[kayceesrk]

@lpw25 @stedolan Are there any other comments on this? If not I'll merge this. The summary is that

• Overhead over vanilla is ~5.4% (using opam-bench-repo)
• sys-threads and vm-threads mostly work. Needs more work to get the tests to pass
• ocamldebug has no clue of the new stack layout
• backtrace needs to be fixed

Here are the tests that currently fail:

tests/asmcomp/fib
tests/asmcomp/tak
tests/asmcomp/quicksort
tests/asmcomp/quicksort2
tests/asmcomp/soli
tests/asmcomp/tagged-fib
tests/asmcomp/tagged-integr
tests/asmcomp/tagged-quicksort
tests/asmcomp/tagged-tak
tests/backtrace/backtrace.ml
tests/backtrace/backtrace.ml
tests/backtrace/backtrace.ml
tests/backtrace/backtrace.ml
tests/backtrace/backtrace.ml
tests/backtrace/backtrace.ml
tests/backtrace/backtrace.ml
tests/backtrace/backtrace.ml
tests/backtrace/backtrace2.ml
tests/backtrace/raw_backtrace.ml
tests/backtrace/backtrace_deprecated.ml
tests/backtrace/backtrace_slots.ml
tests/callback/native
tests/lib-systhreads/testfork.ml
tests/lib-threads/close.ml
tests/lib-threads/sieve.ml
tests/lib-threads/test1.ml
tests/lib-threads/test2.ml
tests/lib-threads/test3.ml
tests/lib-threads/test4.ml

The tests under asmcomp needs to be updated for the new native backend.

[lpw25]

I would still like to see a version which uses the C stack for the initial thread at some point: I think that a 5% slowdown for code inside a handler is probably acceptable, but a 5% slow down for all existing code is a hard sell.

But that certainly shouldn't stop you merging it, go right ahead.

[stedolan]

Why does tests/callback/native fail?

Also, the diff is currently unreadable because it includes the 4.02.2 changes. If you merge 860c670 into master first, this diff will get a lot shorter.

[kayceesrk]

@lpw25 I'd like to bring the overhead down. But I figured this could be done after the merge.

@stedolan I noticed the failure too, and reran the tests after fixing a bug with restoring the correct OCaml state after callbacks. That particular failure went away. More threads tests now pass:

tests/asmcomp/fib
tests/asmcomp/tak
tests/asmcomp/quicksort
tests/asmcomp/quicksort2
tests/asmcomp/soli
tests/asmcomp/tagged-fib
tests/asmcomp/tagged-integr
tests/asmcomp/tagged-quicksort
tests/asmcomp/tagged-tak
tests/backtrace/backtrace.ml
tests/backtrace/backtrace.ml
tests/backtrace/backtrace.ml
tests/backtrace/backtrace.ml
tests/backtrace/backtrace2.ml
tests/backtrace/raw_backtrace.ml
tests/backtrace/backtrace_deprecated.ml
tests/backtrace/backtrace_slots.ml
tests/lib-threads/sieve.ml
tests/lib-threads/test5.ml
tests/lib-threads/test6.ml
tests/lib-threads/test7.ml
tests/lib-threads/testsieve.ml
tests/lib-threads/torture.ml
tests/runtime-errors/stackoverflow.native
tests/tool-debugger/basic/
tests/tool-debugger/no_debug_event/

I've also merged the 4.02.2 changes from trunk to the master branch. The diff only includes relevant commits now.

[kayceesrk]

All of the thread (sys- & vm-) tests run successfully now!

I am not satisfied with the current design for OCaml callbacks from C in the new backend. In particular, OCaml callbacks run on a fresh stack, and not the stack of the parent OCaml function (if one exists). This isn't a problem usually, but it turns out to be very bad for timer interrupts. Every time a timer interrupt is handled, a new OCaml stack is created on the heap and discarded soon after.

There are two options to make this better. (1) Hang onto the callback stack instead of discarding it. The signal handler instances reuse the stack (2) Use the stack of the parent OCaml function. This solution is closer to the vanilla implementation. I am tempted to implement (2) since (1) involves more bookkeeping. But (2) also gets us closer to an implementation where the main OCaml function runs on the C stack.

[kayceesrk]

Callbacks now run on the parent stack instead of allocating a fresh stack (https://github.com/ocamllabs/ocaml-effects/commit/5d78b314d6a036264e05d6b265d7e3c3b0699dd0). Significantly better performance for sys-threads programs!

[kayceesrk]

@stedolan Ready to merge?

[kayceesrk]

Merging in this branch since there have been no new comments.