nspcc-dev / neofs-sdk-go

Go implementation of NeoFS SDK
Apache License 2.0
6 stars 14 forks source link

Do not serialize structures for static signer #400

Closed roman-khimov closed 1 year ago

roman-khimov commented 1 year ago

It can be optimized out, see https://github.com/nspcc-dev/neofs-sdk-go/pull/398#discussion_r1172312597

smallhive commented 1 year ago

It is obvious, that StaticSigner faster and cheaper then regular signer. Despite it we shouldn't have two different methods like Calculate and CalculateLazy. It may lead to misunderstanding to customer, which func should be use to calculate signature.

Instead it I suggest to change Calculate func interface from

func (x *Signature) Calculate(signer Signer, data []byte) error

to

func (x *Signature) Calculate(signer Signer, dataSrc func() []byte) error {

Of course, it leads to some changes in code, but removes misunderstanding. This changed variant has the same performance:

$ go test -bench=. -benchmem
goos: linux
goarch: amd64
pkg: github.com/nspcc-dev/neofs-sdk-go/container
cpu: Intel(R) Core(TM) i7-10875H CPU @ 2.30GHz
BenchmarkCalculate-16                      27002             43873 ns/op            5396 B/op         76 allocs/op
BenchmarkCalculateLazy-16                  27144             44347 ns/op            5396 B/op         76 allocs/op
BenchmarkCalculateLazyStatic-16          1535890               887.0 ns/op           272 B/op          4 allocs/op
BenchmarkCalculateLazySingle-16            27273             46403 ns/op            5396 B/op         76 allocs/op
PASS
ok      github.com/nspcc-dev/neofs-sdk-go/container     6.896s

Description:

Conslusion:

roman-khimov commented 1 year ago

Passing a function means creating some lambda function in 99% of use cases which is just not fun. How about

type MarshalledStably interface {
    StableMarshal([]byte) []byte
}
func (x *Signature) Calculate(signer Signer, data []byte) error {}
func (x *Signature) CalculateMarshalled(signer Signer, smth MarshalledStably) error {}

IIRC we calculate signatures for something with StableMarshal() in 99% of the cases.

smallhive commented 1 year ago

Do you mean left current Calculate as is, append new CalculateMarshalled where we will implement Lazy functionality? I don't mind, even more - I like this approach.

roman-khimov commented 1 year ago

Yeah, generic Calculate can be useful anyway, but marshalled one can be lazy or not depending on the implementation. Static signer will obviously optimize it.

notimetoname commented 1 year ago

Passing a function means creating some lambda function in 99% of use cases which is just not fun.

@roman-khimov, do you mean readability or other reasons?

roman-khimov commented 1 year ago

Mostly that. Performance-wise there will also be a difference, but a tiny one.