Skip to content

perf: cache system cell for resolve deps#2006

Merged
bors[bot] merged 2 commits into
developfrom
op-resolve-dep
Apr 21, 2020
Merged

perf: cache system cell for resolve deps#2006
bors[bot] merged 2 commits into
developfrom
op-resolve-dep

Conversation

@zhangsoledad

@zhangsoledad zhangsoledad commented Apr 13, 2020

Copy link
Copy Markdown
Member

The goal of this PR is to optimize transaction resolve.
Currently, transaction resolve is inefficient, cause system-cell is uncached, every transaction depends on system cell will read system-cell from disk repeatedly. this PR proposal a system-cell cache implementation.

@zhangsoledad zhangsoledad requested review from a team and xxuejie April 13, 2020 12:06
@zhangsoledad

Copy link
Copy Markdown
Member Author

benchmark

@chaoticlonghair

Copy link
Copy Markdown
Contributor

Benchmark Result

  • TPS: 377.90
  • Samples Count: 50
  • CKB Version: e72ebc7ae24800b71cb9a295ac34d189fd3620ab
  • Instance Type: c5.xlarge
  • Instances Count: 3
  • Bench Type: 2in2out
  • CKB Logger Filter: info,ckb=debug

@doitian

doitian commented Apr 14, 2020

Copy link
Copy Markdown
Member

Please add PR description:

  • Why this PR is needed
  • What this PR has done
  • The comparison

Comment thread util/types/src/core/cell.rs Outdated

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think a more elegant solution is creating a wrapper around the resolver, like:

struct<F> CellProviderWithSystemCellCache {
  system_cell: SystemCell,
  inner: F,
}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This PR is a draft for bench, I will improve it later.
But I think CellProvider is also a bad idea, this cache is for dep, not input.

@zhangsoledad zhangsoledad changed the title perf: cache system cell for resolve deps [HOLD] perf: cache system cell for resolve deps Apr 14, 2020
nervos-bot[bot]
nervos-bot Bot previously requested changes Apr 14, 2020

@nervos-bot nervos-bot Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hold as requested by @zhangsoledad.

@zhangsoledad

Copy link
Copy Markdown
Member Author

benchmark

@chaoticlonghair

Copy link
Copy Markdown
Contributor

Benchmark Result

  • TPS: 381.56
  • Samples Count: 51
  • CKB Version: 0c86b9fb64b194d9bfad03e576b8468a54c35a49
  • Instance Type: c5.xlarge
  • Instances Count: 3
  • Bench Type: 2in2out
  • CKB Logger Filter: info,ckb=debug

@zhangsoledad zhangsoledad changed the title [HOLD] perf: cache system cell for resolve deps perf: cache system cell for resolve deps Apr 20, 2020
@nervos-bot nervos-bot Bot dismissed their stale review April 20, 2020 02:48

Unhold as requested by @zhangsoledad.

@quake

quake commented Apr 21, 2020

Copy link
Copy Markdown
Member

bors r+

@bors

bors Bot commented Apr 21, 2020

Copy link
Copy Markdown
Contributor

Build succeeded:

  • continuous-integration/travis-ci/push

@bors bors Bot merged commit 07edc0b into develop Apr 21, 2020
@bors bors Bot deleted the op-resolve-dep branch April 21, 2020 06:16
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