Runtime worker threads #7089

NikVolf · 2020-09-11T13:57:36Z

examples
much more tests

NikVolf · 2020-09-11T13:59:03Z

client/executor/src/async_externalities.rs

+
+type StorageValue = Vec<u8>;
+
+impl Externalities for AsyncExternalities {


This part could be much simpler if we refactored storage access and stuff that is now in extensions into capability-based externalities.

pepyakin

First brief review.

primitives/io/src/tasks.rs

client/executor/src/native_executor.rs

cheme

I started looking a bit in this PR, super nice stuff 👍
But I did not really follow how native is using AsyncExternalities (see comments).
I also start to wonder, since we are sharing a SpawnedNamed for is handling the different calls: should we try to implement some sync at the end of RuntimeInstanceSpawn lifetime (eg to kill sibling threads on panic from the with_externalities_safe).
Similarily should we wait for all threads before completion (if a thread panic but there is no join to wait for it we possibly could have failure or success depending on the scheduling)?

client/executor/runtime-test/src/lib.rs

client/executor/src/async_externalities.rs

client/executor/runtime-test/src/lib.rs

client/executor/src/async_externalities.rs

primitives/io/src/lib.rs

primitives/io/src/tasks.rs

Co-authored-by: Bastian Köcher <[email protected]>

… into nv-parallel-runtime

bkchr

Besides moving the stuff that was added to sp-io to a new crate

bkchr · 2020-10-20T08:23:52Z

client/executor/wasmtime/src/instance_wrapper.rs

+
+impl EntryPoint {
+	/// Call this entry point.
+	pub fn call(&self, data_ptr: Pointer<u8>, data_len: WordSize) -> anyhow::Result<u64> {


You are converting it to a String on the calling side anyway. Instead of pulling in another dependency you could do the same here as well.

NikVolf · 2020-10-20T12:41:44Z

bot merge

ghost · 2020-10-20T12:41:50Z

Trying merge.

xlc · 2020-10-20T22:34:23Z

Good to see this is merged. Looking forward to use this feature in our runtime!

Some questions:
What is the overhead of spawn a new runtime thread? So that we can evaluate in what case that we can actually benefits from this.
What are the limitations of using runtime thread? From my understanding, it only have access to the data been passed into the worker right? Will it able to access some other data? The Trait constants?
Any example use cases?
Any future plans on improve this? e.g. message passing between threads?
What happen if we don't join a thread? Can I spawn a thread in on_initialize and join it in on_finalize? i.e. background worker thread while the main thread continuing process transactions.

kianenigma · 2020-10-21T08:37:12Z

Any example use cases?

I am eager to try it out in the NPOS stuff.

Hard use cases are concurrent phragmen and concurrent feasibility check. But an easier use case is this: I have a PR ready for a new test called PJR check. Each solution should ideally be checked to be PJR and feasible. These two checks can execute purely in parallel afaik.

These are just of the top of my head, I haven't looked into them in detail yet though.

NikVolf · 2020-10-21T09:00:17Z

What is the overhead of spawn a new runtime thread? So that we can evaluate in what case that we can actually benefits from this.

At the moment, should be quite big but with #7354 should be much as any additional runtime call.

What are the limitations of using runtime thread? From my understanding, it only have access to the data been passed into the worker right? Will it able to access some other data? The Trait constants?

Trait constant can change during runtime upgrade, they are not really constants.
So as any storage access this is not available. AFAIK @cheme is working on ideas about data parallelism on top of the current low-level stuff.

Any future plans on improve this? e.g. message passing between threads?

We need to explore deterministic story of message passing. But it is definitely on the list.

What happen if we don't join a thread? Can I spawn a thread in on_initialize and join it in on_finalize? i.e. background worker thread while the main thread continuing process transactions.

You can safely drop handles without joining threads.
And yes, you can do on_initialize spawn + on_finalize join.

Thanks for great questions, I'll add examples/tests so that answers are persisted.

pepyakin · 2020-10-21T09:02:27Z

First of all, I would say that the API is experimental and I don't think there are any guarantees about it.

What are the limitations of using runtime thread? From my understanding, it only have access to the data been passed into the worker right? Will it able to access some other data? The Trait constants?

Yeah, you got it right. The storage is not readable nor writable. Lifting this limitation would involve massive design work, as far as I understand. The workers share the same binary though, so they do have access to any code and data that reside within the binary. Source level stuff like Trait constants also should be accessible.

Any example use cases?

It might be useful for concurrent and/or batch signature verification. Stateless contracts might also work (that is not even on the horizon though).

Any future plans on improve this? e.g. message passing between threads?

I think it would have been cool if storage could be accessed, preferably mutable. That can be achieved near-term if a worker could only operate on isolated child-trie. Message passing: I feel it would be hard to use efficiently (I think the goal for the most efficient use is to make the workers as independent as possible while having the widest forks possible), but at the same time I feel there is potential to explore there.

What happen if we don't join a thread?

That's a good question. I think I'd prefer trapping in this case, since the workers are pure functions right now - not joining to one is basically a no-op. If we ever get to making them some effects we could lift this easily and allow other behavior.

Can I spawn a thread in on_initialize and join it in on_finalize?

Well, yes and no, but mostly no. And this is a very good point. During the block import it's possible. As long as you can carry the handles between on_initialize and on_finalize. But that won't work due to the fact that block building spawns a separate runtime instance for each call.

xlc · 2020-10-21T09:11:22Z

Thanks for the answers. They are really helpful.

Just one more comment, this will make weights & benchmarking very interesting...

NikVolf · 2020-10-21T09:24:55Z

Just one more comment, this will make weights & benchmarking very interesting...

As long as you don't have unbound parallelism and reference machine used for benchmarking has number of cores specified, benchmarks should be valid.

NikVolf · 2020-10-21T09:38:51Z

But that won't work due to the fact that block building spawns a separate runtime instance for each call.

It can be fixed in principle (by keeping tasks alive during block production)

But anyway, the problem with persisting handles will not probably be solved until we ditch native runtime

cheme · 2020-10-21T10:23:04Z

What happen if we don't join a thread?

About it, I was thinking that forcing join would be good (but it requires to manage a pool of thread for the runtime call).

The case I don't like much is a panicking worker that is not joined, then its extrinsic evaluation can non deterministically panic or not. One can even include by mistake a panicking extrinsic in a block.

pepyakin · 2020-10-21T11:32:35Z

How could it be non-deterministic? When runtime finishes its execution there are two outcomes: in one the runtime joined the worker and the other where the runtime didn't. Which outcome takes place depends solely on the actions of the runtime which is assumed to be deterministic.

Sure, strictness is not necessary, but there are 0 reasons when you want to do that and then if that happens then it is certainly a programming error, which should be reported ASAP IMO. Then, this would rule out the users from relying on the behavior of automatic joining leaving us a possibility to endow our own semantics to this event.

cheme · 2020-10-21T12:20:06Z

I was wrongly thinking about the panicking behavior, but it all run behind a panic handler so that is fine.

Still think we should either join or early terminate worker :)

NikVolf added A0-please_review Pull request needs code review. B7-runtimenoteworthy C3-medium PR touches the given topic and has a medium impact on builders. labels Sep 11, 2020

NikVolf commented Sep 11, 2020

View reviewed changes

pepyakin reviewed Sep 14, 2020

View reviewed changes

cheme reviewed Sep 14, 2020

View reviewed changes

NikVolf force-pushed the nv-parallel-runtime branch from 1cbf57e to 19ead82 Compare September 15, 2020 11:06

NikVolf added 12 commits September 15, 2020 11:47

std variant

e9dcebe

principal work

d73ae86

format and naming

d1a27f2

format and naming continued

fbca518

working nested fork

7472521

add comment

8548494

naming and tabs

1249b5a

line width

abe81dd

fix wording

612a730

address review

b07e0c6

refactor dynamic dispatch

e10a58c

update wasmtime

669465a

NikVolf force-pushed the nv-parallel-runtime branch from cf40171 to 669465a Compare September 15, 2020 11:48

NikVolf added 3 commits September 15, 2020 11:54

some care

2752db3

move ext

b001b81

more refactor

f7a02fc

NikVolf force-pushed the nv-parallel-runtime branch from c2cbcf2 to f7a02fc Compare September 15, 2020 12:53

NikVolf added this to the 2.x series milestone Sep 15, 2020

NikVolf added 4 commits September 15, 2020 12:57

doc effort

fdffc76

simplify

330afa3

doc effort

ff8a73c

tests and docs

34ec74d

NikVolf force-pushed the nv-parallel-runtime branch from 2a682e8 to 34ec74d Compare September 16, 2020 11:32

NikVolf and others added 4 commits October 19, 2020 07:13

Update primitives/io/src/tasks.rs

14bfbc0

Co-authored-by: Bastian Köcher <[email protected]>

Update client/executor/wasmtime/src/instance_wrapper.rs

2853632

Co-authored-by: Bastian Köcher <[email protected]>

address some issues

7a62626

Merge branch 'nv-parallel-runtime' of github.com:paritytech/substrate…

5b10f4b

… into nv-parallel-runtime

NikVolf force-pushed the nv-parallel-runtime branch from b9b9772 to 87430f8 Compare October 19, 2020 14:55

address more issues

0583236

NikVolf force-pushed the nv-parallel-runtime branch from 87430f8 to 0583236 Compare October 19, 2020 14:56

wasm_only interface

fd310bd

github-actions bot added the A7-needspolkadotpr label Oct 19, 2020

Merge remote-tracking branch 'origin/master' into nv-parallel-runtime

79da04b

bkchr approved these changes Oct 20, 2020

View reviewed changes

NikVolf added 3 commits October 20, 2020 04:16

define sp_tasks

6e06f38

avoid anyhow

4080e1a

fix example

3d1dc8f

github-actions bot removed the A7-needspolkadotpr label Oct 20, 2020

ghost merged commit 1845278 into master Oct 20, 2020

ghost deleted the nv-parallel-runtime branch October 20, 2020 12:41

xlc mentioned this pull request Oct 20, 2020

Runtime worker threads feature requests #7366

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Runtime worker threads #7089

Runtime worker threads #7089

NikVolf commented Sep 11, 2020 •

edited

Loading

NikVolf Sep 11, 2020

pepyakin left a comment

cheme left a comment

bkchr left a comment

bkchr Oct 20, 2020

NikVolf commented Oct 20, 2020

ghost commented Oct 20, 2020

xlc commented Oct 20, 2020

kianenigma commented Oct 21, 2020

NikVolf commented Oct 21, 2020 •

edited

Loading

pepyakin commented Oct 21, 2020

xlc commented Oct 21, 2020

NikVolf commented Oct 21, 2020

NikVolf commented Oct 21, 2020

cheme commented Oct 21, 2020

pepyakin commented Oct 21, 2020

cheme commented Oct 21, 2020


		type StorageValue = Vec<u8>;

		impl Externalities for AsyncExternalities {

Runtime worker threads #7089

Runtime worker threads #7089

Conversation

NikVolf commented Sep 11, 2020 • edited Loading

NikVolf Sep 11, 2020

Choose a reason for hiding this comment

pepyakin left a comment

Choose a reason for hiding this comment

cheme left a comment

Choose a reason for hiding this comment

bkchr left a comment

Choose a reason for hiding this comment

bkchr Oct 20, 2020

Choose a reason for hiding this comment

NikVolf commented Oct 20, 2020

ghost commented Oct 20, 2020

xlc commented Oct 20, 2020

kianenigma commented Oct 21, 2020

NikVolf commented Oct 21, 2020 • edited Loading

pepyakin commented Oct 21, 2020

xlc commented Oct 21, 2020

NikVolf commented Oct 21, 2020

NikVolf commented Oct 21, 2020

cheme commented Oct 21, 2020

pepyakin commented Oct 21, 2020

cheme commented Oct 21, 2020

NikVolf commented Sep 11, 2020 •

edited

Loading

NikVolf commented Oct 21, 2020 •

edited

Loading