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

consensus/ethash: implement faster difficulty calculators #21976

Merged
merged 3 commits into from
Dec 11, 2020

Conversation

holiman
Copy link
Contributor

@holiman holiman commented Dec 8, 2020

This PR is very safe, and doesn't actually do anything.
While I was investigating header import (https://github.com/holiman/headerimport/blob/main/README.md ) , I noticed that there is a lot of time spent (allocations spent) on calculating the difficulty.
This PR adds re-written difficulty calculators, which are based on uint256. It also adds a fuzzer + oss-fuzz integration for the new fuzzer. It does differential fuzzing between the new and old calculators.

Note: this PR does not actually enable the new calculators.
Anyway, if travis looks green, we could merge this and have it fuzzed, and later decide whether about enabling it.

Oh, and there are some benchmarks too, u256-xx are the new ones, big-xx are the old:

BenchmarkDifficultyCalculator
BenchmarkDifficultyCalculator/big-frontier
BenchmarkDifficultyCalculator/big-frontier-6         	 1484992	       722 ns/op	     280 B/op	      12 allocs/op
BenchmarkDifficultyCalculator/u256-frontier
BenchmarkDifficultyCalculator/u256-frontier-6        	 8540350	       176 ns/op	      64 B/op	       2 allocs/op
BenchmarkDifficultyCalculator/big-homestead
BenchmarkDifficultyCalculator/big-homestead-6        	 1506804	       831 ns/op	     288 B/op	      13 allocs/op
BenchmarkDifficultyCalculator/u256-homestead
BenchmarkDifficultyCalculator/u256-homestead-6       	 9134642	       184 ns/op	      64 B/op	       2 allocs/op
BenchmarkDifficultyCalculator/big-generic
BenchmarkDifficultyCalculator/big-generic-6          	 2789331	       387 ns/op	      80 B/op	       7 allocs/op
BenchmarkDifficultyCalculator/u256-generic
BenchmarkDifficultyCalculator/u256-generic-6         	 7718224	       154 ns/op	      64 B/op	       2 allocs/op
PASS

@holiman
Copy link
Contributor Author

holiman commented Dec 8, 2020

Tested this on bigendian too

[user@work ethash]$ GOARCH=ppc64 go test -c
[user@work ethash]$ qemu-ppc64-static ethash.test -test.v

Works fine

@holiman holiman changed the title consensus/ethash: speed up difficulty calculators consensus/ethash: implement faster difficulty calculators Dec 8, 2020
Copy link
Member

@MariusVanDerWijden MariusVanDerWijden left a comment

Choose a reason for hiding this comment

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

LGTM (I mainly looked at the fuzzers though)

@holiman holiman merged commit efe6dd2 into ethereum:master Dec 11, 2020
@holiman holiman added this to the 1.10.0 milestone Dec 11, 2020
x = x - 1
}
z := new(uint256.Int).SetUint64(x)
adjust.Mul(adjust, z) // adjust: (pdiff / 2048) * max((time - ptime) / 10 - 1, 99)
Copy link
Member

Choose a reason for hiding this comment

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

Just thinking out loud, if we moved the formula from max(1 - (block_timestamp - parent_timestamp) // 10, -99) to max((time - ptime) / 10 - 1, 99) this should become min right? e.g. min((time - ptime) / 10 - 1, 99)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, so what we want is the absolute value to not exceed 99. So hence the if x>=100 above.

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.

2 participants