Refactor sign/verify message #6

fyrchik · 2023-02-28T14:56:14Z

dstepanov-yadro commented

2023-02-28 14:56:14 +00:00

(Migrated from github.com)

Code refactor: drop some wrappers, split methods to different files, extract methods
Do signing/verification parallel
benchstat compare with master:

1. Code refactor: drop some wrappers, split methods to different files, extract methods 2. Do signing/verification parallel benchstat compare with master: ![image](https://user-images.githubusercontent.com/125876766/221891081-6da503b9-bceb-4b7d-b2ae-fd581ed03b2f.png)

fyrchik (Migrated from github.com) reviewed 2023-02-28 14:56:14 +00:00

fyrchik (Migrated from github.com) reviewed 2023-02-28 15:27:13 +00:00

fyrchik (Migrated from github.com) left a comment

I think api-go: can be replaced with a signature: in the commit header, it should designate a component in this repo (or omitted).
Also I believe it should be Verify parts IN parallel.

I think `api-go:` can be replaced with a `signature:` in the commit header, it should designate a component in this repo (or omitted). Also I believe it should be `Verify parts IN parallel`.

signature/verify.go

					
				@ -0,0 +32,4 @@

					case serviceResponse:

						return verifyServiceResponse(v)

					default:

						panic(fmt.Sprintf("unsupported session message %T", v))

fyrchik (Migrated from github.com) commented

2023-02-28 15:18:50 +00:00

I am wondering if it makes sense to allocate once and use sub-slices here. Probably not, can bite us in an unexpected way.

fyrchik (Migrated from github.com) commented

2023-02-28 15:20:25 +00:00

Max was here when we had a single buffer, do we need a single size now?

`Max` was here when we had a single buffer, do we need a single `size` now?

util/collection/slice.go

fyrchik (Migrated from github.com) commented

2023-02-28 15:22:51 +00:00

It seems to be used only from 1 package, why have you decided to exprort it?

Also, it isn't needed after parallelizations (see my later comment).

It seems to be used only from 1 package, why have you decided to exprort it? Also, it isn't needed after parallelizations (see my later comment).

dstepanov-yadro (Migrated from github.com) reviewed 2023-02-28 15:57:49 +00:00

util/collection/slice.go

dstepanov-yadro (Migrated from github.com) commented

2023-02-28 15:57:48 +00:00

dropped

dstepanov-yadro (Migrated from github.com) reviewed 2023-02-28 15:58:15 +00:00

signature/verify.go

					
				@ -0,0 +32,4 @@

					case serviceResponse:

						return verifyServiceResponse(v)

					default:

						panic(fmt.Sprintf("unsupported session message %T", v))

dstepanov-yadro (Migrated from github.com) commented

2023-02-28 15:58:15 +00:00

fixed

dstepanov-yadro (Migrated from github.com) reviewed 2023-02-28 15:58:40 +00:00

signature/verify.go

					
				@ -0,0 +32,4 @@

					case serviceResponse:

						return verifyServiceResponse(v)

					default:

						panic(fmt.Sprintf("unsupported session message %T", v))

dstepanov-yadro (Migrated from github.com) commented

2023-02-28 15:58:40 +00:00

fixed

dstepanov-yadro commented

2023-02-28 15:58:51 +00:00

(Migrated from github.com)

I think api-go: can be replaced with a signature: in the commit header, it should designate a component in this repo (or omitted). Also I believe it should be Verify parts IN parallel.

fixed

> I think `api-go:` can be replaced with a `signature:` in the commit header, it should designate a component in this repo (or omitted). Also I believe it should be `Verify parts IN parallel`. fixed

acid-ant (Migrated from github.com) reviewed 2023-02-28 16:25:27 +00:00

fyrchik commented

2023-03-07 07:56:03 +00:00

Superceeded by #8.

fyrchik closed this pull request

2023-03-07 07:56:03 +00:00

fyrchik deleted branch dstepanov-yadro/feat/refactor-signservicemessage

2023-03-07 07:56:09 +00:00