8000 Refactor Trie hasher using onRoot flag by blukat29 · Pull Request #1840 · klaytn/klaytn · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was archived by the owner on Aug 19, 2024. It is now read-only.

Refactor Trie hasher using onRoot flag #1840

Merged
merged 3 commits into from
May 25, 2023
Merged

Refactor Trie hasher using onRoot flag #1840

merged 3 commits into from
May 25, 2023

Conversation

blukat29
Copy link
Contributor
@blukat29 blukat29 commented May 9, 2023 •
edited
Loading

Proposed changes

De-duplicate code in the trie hasher (storage/statedb/hasher.go).

  • Current implementation has two pairs of similar functions:
    • single-threaded version hash() + hashChildren()
    • multi-threaded version hashRoot() + hashChildrenFromRoot()
  • This PR removes the latter. Instead, the onRoot argument switches multi-threading.
    • hashNode(..., onRoot) + hashChildren(..., onRoot) runs multi-threaded if true.
      • hash(...) = hashNode(..., >
      • hashRoot(...) = hashNode(..., >

Types of changes

  • Bugfix
  • New feature or enhancement
  • Others

Checklist

  • I have read the CONTRIBUTING GUIDELINES doc
  • I have signed the CLA
  • Lint and unit tests pass locally with my changes ($ make test)
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in downstream modules

Related issues

Further comments

Benchmark results show that this PR does not affect hashing performance (on Apple M1 Max 10-core)

  • Before PR 
     $ go test -run=^$ -bench=BenchmarkHash
 BenchmarkHash-10                 2582688               402.2 ns/op           684 B/op          7 allocs/op
 BenchmarkHashFixedSize/10-10       26340             43323 ns/op           14891 B/op        179 allocs/op
 BenchmarkHashFixedSize/100-10              12986             89688 ns/op           66881 B/op        809 allocs/op
 BenchmarkHashFixedSize/1K-10                2024            618712 ns/op          659172 B/op       7811 allocs/op
 BenchmarkHashFixedSize/10K-10                282           4171724 ns/op         6907710 B/op      79703 allocs/op
 BenchmarkHashFixedSize/100K-10                33          35102691 ns/op        68240330 B/op     794537 allocs/op
  • After PR 
     BenchmarkHash-10                 3081148               439.9 ns/op           677 B/op          7 allocs/op
 BenchmarkHashFixedSize/10-10       27524             42308 ns/op           14262 B/op        178 allocs/op
 BenchmarkHashFixedSize/100-10              13624             88954 ns/op           66293 B/op        808 allocs/op
 BenchmarkHashFixedSize/1K-10                2050            604276 ns/op          658532 B/op       7810 allocs/op
 BenchmarkHashFixedSize/10K-10                280           4168147 ns/op         6906603 B/op      79701 allocs/op
 BenchmarkHashFixedSize/100K-10                30          38524168 ns/op        68240050 B/op     794539 allocs/op
  • (for comparison) After PR, if false 
     BenchmarkHash-10                  564274              2035 ns/op             628 B/op          7 allocs/op
 BenchmarkHashFixedSize/10-10       41187             28873 ns/op           13213 B/op        155 allocs/op
 BenchmarkHashFixedSize/100-10               8718            134050 ns/op           64321 B/op        772 allocs/op
 BenchmarkHashFixedSize/1K-10                 842           1415162 ns/op          649540 B/op       7742 allocs/op
 BenchmarkHashFixedSize/10K-10                 70          17122263 ns/op         6865380 B/op      79500 allocs/op
 BenchmarkHashFixedSize/100K-10                 7         164345345 ns/op        68196154 B/op     794339 allocs/op

@blukat29 blukat29 self-assigned this May 9, 2023
@blukat29 blukat29 marked this pull request as draft May 10, 2023 03:35
@blukat29 blukat29 changed the title Refactor Trie hasher using parallel flag Refactor Trie hasher using onRoot flag May 11, 2023
@blukat29 blukat29 force-pushed the refactor-trie-hasher branch from 8f5678b to 3c17660 Compare May 11, 2023 09:16
@blukat29 blukat29 marked this pull request as ready for review May 12, 2023 00:39
ethan-kr
ethan-kr reviewed May 16, 2023
View reviewed changes
ethan-kr
ethan-kr previously approved these changes May 16, 2023
View reviewed changes
Copy link
Contributor
@ethan-kr ethan-kr left a comment

Choose a reason for hiding this comment

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

I made a comment, but there is no disagreement, so I approve it.

jeongkyun-oh
jeongkyun-oh approved these changes May 18, 2023
View reviewed changes
Copy link
Contributor
@jeongkyun-oh jeongkyun-oh left a comment

Choose a reason for hiding this comment

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

LGTM

@blukat29
Copy link
Contributor Author

@ethan-kr PTAL :)

@blukat29 blukat29 merged commit 3825a41 into klaytn:dev May 25, 2023
@blukat29 blukat29 deleted the refactor-trie-hasher branch July 17, 2023 02:06
@JayChoi1736 JayChoi1736 mentioned this pull request Jul 24, 2023
Closed
20 tasks
@blukat29 blukat29 mentioned this pull request Sep 20, 2023
Closed
9 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jeongkyun-oh jeongkyun-oh jeongkyun-oh approved these changes

@aidan-kwon aidan-kwon aidan-kwon approved these changes

@ethan-kr ethan-kr ethan-kr approved these changes

Assignees

@blukat29 blukat29

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@blukat29 @jeongkyun-oh @aidan-kwon @ethan-kr
0