Web workers

[afinch7]

part of #1955 fixes

• tests/error_004_missing_module.test
• tests/error_005_missing_dynamic_import.test
• tests/error_006_import_ext_failure.test

part of #1222

part of #1047

[ry]

@afinch7 Have a look at the following disabled tests:

tests/error_004_missing_module.disabled
tests/error_005_missing_dynamic_import.disabled
tests/error_006_import_ext_failure.disabled

They involve the compiler worker. The problem, I think, is the exit(1) here:

deno/cli/workers.rs

Lines 88 to 91 in 129eae0

	if let Err(err) = r {
	eprintln!("{}", JSErrorColor(&err).to_string());
	std::process::exit(1);
	}

[afinch7]

@afinch7 Have a look at the following disabled tests:

tests/error_004_missing_module.disabled
tests/error_005_missing_dynamic_import.disabled
tests/error_006_import_ext_failure.disabled

They involve the compiler worker. The problem, I think, is the exit(1) here:
deno/cli/workers.rs

Lines 88 to 91 in 129eae0

if let Err(err) = r {
eprintln!("{}", JSErrorColor(&err).to_string());
std::process::exit(1);
}

Maybe we need a second resource for worker like a "stderr" for each worker.

[ry]

@afinch7 My inclination is that any JSError hit inside the worker should be set back to the caller. In the case of compile_sync() it would be good if it returned Result<ModuleMetaData, JSError>

deno/cli/compiler.rs

Lines 126 to 131 in c43cfed

	pub fn compile_sync(
	parent_state: Arc<IsolateState>,
	specifier: &str,
	referrer: &str,
	module_meta_data: &ModuleMetaData,
	) -> ModuleMetaData {

In the case of the compiler, the caller is clear. But in general it is not... so not sure how to handle that.

[afinch7]

I will at least give it a try for sure before I finish this commit. Maybe poll after each call or use futures::future::select_all?

[ry]

select_all is a very interesting idea!

You'll have to change it so both the worker and main isolate are running in the "main" tokio executor. That is, change the thread spawn into a tokio::spawn

[afinch7]

It took me way too long to figure out that tokio was waiting on the compiler worker future to complete after the main isolate finished. I ended up having to use a second tokio runtime for the compiler worker. I also decided against compile_sync returning a result for now. It might make sense to wrap compile() in compiler.ts with a try catch and post back a object with a success field, so we can have compiler failures and report them without causing the compiler worker isolate to exit.

[ry]

It seems to be green now. Nice! What was the unit_test hanging problem?

[afinch7]

Tokio scheduling. Had to use the same runtime for both spawns. e6a3bab

[ry]

This runtime is different than the tokio runtime? (defined by crate::tokio_util::run)
Ideally we could get all isolates using the same runtime.

[afinch7]

I tried that, but this leads to tokio waiting indefinitely for the compiler worker to exit. The compiler worker only has two ways to exit right now: calling workerClose as the worker or throwing a uncaught error. Maybe work on a better way to terminate the compiler worker later?

[ry]

Is it possible to refactor this into a compile_async and compile_sync which calls the async version? I'd like to experiment with parallel compilation at some point.

[afinch7]

Maybe in another PR? This would require a major rework of communication with the compiler worker. There is no way to relate responses directly to requests currently, and also no way to relate errors back to requests or even handle those errors without the compiler exiting(permanently).

[ry]

I'd like to land this PR soon - it's getting quite big and I think it's already an improvement over the existing code base. I'm worried it's going to get difficult to rebase.

My main concern now is removing the worker specific declaration/bundle/snapshot - I think it's too much complexity. Sharing the main isolate's tokio runtime is my other concern - but that can be done later if it's too much to do now.

[ry]

LGTM!

This is a massive improvement - thank you very much! I guess we'll have to iterate a bit more on workers - mostly I'd like to get them working in the same Tokio runtime.