perf: improve actor storage performance #990
Conversation
There was a problem hiding this comment.
WriteStorage and Put are on the boundary of the VM abstraction and golang typed data is not supposed to cross it. By the time we really need the abstraction, we'll have a whole new set of performance issues anyway. This optimization is significant, so I think it's worth a little flex.
|
|
||
| var retVal []byte | ||
| outErr, ok := out[len(out)-1].(error) | ||
| outErr, ok := out[len(out)-1].Interface().(error) | ||
| if ok { |
There was a problem hiding this comment.
What if not ok? I realize this isn't something you introduced, just wondering.
There was a problem hiding this comment.
That is the case below, that means it was nil and there was no error
| func (s Storage) Put(v interface{}) (*cid.Cid, error) { | ||
| var nd format.Node | ||
| var err error | ||
| if blk, ok := v.(blocks.Block); ok { |
There was a problem hiding this comment.
This feels pretty hacky to me, though I get why we're doing it and think we should. Do we agree that in the limit there should be one kind of thing that you pass to Put() and that optimizations should happen at a different level?
There was a problem hiding this comment.
Yes, I agree. The long term fix is making go-ipld-cbor much more intelligent and less wasteful. Then we can go back to nearly the same code we had before, modulo integrating that.
This minimizes the amount of marshal & unmarshal calls being made when using
actor.WithStorage.I used the test
TestTipSetWeightDeepas benchmark and went from140sto61s.When applying ipfs/go-ipld-cbor#45 the time goes down to
45s.First round for #979