Skip to content

Conversation

@holiman
Copy link
Contributor

@holiman holiman commented Sep 5, 2019

I noticed this wile investigating the state a bit. What we currently do before RLP-encoding a slot value is to call TrimLeft:

func TrimLeft(s []byte, cutset string) []byte {
	return TrimLeftFunc(s, makeCutsetFunc(cutset))
}

... which dynamically creates a cutSetFunction .
Then it calls TrimLeftFunc, which:

// TrimLeftFunc treats s as UTF-8-encoded bytes and returns a subslice of s by slicing off
// all leading UTF-8-encoded code points c that satisfy f(c).
func TrimLeftFunc(s []byte, f func(r rune) bool) []byte {

It interprets the data as UTF-8 encoded data, and parses runes.

In our case, it's the wrong abstraction, what we want to do here is simply trim leading zeroes, which has nothing whatsoever to do with UTF-8 and runes.

This PR replaces that with a simple for-loop, more or less.

benchmark results:

[user@work state]$ go test . -bench BenchmarkCut*
OK: 5 passed
goos: linux
goarch: amd64
pkg: github.com/ethereum/go-ethereum/core/state
BenchmarkCutOriginal-6     	10000000	       146 ns/op
BenchmarkCutsetterFn-6     	20000000	        86.3 ns/op
BenchmarkCutCustomTrim-6   	50000000	        28.9 ns/op
PASS
ok  	github.com/ethereum/go-ethereum/core/state	11.941s

It's probably a drop in the ocean though...

@holiman holiman force-pushed the minor_encodingfix branch 4 times, most recently from 3631654 to f8e2c3a Compare September 6, 2019 07:59
Copy link
Member

@karalabe karalabe Sep 10, 2019

Choose a reason for hiding this comment

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

return s[idx:] always?

Copy link
Member

@karalabe karalabe left a comment

Choose a reason for hiding this comment

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

SGTM

@karalabe karalabe added this to the 1.9.4 milestone Sep 10, 2019
@holiman holiman force-pushed the minor_encodingfix branch 2 times, most recently from 14cd31d to 1c82f03 Compare September 10, 2019 09:04
@karalabe
Copy link
Member

Linter fails, pls gofmt

@holiman
Copy link
Contributor Author

holiman commented Sep 10, 2019

done, let's see what the linter says next...

@karalabe karalabe merged commit 305ed95 into ethereum:master Sep 10, 2019
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Dec 28, 2024
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