Coroutine Experiments

[nadako]

I want to get this thing going, so I'm publishing my branch with some experiments.

I'm adding some tests and demos here (js-only for now): https://github.com/nadako/haxe/tree/coroutines/tests/misc/coroutines/src

[Simn]

What are your thoughts on expressing this in the type-system instead? This reminds me of the trace problem where we have a variable amount of arguments followed by PosInfos, except here we have the _hx_continuation argument instead.

[nadako]

I haven't thought about this. I wonder if TypeScript "mapped types" feature has something that could allow this, but I don't know to what extent that makes sense in Haxe. Also @Aurel300 is our type system expert, maybe he has some ideas?

We could also have compiler-supported callable abstracts for these specific cases, like CoroutineStart<(CoroArg1,CoroArg2)->CoroReturn> (and similar for .bind and trace), but I'm not sure if that'a good idea either...

[back2dos]

How does this relate to https://github.com/nadako/haxe-coroutines/ ? It seems you've settled for a quite different approach after all. How come?

[nadako]

How does this relate to https://github.com/nadako/haxe-coroutines/ ? It seems you've settled for a quite different approach after all. How come?

No, it's pretty much the same actually. The current difference is in the implementation - instead of generating classes, I use inner functions and the continuation itself is also a function type, simply because it's easier to implement and the generators already handle functions well. This might change when we'll get to optimizing and making it work on all the targets, but that's more or less an implementation detail.

[Simn]

What do you mean? If you compare https://github.com/nadako/haxe-coroutines/#generators and https://github.com/nadako/haxe/blob/coroutines/tests/misc/coroutines/src/TestGenerator.hx you will find that it is pretty much the same except for some names.

Edit: Was replying to @back2dos when nadako happened.

[back2dos]

Ah, ok. The substitution of Continuation for plain callbacks had me confused a little, because the design doc has something like:

suspend function getAnswer(arg:Bool):Int {
  if (arg)
    return 42;
  else
    throw "oh no";
}

The current API doesn't seem to allow the caller to retrieve that error in any way, so I was wondering if something changed, but I suppose it's just not there yet.

[nadako]

Yeah, I decided not to care about exceptions for now. With the callback continuations it could be just a nodejs-style argument, but I don't know if it's good or bad.

[Simn]

I'll experiment with some analyzer-specific data design changes on this branch so that we don't end up with silly conflicts. If this works we can factor out these changes into a separate PR.

[nadako]

FYI I was testing and playing a bit with coroutines outside of await/yield scenarios and come up with this nice example:

UhW3fjmbVt.mp4

[Simn]

It's really nice that we can do this with like 50 lines of code!

And the writing is better than most stuff on Netflix too.

[Simn]

I have merged development into this. Here are some observations:

• It still works!
• I was debating with myself if the coro flag is really needed at type-level. The transformer wouldn't necessarily need it because it has access to the expression, and the call unification could be taught to work without it as well. However, we definitely want to catch cases like this in unification:

function main() {
	var a:Coroutine<() -> Void> = null;
	var b:() -> Void = null;
	a = b; // () -> Void should be Coroutine<() -> Void>
}

This indeed makes it necessary to have the flag on TFun itself, even though the vast majority of the compiler doesn't need it.

• I'm not sure what to do with the type loading. We're currently introducing a magic type Coroutine similar to Dynamic, but I'm not sure if that's really acceptable in 2023. The problem is that this hijacks any unqualified type access to Coroutine, which isn't very elegant.
• There is a comment saying that something is wrong on one of the functions:

(* TODO: this is wrong *)
let coroutine_type ctx args ret =
	let args = args @ [("_hx_continuation",false,(tfun [ret; t_dynamic] ctx.com.basic.tvoid))] in
	let ret = ctx.com.basic.tvoid in
	TFun(args,ret,true)

I'm pretty sure I asked nadako about this before, but I forget what this is about and why it's wrong.

• It would be nice to decouple the transformation from the analyzer, but I don't have high hopes for that. The nice thing about the analyzer transformation is that it cleans up the control flow, which means we automatically deal with some of the crazier cases one could construct. On the other hand, it would be good to deal with the captured variable situation somehow.
• I might extract the TFun change out to reduce the diff. I'm still debating if it should be changed to a record, but that's a bit involved (481 occurrences) and something we can always do later.
• Unfortunately, I don't have a clear roadmap of what the next steps are here. I suppose the main question is how to get this working on targets other than JS. From my understanding, this mostly comes down to implementing Coroutine.suspend.

[Aurel300]

Chiming in with my regular "this is how it works in Rust" update: closures and coroutines in Rust generate a unique class per declaration. Each source code closure is a different instance, of a type that you cannot refer to from user syntax. However, the type implements a trait (~interface for us I guess), e.g. FnMut(i32, i32) -> i32 saying "this is a function that mutates some captured state, takes two int arguments and returns an int". I don't know how doable this is with the current approach here. If we were still generating anonymous classes, we could view Coroutine<...> as such an interface.

[Simn]

Isn't that more about how TFunction is handled in general? We leave that up to the generators, and some of them generate classes like that as well.

[Simn]

I have updated the branch again, which was a bit tricky with the recent refactoring. I hope I didn't break anything! The tests in misc/coroutines still work though.

I'll see if I can assemble a coroutine working group for Haxe 5 because I really want this as the next big feature. Will see what needs to be done here and how to make progress.

On that note, pinging @Aidan63 because this is at least adjacent to what he's been working on, and targets like hxcpp are likely going to be the most difficult to get right. I'd like to make sure that we're not designing something that's difficult to support on our "own" targets. The design we're going with is still this: https://github.com/nadako/haxe-coroutines, so if you could take a look at the overall situation here some time, that'd be appreciated!

[Aidan63]

Had a quick gander at the proposal as well as some code dumps generated by this branches ci builds and made some notes.

Having generators be able to know if a class implementing Continuation was auto generated by the compiler would be useful as you could then skip generating a lot of the GC guff you only really need with user created classes (type checking, static field registration, virtual functions, etc, etc).
The core Continuation type being implemented in hxcpp would also be useful as you can then avoid avoid the type erasure that usually happens with generics and therefore avoid boxing of ints, floats, bools, etc when used in the state machines (see the difference in generated C++ between Array<Int> which is implemented in hxcpp vs List<Int> which is all haxe). Another up side of having the core Continuation be in C++ for hxcpp is it would allow users to write properly typed externs / glue functions in C++ which interact nicely with coroutines.
The proposed type being an interface makes the above a bit of a pain as C++ doesn't really have interfaces so hxcpp's interface generation is not straight forward and is mostly passed around as dynamic. So you lose all types in the generated code and its a lot of blind casting which can lead to unrecoverable errors from haxe's point of view (calling a function on a null interface on the C++ target results in a native access violation which cannot be caught by haxe's catch and does not result in a nice stack trace).

[Aidan63]

Spent some time playing around with this some more this evening. I quickly threw together an implementation of Coroutine.suspend for hxcpp and got things working albeit with some minor manual patching of the generated code. The problem is the _hx_stateMachine function refers to itself which seems to throw things off for hxcpp.

_hx_stateMachine is passed into the constructor of the closure object before its been constructed and assigned, so any reference to that in the state machine function causes access violation exceptions. Not sure if this is an issue outside of this branch but I was able to work around it by manually patching all references to _hx_stateMachine with ::Dynamic(this) inside the closures actual code.

I quickly bodged together a coroutine for my hxcpp asys implementation and it seems to work fine, the file gets created and the file path is printed out when the coroutine ends.

import asys.native.filesystem.File;
import asys.native.filesystem.FilePath;
import asys.native.filesystem.FileSystem;
import asys.native.filesystem.FileOpenFlag;

class CoroFileSystem {
	@:coroutine static public function openFile<T>(path:FilePath, flag:FileOpenFlag<T>) : CoroFile {
		return Coroutine.suspend(cont -> {
			FileSystem.openFile(path, flag, (file, error) -> {
				switch error {
					case null:
						cont(new CoroFile(cast file), null);
					case exn:
						cont(null, exn);
				}
			});
		});
	}
}

class CoroFile {
	public final file : File;

	public function new(file) {
		this.file = file;
	}
}

@:coroutine function listen() : String {
	final file = CoroFileSystem.openFile("test.txt", FileOpenFlag.Write);

	return file.file.path;
}

function main() {
	listen.start((name, error) -> {
		trace(name);
	});
}

Things got more broken when I tried to build on it a bit more by exposing the write and close file functions.

import haxe.NoData;
import haxe.io.Bytes;
import asys.native.filesystem.File;
import asys.native.filesystem.FilePath;
import asys.native.filesystem.FileSystem;
import asys.native.filesystem.FileOpenFlag;

class CoroFileSystem {
	@:coroutine static public function openFile<T>(path:FilePath) : CoroFile {
		return Coroutine.suspend(cont -> {
			FileSystem.openFile(path, FileOpenFlag.Append, (file, error) -> {
				switch error {
					case null:
						cont(new CoroFile(file), null);
					case exn:
						cont(null, exn);
				}
			});
		});
	}
}

class CoroFile {
	public final file : FileAppend;

	public function new(file) {
		this.file = file;
	}

	@:coroutine public function write(text:String) : Int {
		return Coroutine.suspend(cont -> {
			file.write(Bytes.ofString(text), 0, text.length, (count, error) -> {
				switch error {
					case null:
						cont(count, null);
					case exn:
						cont(0, exn);
				}
			});
		});
	}

	@:coroutine public function close() : NoData {
		return Coroutine.suspend(cont -> {
			file.close((_, error) -> {
				switch error {
					case null:
						cont(_, null);
					case exn:
						cont(0, exn);
				}
			});
		});
	}
}

@:coroutine function listen() : String {
	final file = CoroFileSystem.openFile("test.txt");

	var count = 0;

	count += file.write("Hello, ");
	count += file.write("World!");

	file.close();

	return 'We wrote $count bytes to the file';
}

function main() {
	listen.start((name, error) -> {
		trace(name);
	});
}

The above sample gives C++ errors and looking at the generated code file and count variables are not assigned properly in the state machine!

Interestingly the pretty print dump of this looks fine, so appears to be a C++ generator specific issue? Another issue which has cropped up is that the write function code tries to reference a variable called __this, this is a special hxcpp generated variable which only exists in local functions, but the code its being called at isn't in a local function...so somethings getting confused somewhere.

This was all using that "coroutines without the tfun change" branch from the other day.

[Simn]

Thanks for looking into this! It's good to know that some things work, and we'll figure out the rest.

Really curious about the C++ code generation issue there. I don't know how it's possible for gencpp to generate "nothing" in cases like that. Generally, any coroutine transformation we make should also be writable by hand, unless there's some bad type somewhere. I'd look at it with -D dump (not pretty) to see if the typing seems good.

Actually it wouldn't be very surprising if there were some type issues because the only target this has been tested on is JavaScript, which doesn't care much about types.

[Simn]

I have updated the branch again. It's a pretty big conflict magnet due to the TFun changes, so I'd like to make the decision between this PR and #11554 soon. If nobody has any concerns there, I'd actually like to merge the changes into this branch so that we have all the discussion/code in one place.

[Aidan63]

The _hx_stateMachine issue seems to be because the analyser coro module manually builds the function but doesn't do the array trick that usually happens with local functions that reference themeselves.

function main() {
    // Lambdas can't refer to themselves.
    final lambda =() -> {
        lambda();
    };

    // local functions can.
    function local() {
        local();
    }

    // they get put into an array to be allowed to refer to themselves.
    final local = [ null ];

    local[0] = () -> {
        local[0]();
    }
}

Guessing the coroutine stuff gets done after that local function transform so its missed and javascript just so happens to support this?

Regarding the missing assignments, the normal non pretty dump looks fine as well. The issue seems to be those variables go through the CppExternRef case of variable assignment where nothing is assigned to them... (not sure under what circumstances this is used considering it doesn't assign anything).

         | CppExternRef(name, isGlobal) ->
            (* Printf.printf "%s - %s - %s - %s\n" class_name func_name name (tcpp_to_string rvalue.cpptype); *)
            if isGlobal then out " ::"; out (name ^ " = ");

this originates from when a TLocal name isn't found in gencpp's retypers declarations hashmap, why its not there, I don't know.

         | TLocal tvar ->
            let name = tvar.v_name in
            if (Hashtbl.mem !declarations name) then begin
               (*print_endline ("Using existing tvar " ^ tvar.v_name);*)
               CppVar(VarLocal(tvar)), cpp_type_of tvar.v_type
            end else begin
               (*print_endline ("Missing tvar " ^ tvar.v_name);*)
               Hashtbl.replace !undeclared name tvar;
               match has_var_flag tvar VCaptured with
               | true ->  CppVar(VarClosure(tvar)), cpp_type_of tvar.v_type
               | false ->
                  Printf.printf "1 - %s\n" name;
                  CppExtern(name,false), cpp_type_of tvar.v_type
            end

I have no looked into the __this issue yet but I wonder if all these are knock on effects from the weird non standard self referencing state machine closure.

[Aidan63]

Oh yes, and here's my hxcpp coroutine suspend code which just does what the existing js one does, but for hxcpp. Just have to change the suspend function in std types to call ::hx::Coroutine::suspend.

#pragma once

#include <hxcpp.h>

namespace hx
{
	struct Coroutine
	{
		static Dynamic suspend(Dynamic f, Dynamic cont)
		{
			struct SuspendCaller final : public hx::Object
			{
				Dynamic f;
				Dynamic cont;

				SuspendCaller(Dynamic inF, Dynamic inCont) : f(inF), cont(inCont)
				{
					HX_OBJ_WB_NEW_MARKED_OBJECT(this);
				}

				Dynamic HX_LOCAL_RUN()
				{
					return f(cont);
				}

				Dynamic __run(const Dynamic&, const Dynamic&) override
				{
					return HX_LOCAL_RUN();
				}

				Dynamic __Run(const Array<Dynamic>& args) override
				{
					return HX_LOCAL_RUN();
				}

				void __Mark(hx::MarkContext* __inCtx) override
				{
					HX_MARK_MEMBER(__inCtx);
				}

#ifdef HXCPP_VISIT_ALLOCS
				void __Visit(hx::VisitContext* __inCtx) override
				{
					HX_VISIT_MEMBER(__inCtx);
				}
#endif
			};

			return new SuspendCaller(f, cont);
		}
	};
}

[Aidan63]

Something I forgot to bring up the other day, possibly related to js not caring much about types. If a coroutine function doesn't actually suspend execution and returns void, that void will sneak through to the generators and gencpp will try and create a function where an argument has a type of void leading to cpp compiler errors.

@:coroutine function print() : Void {
	trace("Hello, World!");
}

function main() {
	print.start((_, error) -> {
		trace("done");
	});
}

Mousing over start also shows the void argument in the hover hint.

I'd imagine most typed targets will get upset at this.

[Simn]

I'll see if I can port the coroutine transformation out of the analyzer. I originally liked the idea to do it there and the structure supports it nicely, but there are too many issues with having it in there. I'm thinking that the coroutine transformation should be one of the first post-processing filters to run.

I think this should be doable in a way that we can still use most of the code in analyzerCoro.ml.

[Aidan63]

I updated the analyser coro file to use the array trick over my lunch break, but my hopes of it magically solving all those gencpp issues was probably a bit far fetched. While it solves that _hx_stateMachine issue and my first sample now compiles and runs with no manual twiddling, the more advance one still has missing assignments and unexpected usage of __this.

changes.patch

[Aidan63]

Ah, just realised the cause of the missing assignments. The local variables captured by the state machine function are never given the VCaptured flag. Setting VCaptured when it loops over all TVars used across states solves the issue.

[Simn]

Doing this without the analzyer seems... difficult. There's a lot of heavy lifting that we would have to implement again, and overall it feels like we would have to solve many problems we have already solved.

But keeping it in the analyzer doesn't seem ideal either, so I don't really know how to proceed with this.

[Simn]

Approaching this from the other direction, analyzerCoro doesn't actually need that much from the analyzer. By declaring these types and functions, the module will happily compile:

type suspend_call = {
	efun : texpr;      (* coroutine function expression *)
	args : texpr list; (* call arguments without the continuation *)
	pos : pos;         (* call position *)
}

type bb = {
	bb_pos : pos;
	bb_el : texpr DynArray.t;
	bb_syntax_edge : syntax_edge;
	bb_terminator : terminator_kind;
}

and syntax_switch = {
	ss_cases : (texpr list * bb) list;
	ss_default : bb option;
	ss_next : bb;
}

and syntax_edge =
	| SEIfThen of bb * bb * pos                               (* `if` with "then" and "next" *)
	| SEIfThenElse of bb * bb * bb * Type.t * pos             (* `if` with "then", "else" and "next" *)
	| SESwitch of syntax_switch                               (* `switch` with cases, "default" and "next" *)
	| SETry of bb * bb * (tvar * bb) list * bb *  pos         (* `try` with "exc", catches and "next" *)
	| SEWhile of bb * bb * pos                                (* `while` with "body" and "next" *)
	| SESubBlock of bb * bb                                   (* "sub" with "next" *)
	| SEMerge of bb                                           (* Merge to same block *)
	| SESuspend of (suspend_call * bb)                        (* Suspension point *)
	| SENone

and terminator_kind =
	| TermNone
	| TermCondBranch of texpr
	| TermReturn of pos
	| TermReturnValue of texpr * pos
	| TermBreak of pos
	| TermContinue of pos
	| TermThrow of texpr * pos

let declare_var g v bb =
	()

For this purpose, the syntax edges and terminators could be merged together, which would clean up the code a bit. The transformation from this data structure back to texpr also shouldn't be too difficult (see block_to_texpr_el which is quite straightforward). The hard part is going from texpr to this data structure, which is also the hard part in the real analyzer.

[Aidan63]

Another thing I've noticed, only static functions seem to work with the coroutine meta, member function don't work correctly (for cpp atleast). There appears to be a difference between the class field type and the inner TFunction return type.

CoroFile_obj - write
(text : String, _hx_continuation : ((Int, Dynamic) -> Void)) -> Void vs Void
CoroFile_obj - close
(_hx_continuation : ((haxe.NoData, Dynamic) -> Void)) -> Void vs Void
CoroFile_obj - static_close
(file : CoroFile, ((haxe.NoData, Dynamic) -> Void)) -> Void vs (Dynamic, Dynamic) -> Void

The first two entries there are member functions and the void's on the right hand side of the "vs" is the TFunction tf_type, these are coming out void when I'd expect them to be the return type of the left sides (class field) function type. That last static_close entry is a static function and the TFunction type comes out as expected.

[Simn]

Closing this in favor of #11554. Here's the transcript of nadako-chat which I just made to ensure that it still works:

import haxe.coro.Coroutine;
import js.Node.process;
import js.node.Readline;
import js.node.readline.Interface;

final rl = Readline.createInterface({
	input: process.stdin,
	output: process.stdout,
	prompt: "> "
});

@:coroutine function printChar(c:String) {
	return Coroutine.suspend(cont -> {
		process.stdout.write(c, () -> cont(null, null));
	});
}

@:coroutine function delay(ms:Int) {
	Coroutine.suspend(cont -> haxe.Timer.delay(() -> cont(null, null), ms));
}

@:coroutine function fancyPrint(text:String) {
	for (i in 0...text.length) {
		printChar(text.charAt(i));
		delay(100);
	}
	printChar("\n");
}

@:coroutine function promptLine() {
	rl.prompt();
	return Coroutine.suspend(cont -> {
		rl.on(InterfaceEvent.Line, line -> cont(line, null));
	});
}

function main() {
	@:coroutine function script() {
		fancyPrint("Hello");
		delay(1500);
		fancyPrint("Who are you?");
		switch (promptLine()) {
			case _.toLowerCase() => "world":
				fancyPrint("Haha, very funny... not!");
				return;
			case name:
				fancyPrint('Nice to meet you, $name!');
		}
		fancyPrint("Take care!");
	}
	script.start((res, err) -> rl.close());
}

Thank you for your work! This PR is what ultimately got me started on the coroutine implementation, and hopefully I'll be able to come through with a polished implementation.