Skip to content

Conversation

@ucwong
Copy link
Contributor

@ucwong ucwong commented Dec 18, 2020

No description provided.

@rjl493456442
Copy link
Member

Is there any benefit? E.g. is there any performance speedup? Would be nice to have some benchmarks.

We are inclined to not accept the changes if it's basically noop.

Comment on lines 29 to 30
"sync"
"time"
Copy link
Contributor

Choose a reason for hiding this comment

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

goimports needed

@holiman
Copy link
Contributor

holiman commented Dec 18, 2020

Is there any benefit? E.g. is there any performance speedup? Would be nice to have some benchmarks.

Yes, Read avoid a copy which Sum does

@holiman
Copy link
Contributor

holiman commented Dec 18, 2020

Similar one in a different area: https://github.com/ethereum/go-ethereum/pull/22023/files

Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM

@holiman holiman added this to the 1.10.0 milestone Dec 21, 2020
@holiman holiman merged commit c5a3ffa into ethereum:master Dec 21, 2020
@holiman
Copy link
Contributor

holiman commented Dec 21, 2020

Thanks!

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.

3 participants