Skip to content

[tstate] inline byte to string conversions #174

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.

Sign up for GitHub

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
+166 −60
Open

[tstate] inline byte to string conversions #174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
81d0cb9
add bench
hexfusion May 14, 2023
3002fd4
Inline all byte to string conversions
hexfusion May 15, 2023
06ebd7f
nits
hexfusion May 15, 2023
e5fe915
reduce allocations
hexfusion May 16, 2023
f687764
cleanup benchmarks
hexfusion May 17, 2023
487c2b0
Fix typos
hexfusion May 17, 2023
a7628fc
Add checkScope bench
hexfusion May 17, 2023
0c7bef5
[processor] inline all byte to string conversions
hexfusion May 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 5 additions & 6 deletions chain/processor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,22 +53,21 @@ func (p *Processor) Prefetch(ctx context.Context, db Database) {
for _, tx := range p.blk.GetTxs() {
storage := map[string][]byte{}
for _, k := range tx.StateKeys(sm) {
sk := string(k)
if v, ok := alreadyFetched[sk]; ok {
if v, ok := alreadyFetched[string(k)]; ok {
if v.exists {
storage[sk] = v.v
storage[string(k)] = v.v
}
continue
}
v, err := db.GetValue(ctx, k)
if errors.Is(err, database.ErrNotFound) {
alreadyFetched[sk] = &fetchData{nil, false}
alreadyFetched[string(k)] = &fetchData{nil, false}
continue
} else if err != nil {
panic(err)
}
alreadyFetched[sk] = &fetchData{v, true}
storage[sk] = v
alreadyFetched[string(k)] = &fetchData{v, true}
storage[string(k)] = v
}
p.readyTxs <- &txData{tx, storage}
}
Expand Down
43 changes: 19 additions & 24 deletions tstate/tstate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
)

type op struct {
k string
k []byte

pastExists bool
pastV []byte
Expand Down Expand Up @@ -67,22 +67,21 @@ func (ts *TState) GetValue(ctx context.Context, key []byte) ([]byte, error) {
if !ts.checkScope(ctx, key) {
return nil, ErrKeyNotSpecified
}
k := string(key)
v, _, exists := ts.getValue(ctx, k)
v, _, exists := ts.getValue(ctx, key)
if !exists {
return nil, database.ErrNotFound
}
return v, nil
}

func (ts *TState) getValue(_ context.Context, key string) ([]byte, bool, bool) {
if v, ok := ts.changedKeys[key]; ok {
func (ts *TState) getValue(_ context.Context, key []byte) ([]byte, bool, bool) {
if v, ok := ts.changedKeys[string(key)]; ok {
if v.removed {
return nil, true, false
}
return v.v, true, true
}
v, ok := ts.scopeStorage[key]
v, ok := ts.scopeStorage[string(key)]
if !ok {
return nil, false, false
}
Expand All @@ -93,25 +92,24 @@ func (ts *TState) getValue(_ context.Context, key string) ([]byte, bool, bool) {
// FetchAndSetScope then sets the scope of ts to [keys]. If a key exists in
// ts.fetchCache set the key's value to the value from cache.
func (ts *TState) FetchAndSetScope(ctx context.Context, keys [][]byte, db Database) error {
ts.scopeStorage = map[string][]byte{}
ts.scopeStorage = make(map[string][]byte, len(keys))
for _, key := range keys {
k := string(key)
if val, ok := ts.fetchCache[k]; ok {
if val, ok := ts.fetchCache[string(key)]; ok {
if val.Exists {
ts.scopeStorage[k] = val.Value
ts.scopeStorage[string(key)] = val.Value
}
continue
}
v, err := db.GetValue(ctx, key)
if errors.Is(err, database.ErrNotFound) {
ts.fetchCache[k] = &cacheItem{Exists: false}
ts.fetchCache[string(key)] = &cacheItem{Exists: false}
continue
}
if err != nil {
return err
}
ts.fetchCache[k] = &cacheItem{Value: v, Exists: true}
ts.scopeStorage[k] = v
ts.fetchCache[string(key)] = &cacheItem{Value: v, Exists: true}
ts.scopeStorage[string(key)] = v
}
ts.scope = keys
return nil
Expand All @@ -126,7 +124,6 @@ func (ts *TState) SetScope(_ context.Context, keys [][]byte, storage map[string]
// checkScope returns whether [k] is in ts.readScope.
func (ts *TState) checkScope(_ context.Context, k []byte) bool {
for _, s := range ts.scope {
// TODO: benchmark and see if creating map is worth overhead
Copy link
Contributor Author

@hexfusion hexfusion May 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

this very much depends on the size of the set. since lookup is O(n) its reasonable to have a map if we might have 8+ keys. I added the change for ref but we can revert the commit if we feel the set size would remain similar to tokenvm (4).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although the lookup is much faster for map it does increase the alloc in FetchAndSetScope by a fair amount. As this set grows into possibly hundreds the time gained on lookup will be probably overshadowed by memory bloat/cpu time.

before

goos: linux
goarch: amd64
pkg: github.com/ava-labs/hypersdk/tstate
cpu: Intel(R) Core(TM) i7-10750H CPU @ 2.60GHz
BenchmarkFetchAndSetScope/fetch_and_set_scope_4_keys_with_length_65-12         	  894080	      1408 ns/op	     800 B/op	       7 allocs/op
BenchmarkFetchAndSetScope/fetch_and_set_scope_8_keys_with_length_65-12         	  687463	      2001 ns/op	    1473 B/op	      11 allocs/op
BenchmarkFetchAndSetScope/fetch_and_set_scope_16_keys_with_length_65-12        	  372249	      3031 ns/op	    2833 B/op	      19 allocs/op
BenchmarkFetchAndSetScope/fetch_and_set_scope_32_keys_with_length_65-12        	  243076	      5262 ns/op	    5424 B/op	      35 allocs/op
BenchmarkFetchAndSetScope/fetch_and_set_scope_64_keys_with_length_65-12        	  110935	     14345 ns/op	   11416 B/op	      68 allocs/op
BenchmarkFetchAndSetScope/fetch_and_set_scope_128_keys_with_length_65-12       	   42150	     26776 ns/op	   22680 B/op	     132 allocs/op

after

goos: linux
goarch: amd64
pkg: github.com/ava-labs/hypersdk/tstate
cpu: Intel(R) Core(TM) i7-10750H CPU @ 2.60GHz
BenchmarkFetchAndSetScope/fetch_and_set_scope_4_keys_with_length_65-12         	  716866	      1802 ns/op	    1392 B/op	      14 allocs/op
BenchmarkFetchAndSetScope/fetch_and_set_scope_8_keys_with_length_65-12         	  404772	      2923 ns/op	    2529 B/op	      22 allocs/op
BenchmarkFetchAndSetScope/fetch_and_set_scope_16_keys_with_length_65-12        	  282474	      4318 ns/op	    4824 B/op	      38 allocs/op
BenchmarkFetchAndSetScope/fetch_and_set_scope_32_keys_with_length_65-12        	  157590	      7921 ns/op	    9284 B/op	      70 allocs/op
BenchmarkFetchAndSetScope/fetch_and_set_scope_64_keys_with_length_65-12        	   83341	     15155 ns/op	   19376 B/op	     136 allocs/op
BenchmarkFetchAndSetScope/fetch_and_set_scope_128_keys_with_length_65-12       	   42186	     40291 ns/op	   38448 B/op	     264 allocs/op

Copy link
Contributor Author

Choose a reason for hiding this comment

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

reverting for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can see the cost of lookup as the set increases.

BenchmarkCheckScope/set_scope_4_keys_with_length_65-12         	55952859	        21.66 ns/op	       0 B/op	       0 allocs/op
BenchmarkCheckScope/set_scope_8_keys_with_length_65-12         	30695019	        39.45 ns/op	       0 B/op	       0 allocs/op
BenchmarkCheckScope/set_scope_16_keys_with_length_65-12        	14245750	        84.93 ns/op	       0 B/op	       0 allocs/op
BenchmarkCheckScope/set_scope_32_keys_with_length_65-12        	 7016282	       160.0 ns/op	       0 B/op	       0 allocs/op
BenchmarkCheckScope/set_scope_64_keys_with_length_65-12        	 3868280	       286.7 ns/op	       0 B/op	       0 allocs/op
BenchmarkCheckScope/set_scope_128_keys_with_length_65-12       	 2166471	       547.1 ns/op	       0 B/op	       0 allocs/op

if bytes.Equal(k, s) {
return true
}
Expand All @@ -139,15 +136,14 @@ func (ts *TState) Insert(ctx context.Context, key []byte, value []byte) error {
if !ts.checkScope(ctx, key) {
return ErrKeyNotSpecified
}
k := string(key)
past, changed, exists := ts.getValue(ctx, k)
past, changed, exists := ts.getValue(ctx, key)
ts.ops = append(ts.ops, &op{
k: k,
k: key,
pastExists: exists,
pastV: past,
pastChanged: changed,
})
ts.changedKeys[k] = &tempStorage{value, false}
ts.changedKeys[string(key)] = &tempStorage{value, false}
return nil
}

Expand All @@ -156,18 +152,17 @@ func (ts *TState) Remove(ctx context.Context, key []byte) error {
if !ts.checkScope(ctx, key) {
return ErrKeyNotSpecified
}
k := string(key)
past, changed, exists := ts.getValue(ctx, k)
past, changed, exists := ts.getValue(ctx, key)
if !exists {
return nil
}
ts.ops = append(ts.ops, &op{
k: k,
k: key,
pastExists: true,
pastV: past,
pastChanged: changed,
})
ts.changedKeys[k] = &tempStorage{nil, true}
ts.changedKeys[string(key)] = &tempStorage{nil, true}
return nil
}

Expand All @@ -188,13 +183,13 @@ func (ts *TState) Rollback(_ context.Context, restorePoint int) {
//
// remove: Removed key that was modified for first time in run
if !op.pastChanged {
delete(ts.changedKeys, op.k)
delete(ts.changedKeys, string(op.k))
continue
}
// insert: Modified key for the nth time
//
// remove: Removed key that was previously modified in run
ts.changedKeys[op.k] = &tempStorage{op.pastV, !op.pastExists}
ts.changedKeys[string(op.k)] = &tempStorage{op.pastV, !op.pastExists}
}
ts.ops = ts.ops[:restorePoint]
}
Expand Down
Loading