Skip to content

Conversation

@sipa
Copy link
Member

@sipa sipa commented Mar 26, 2015

I don't expect a measurable performance improvement from this, but result seems more readable too.

Copy link
Member

Choose a reason for hiding this comment

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

Looks good to me. This assert could even be left out as GetAncestor already returns NULL for heights <0.

Copy link
Member Author

Choose a reason for hiding this comment

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

To mimick the exact old code, it would need to be nHeightFirst = std::max(0, pindex->nHeight - (... - 1));. I added the assert to be sure for now that dropping the std::max is safe (which it should be afaict).

Copy link
Member

Choose a reason for hiding this comment

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

OK, fair enough

@laanwj
Copy link
Member

laanwj commented Apr 1, 2015

Going to test this on my test node.

@laanwj
Copy link
Member

laanwj commented Apr 6, 2015

Tested ACK

@sipa sipa merged commit 1cc0e96 into bitcoin:master Apr 7, 2015
sipa added a commit that referenced this pull request Apr 7, 2015
1cc0e96 Trivial optimization: use GetAncestor to compute new target (Pieter Wuille)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants