Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

State Tracing #134

Merged
merged 115 commits into from
Jan 29, 2021
Merged

State Tracing #134

merged 115 commits into from
Jan 29, 2021

Conversation

insipx
Copy link
Collaborator

@insipx insipx commented Nov 18, 2020

Continuation of work from #92

Not ready for merge yet because:

  • migrations for tracing tables need to be created
  • Traces need insertion into Postgres Database
  • Documentation
  • Testing on Kusama and Polkadot
  • Filtering by Target/Level

In this PR

  • ActorContext -> SystemConfig
  • Instantiating our own native_executor_instance in order to have access and change around the HostFunctions available in the executing WASM environment
  • add a config option for WASM targets, add a CLI argument for the folder to override WASM

@insipx insipx mentioned this pull request Nov 18, 2020
substrate-archive/src/actors/workers/state_tracing.rs Outdated Show resolved Hide resolved
substrate-archive/src/actors/workers/state_tracing.rs Outdated Show resolved Hide resolved
substrate-archive/src/actors/workers.rs Outdated Show resolved Hide resolved
substrate-archive/src/archive.rs Outdated Show resolved Hide resolved
substrate-archive-backend/src/block_exec.rs Outdated Show resolved Hide resolved
substrate-archive-common/src/util.rs Outdated Show resolved Hide resolved
@insipx
Copy link
Collaborator Author

insipx commented Nov 19, 2020

Didn't expect a review so fast! There are some problems with the current implementation I wrote up and was going to ask for feedback today

State tracing seems to work -- sort of. Replacing WASM code at runtime and tracing itself is working, but Substrate Archive is getting traces for only some parts of the substrate landscape: sp_io::{hashing,allocator,crypto,storage} but no top-level traces from e.g frame_executive/frame_support. I know that for some of these pallets, tracing hasn't been implemented in the runtime yet. However, I'd expect to get traces from at least executive for this line (for example): https://github.com/paritytech/substrate/blob/69198d1e5735798c8baa1921c6e2091c93260752/frame/executive/src/lib.rs#L295

Haven't made the migrations yet because i'm not sure if there might be something wrong with how i've setup the tracing. So far, it just logs the sp_io:: targets outlined earlier as an INFO logline. For example this is a photo of two blocks executing and only getting trie/crypto tracing targets (with some filtered out because there were alot of loglines):

cc @mattrutherford

state tracing

@insipx insipx requested review from maciejhirsz, dvdplm and mattrutherford and removed request for dvdplm January 28, 2021 16:32
let executor =
LocalCallExecutor::new(backend.clone(), executor, Box::new(TaskExecutor::new()), Default::default())?;
let executor = NativeExecutor::<Dispatch>::new(
WasmExecutionMethod::Interpreted,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not WasmExecutionMethod::Compiled? It's enabled in this PR and I suppose it's faster?!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I measured this and it's actually not faster. I was surprised by this and it might be worth taking a deeper look. When syncing the chain in normal polkadot using --wasm-execution=Compiled is much much faster.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I see.

Andrew Plaza added 4 commits January 29, 2021 14:40
- Remove clones in Trace database insert
- remove frame-benchmarking dependency from polkadot-archive
- add new WASM binaries for 8.27 (older ones didn't work)
- add comment explaining why we unwrap Arc and Mutex
@insipx insipx merged commit 00b5f3c into master Jan 29, 2021
@niklasad1 niklasad1 deleted the insipx/state-tracing branch January 29, 2021 15:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants