Skip to content
This repository has been archived by the owner on Sep 13, 2022. It is now read-only.

perf(executor): use inner call instead of service dispatcher #365

Merged
merged 2 commits into from
Jul 23, 2020

Conversation

KaoImin
Copy link

@KaoImin KaoImin commented Jul 20, 2020

What type of PR is this?
perf

What this PR does / why we need it:

  1. Add an SDKFactory trait to cache SDKs of each service at the beginning of executing a block.
  2. Remove service dispatcher.
  3. The check items of the authorization service are hard-coded into the code and cannot be changed through transactions.
Bench in Intel(R) Core(TM) i7-4770HQ CPU @ 2.20GHz (8 x 2200)
100 txs bench_execute ... bench:  11,299,912 ns/iter (+/- 3,402,276)
1000 txs bench::bench_execute ... bench: 101,187,934 ns/iter (+/- 26,000,469)

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

@KaoImin KaoImin requested review from zeroqn and removed request for rev-chaos and zhouyun-zoe July 20, 2020 03:12
@KaoImin KaoImin marked this pull request as ready for review July 20, 2020 06:14
@KaoImin KaoImin requested a review from yejiayu July 20, 2020 06:14
@yejiayu
Copy link
Contributor

yejiayu commented Jul 20, 2020

Add describe

@yejiayu
Copy link
Contributor

yejiayu commented Jul 20, 2020

/test all

@yejiayu
Copy link
Contributor

yejiayu commented Jul 20, 2020

I think you should define a trait for functions if you don't want to be exposed, and give you two examples

  1. metadata
pub trait MetadataService {
  fn get(&self) -> Metadata;
  fn update(&mut self, metadata: Metadata);
}

#[service]
impl NormalMetadataService {
  #[read]
  // the function can be called with transaction
  fn get_metadata() -> Metadata { self.get() }
}

impl NormalMetadataService for MetadataService {
  // the function can be called only each other services
  fn get() -> Metadata { ... }
  
  // Now, we has a safety calling way on updating metadata that don't need rewrite metadata.
  fn update(&mut self) -> { ... }
}
  1. Authorization

    pub trait AuthorizationService {
      fn allow(&self) -> bool;
    }
    
    // We don't need to expose any methods.
    impl NoopAuthorizationService for AuthorizationService {
      fn allow(&self) -> { ... };
    }

ping @KaoImin

@muta-robot muta-robot added size/XXL #d12d0c and removed size/XL labels Jul 20, 2020
Cargo.toml Outdated Show resolved Hide resolved
@KaoImin KaoImin changed the title refactor(executor): use inner call instead of service dispatcher perf(executor): use inner call instead of service dispatcher Jul 21, 2020
benchmark/benchmark_genesis.toml Outdated Show resolved Hide resolved
built-in-services/asset/src/lib.rs Outdated Show resolved Hide resolved
@KaoImin KaoImin force-pushed the inner_call branch 4 times, most recently from abbdfc2 to de91d59 Compare July 22, 2020 04:56
@yejiayu
Copy link
Contributor

yejiayu commented Jul 23, 2020

/lgtm

@muta-robot muta-robot added the lgtm #8ef42e label Jul 23, 2020
@muta-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: yejiayu

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@muta-robot muta-robot merged commit 7b1d2a3 into nervosnetwork:master Jul 23, 2020
@KaoImin KaoImin deleted the inner_call branch August 1, 2020 06:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants