From ce40e9e7b194e54aafaadd5b804319b04c762d0d Mon Sep 17 00:00:00 2001 From: Friedel Ziegelmayer Date: Thu, 27 Sep 2018 11:15:21 -0700 Subject: [PATCH] perf: improve actor storage performance (#990) * perf: improve actor storage performance This minimizes the amount of marshal & unmarshal calls being made when using `actor.WithStorage`. I used the test `TestTipSetWeightDeep` as benchmark and went from `140s` to `61s`. When applying https://github.com/ipfs/go-ipld-cbor/pull/45 the time goes down to `45s`. Warning: this reduces the strictness of the vm memory barrier with the rest of filecoin, but we accept this as a tradeof for now, to improve performance. Ref #979 --- actor/export.go | 38 ++++++++++++++------------------------ actor/storage.go | 16 +++------------- exec/exec.go | 4 ++-- vm/context.go | 2 +- vm/storage.go | 25 ++++++++++++++++++------- 5 files changed, 38 insertions(+), 47 deletions(-) diff --git a/actor/export.go b/actor/export.go index e3faa165fc..5b52facd06 100644 --- a/actor/export.go +++ b/actor/export.go @@ -91,38 +91,28 @@ func MakeTypedExport(actor exec.ExecutableActor, method string) exec.ExportedFun args = append(args, reflect.ValueOf(param.Val)) } - toInterfaces := func(v []reflect.Value) []interface{} { - r := make([]interface{}, 0, len(v)) - for _, vv := range v { - r = append(r, vv.Interface()) - } - return r - } - - out := toInterfaces(val.Call(args)) - - exitCode, ok := out[len(out)-2].(uint8) - if !ok { - panic("invalid return value") - } + out := val.Call(args) + exitCode := uint8(out[len(out)-2].Uint()) - var retVal []byte - outErr, ok := out[len(out)-1].(error) + outErr, ok := out[len(out)-1].Interface().(error) if ok { if !(errors.ShouldRevert(outErr) || errors.IsFault(outErr)) { panic("you are a bad person: error must be either a reverterror or a fault") } - } else { - // The value of the returned error was nil. - outErr = nil - retVal, err = abi.ToEncodedValues(out[:len(out)-2]...) - if err != nil { - return nil, 1, errors.FaultErrorWrap(err, "failed to marshal output value") - } + return nil, exitCode, outErr + } + + vals := make([]interface{}, 0, len(out)-2) + for _, vv := range out[:len(out)-2] { + vals = append(vals, vv.Interface()) + } + retVal, err := abi.ToEncodedValues(vals...) + if err != nil { + return nil, 1, errors.FaultErrorWrap(err, "failed to marshal output value") } - return retVal, exitCode, outErr + return retVal, exitCode, nil } } diff --git a/actor/storage.go b/actor/storage.go index 688fe91578..5f008007f5 100644 --- a/actor/storage.go +++ b/actor/storage.go @@ -52,12 +52,7 @@ func WithState(ctx exec.VMContext, st interface{}, f func() (interface{}, error) return nil, err } - data, err := MarshalStorage(st) - if err != nil { - return nil, err - } - - if err := ctx.WriteStorage(data); err != nil { + if err := ctx.WriteStorage(st); err != nil { return nil, err } @@ -156,7 +151,7 @@ func (sab *storageAsBlocks) GetBlock(ctx context.Context, c *cid.Cid) (block.Blo // AddBlock add a block to underlying storage func (sab *storageAsBlocks) AddBlock(b block.Block) error { - _, err := sab.s.Put(b.RawData()) + _, err := sab.s.Put(b) return err } @@ -205,12 +200,7 @@ func (l *lookup) Commit(ctx context.Context) (*cid.Cid, error) { return nil, err } - chunk, err := cbor.DumpObject(l.n) - if err != nil { - return nil, err - } - - return l.s.Put(chunk) + return l.s.Put(l.n) } // IsEmpty returns true if this node contains no key values diff --git a/exec/exec.go b/exec/exec.go index de4eea7c64..4539ed5f9e 100644 --- a/exec/exec.go +++ b/exec/exec.go @@ -76,13 +76,13 @@ type VMContext interface { // TODO: Remove these when Storage above is completely implemented ReadStorage() ([]byte, error) - WriteStorage(memory []byte) error + WriteStorage(interface{}) error } // Storage defines the storage module exposed to actors. type Storage interface { // TODO: Forgot that Put() can fail in the spec, need to update. - Put([]byte) (*cid.Cid, error) + Put(interface{}) (*cid.Cid, error) Get(*cid.Cid) ([]byte, error) Commit(*cid.Cid, *cid.Cid) error Head() *cid.Cid diff --git a/vm/context.go b/vm/context.go index 474464a077..79a295bce6 100644 --- a/vm/context.go +++ b/vm/context.go @@ -75,7 +75,7 @@ func (ctx *Context) ReadStorage() ([]byte, error) { } // WriteStorage writes to the storage of the associated to actor. -func (ctx *Context) WriteStorage(memory []byte) error { +func (ctx *Context) WriteStorage(memory interface{}) error { stage := ctx.Storage() cid, err := stage.Put(memory) diff --git a/vm/storage.go b/vm/storage.go index 2360bd63b5..46b42b8e48 100644 --- a/vm/storage.go +++ b/vm/storage.go @@ -4,7 +4,8 @@ import ( "errors" cbor "gx/ipfs/QmV6BQ6fFCf9eFHDuRxvguvqfKLZtZrxthgZvDfRCs4tMN/go-ipld-cbor" - "gx/ipfs/QmWAzSEoqZ6xU6pu8yL8e5WaMb7wtbfbhhN4p1DknUPtr3/go-block-format" + blocks "gx/ipfs/QmWAzSEoqZ6xU6pu8yL8e5WaMb7wtbfbhhN4p1DknUPtr3/go-block-format" + format "gx/ipfs/QmX5CsuHyVZeTLxgRSYkgLSDQKb9UjE8xnhQzCEJWWWFsC/go-ipld-format" ipld "gx/ipfs/QmX5CsuHyVZeTLxgRSYkgLSDQKb9UjE8xnhQzCEJWWWFsC/go-ipld-format" "gx/ipfs/QmZFbDTY9jfSBms2MchvYM9oYRbAF19K7Pby47yDBfpPrb/go-cid" "gx/ipfs/QmcmpX42gtDv1fz24kau4wjS9hfwWj5VexWBKgGnWzsyag/go-ipfs-blockstore" @@ -101,16 +102,26 @@ func NewStorage(bs blockstore.Blockstore, act *actor.Actor) Storage { } } -// Put adds a node to temporary storage by id -func (s Storage) Put(chunk []byte) (*cid.Cid, error) { - n, err := cbor.Decode(chunk, types.DefaultHashFunction, -1) +// Put adds a node to temporary storage by id. +func (s Storage) Put(v interface{}) (*cid.Cid, error) { + var nd format.Node + var err error + if blk, ok := v.(blocks.Block); ok { + // optimize putting blocks + nd, err = cbor.DecodeBlock(blk) + } else if bytes, ok := v.([]byte); ok { + nd, err = cbor.Decode(bytes, types.DefaultHashFunction, -1) + } else { + nd, err = cbor.WrapObject(v, types.DefaultHashFunction, -1) + } if err != nil { return nil, exec.Errors[exec.ErrDecode] } - cid := n.Cid() - s.chunks[cid.KeyString()] = n - return cid, nil + c := nd.Cid() + s.chunks[c.KeyString()] = nd + + return c, nil } // Get retrieves a chunk from either temporary storage or its backing store.