TrueCloudLab/frostfs-api-go
21
0
Fork 13
Code Issues 9 Pull requests 3 Projects Releases Activity Actions

Use custom protoc plugin for codegen #77

Merged
fyrchik merged 1 commit from fyrchik/frostfs-api-go:update-marshaling into master 2024-09-04 19:51:16 +00:00
Conversation 5 Commits 1 Files changed 1 +3 -3
fyrchik commented 2024-04-26 10:11:08 +00:00
Owner
Copy link

easyproto + easyjson = easy protoc plugin <3

It touches only grpc/ packages, so all code on which SDK is dependent is untouched.

TBD:

  • end-to-end test with on the dev-env (frostfs-cli + frostfs-node in different version combinations =
    before/after this change)
  • try generating tests automatically too (e.g. fuzzing)
  • make code more readable

Depends on #75, #76

easyproto + easyjson = easy protoc plugin <3 It touches only `grpc/` packages, so all code on which SDK is dependent is untouched. TBD: - end-to-end test with on the dev-env (frostfs-cli + frostfs-node in different version combinations = before/after this change) - try generating tests automatically too (e.g. fuzzing) - make code more readable Depends on #75, #76
fyrchik commented 2024-04-26 10:17:57 +00:00
Author
Owner
Copy link

I am not sure we need it here, maybe now is the time to move to a separate repo.

I am not sure we need it here, maybe now is the time to move to a separate repo.
fyrchik commented 2024-04-26 12:56:24 +00:00
Author
Owner
Copy link

Here are results for object.Unmarshal (this will be transparently picked up by frostfs-node storage engine)

func BenchmarkObjectUnmarshal(b *testing.B) {
	for _, payloadSize := range []int{1024, 8 * 1024, 1024 * 1024} {
		b.Run(strconv.Itoa(payloadSize), func(b *testing.B) {
			obj := testutil.GenerateObject()
			obj.SetPayloadSize(uint64(payloadSize))
			obj.SetPayload(make([]byte, payloadSize))

			data, err := obj.Marshal()
			require.NoError(b, err)

			b.Run("marshal", func(b *testing.B) {
				b.ReportAllocs()
				for i := 0; i < b.N; i++ {
					if _, err := obj.Marshal(); err != nil {
						b.Fatal(err)
					}
				}
			})
			b.Run("unmarshal", func(b *testing.B) {
				b.ReportAllocs()
				for i := 0; i < b.N; i++ {
					var obj objectSDK.Object
					if err := obj.Unmarshal(data); err != nil {
						b.Fatal(err)
					}
				}
			})
		})
	}
}

goos: linux
goarch: amd64
pkg: git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/metabase
cpu: 11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz
                                    │       old       │                 new                  │
                                    │     sec/op      │    sec/op     vs base                │
ObjectUnmarshal/1024/marshal-8           494.2n ±  3%   484.9n ±  5%        ~ (p=0.256 n=10)
ObjectUnmarshal/1024/unmarshal-8        1612.0n ±  7%   855.6n ± 12%  -46.92% (p=0.000 n=10)
ObjectUnmarshal/8192/marshal-8           1.360µ ± 10%   1.600µ ± 19%  +17.69% (p=0.043 n=10)
ObjectUnmarshal/8192/unmarshal-8        2915.5n ±  3%   912.5n ± 34%  -68.70% (p=0.000 n=10)
ObjectUnmarshal/1048576/marshal-8        183.5µ ± 10%   272.7µ ± 16%  +48.60% (p=0.000 n=10)
ObjectUnmarshal/1048576/unmarshal-8   159321.5n ±  6%   881.1n ±  7%  -99.45% (p=0.000 n=10)
geomean                                  6.723µ         2.294µ        -65.88%

                                    │      old       │                  new                   │
                                    │      B/op      │     B/op      vs base                  │
ObjectUnmarshal/1024/marshal-8          1.250Ki ± 0%   1.250Ki ± 0%        ~ (p=1.000 n=10) ¹
ObjectUnmarshal/1024/unmarshal-8         2208.0 ± 0%     864.0 ± 0%  -60.87% (p=0.000 n=10)
ObjectUnmarshal/8192/marshal-8          9.250Ki ± 0%   9.250Ki ± 0%        ~ (p=1.000 n=10) ¹
ObjectUnmarshal/8192/unmarshal-8         9376.0 ± 0%     864.0 ± 0%  -90.78% (p=0.000 n=10)
ObjectUnmarshal/1048576/marshal-8       1.008Mi ± 0%   1.008Mi ± 0%   +0.00% (p=0.001 n=10)
ObjectUnmarshal/1048576/unmarshal-8   1049768.0 ± 0%     864.0 ± 0%  -99.92% (p=0.000 n=10)
geomean                                 24.95Ki        4.391Ki       -82.40%
¹ all samples are equal

                                    │    old     │                 new                  │
                                    │ allocs/op  │ allocs/op   vs base                  │
ObjectUnmarshal/1024/marshal-8        1.000 ± 0%   1.000 ± 0%        ~ (p=1.000 n=10) ¹
ObjectUnmarshal/1024/unmarshal-8      22.00 ± 0%   16.00 ± 0%  -27.27% (p=0.000 n=10)
ObjectUnmarshal/8192/marshal-8        1.000 ± 0%   1.000 ± 0%        ~ (p=1.000 n=10) ¹
ObjectUnmarshal/8192/unmarshal-8      22.00 ± 0%   16.00 ± 0%  -27.27% (p=0.000 n=10)
ObjectUnmarshal/1048576/marshal-8     1.000 ± 0%   1.000 ± 0%        ~ (p=1.000 n=10) ¹
ObjectUnmarshal/1048576/unmarshal-8   22.00 ± 0%   16.00 ± 0%  -27.27% (p=0.000 n=10)
geomean                               4.690        4.000       -14.72%
¹ all samples are equal
Here are results for `object.Unmarshal` (this will be transparently picked up by frostfs-node storage engine) ```golang func BenchmarkObjectUnmarshal(b *testing.B) { for _, payloadSize := range []int{1024, 8 * 1024, 1024 * 1024} { b.Run(strconv.Itoa(payloadSize), func(b *testing.B) { obj := testutil.GenerateObject() obj.SetPayloadSize(uint64(payloadSize)) obj.SetPayload(make([]byte, payloadSize)) data, err := obj.Marshal() require.NoError(b, err) b.Run("marshal", func(b *testing.B) { b.ReportAllocs() for i := 0; i < b.N; i++ { if _, err := obj.Marshal(); err != nil { b.Fatal(err) } } }) b.Run("unmarshal", func(b *testing.B) { b.ReportAllocs() for i := 0; i < b.N; i++ { var obj objectSDK.Object if err := obj.Unmarshal(data); err != nil { b.Fatal(err) } } }) }) } } ``` ``` goos: linux goarch: amd64 pkg: git.frostfs.info/TrueCloudLab/frostfs-node/pkg/local_object_storage/metabase cpu: 11th Gen Intel(R) Core(TM) i5-1135G7 @ 2.40GHz │ old │ new │ │ sec/op │ sec/op vs base │ ObjectUnmarshal/1024/marshal-8 494.2n ± 3% 484.9n ± 5% ~ (p=0.256 n=10) ObjectUnmarshal/1024/unmarshal-8 1612.0n ± 7% 855.6n ± 12% -46.92% (p=0.000 n=10) ObjectUnmarshal/8192/marshal-8 1.360µ ± 10% 1.600µ ± 19% +17.69% (p=0.043 n=10) ObjectUnmarshal/8192/unmarshal-8 2915.5n ± 3% 912.5n ± 34% -68.70% (p=0.000 n=10) ObjectUnmarshal/1048576/marshal-8 183.5µ ± 10% 272.7µ ± 16% +48.60% (p=0.000 n=10) ObjectUnmarshal/1048576/unmarshal-8 159321.5n ± 6% 881.1n ± 7% -99.45% (p=0.000 n=10) geomean 6.723µ 2.294µ -65.88% │ old │ new │ │ B/op │ B/op vs base │ ObjectUnmarshal/1024/marshal-8 1.250Ki ± 0% 1.250Ki ± 0% ~ (p=1.000 n=10) ¹ ObjectUnmarshal/1024/unmarshal-8 2208.0 ± 0% 864.0 ± 0% -60.87% (p=0.000 n=10) ObjectUnmarshal/8192/marshal-8 9.250Ki ± 0% 9.250Ki ± 0% ~ (p=1.000 n=10) ¹ ObjectUnmarshal/8192/unmarshal-8 9376.0 ± 0% 864.0 ± 0% -90.78% (p=0.000 n=10) ObjectUnmarshal/1048576/marshal-8 1.008Mi ± 0% 1.008Mi ± 0% +0.00% (p=0.001 n=10) ObjectUnmarshal/1048576/unmarshal-8 1049768.0 ± 0% 864.0 ± 0% -99.92% (p=0.000 n=10) geomean 24.95Ki 4.391Ki -82.40% ¹ all samples are equal │ old │ new │ │ allocs/op │ allocs/op vs base │ ObjectUnmarshal/1024/marshal-8 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ ObjectUnmarshal/1024/unmarshal-8 22.00 ± 0% 16.00 ± 0% -27.27% (p=0.000 n=10) ObjectUnmarshal/8192/marshal-8 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ ObjectUnmarshal/8192/unmarshal-8 22.00 ± 0% 16.00 ± 0% -27.27% (p=0.000 n=10) ObjectUnmarshal/1048576/marshal-8 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ ObjectUnmarshal/1048576/unmarshal-8 22.00 ± 0% 16.00 ± 0% -27.27% (p=0.000 n=10) geomean 4.690 4.000 -14.72% ¹ all samples are equal ```
fyrchik force-pushed update-marshaling from c9f18aba0b to fbba808d45 2024-04-27 08:25:20 +00:00 Compare
fyrchik force-pushed update-marshaling from fbba808d45 to de3eb95dd3 2024-05-03 06:18:46 +00:00 Compare
fyrchik force-pushed update-marshaling from de3eb95dd3 to 3e23366c52 2024-05-03 07:11:54 +00:00 Compare
fyrchik commented 2024-05-03 07:41:20 +00:00
Author
Owner
Copy link

Added fuzz test autogeneration.
For now, just for unmarshal.
In future we may add grpc/test/ package with native protoc codegen and check that our code and protoc code return equal errors (nil or non-nil) on any input.

Added fuzz test autogeneration. For now, just for unmarshal. In future we may add `grpc/test/` package with native protoc codegen and check that our code and protoc code return equal errors (nil or non-nil) on any input.
fyrchik force-pushed update-marshaling from 3e23366c52 to 59d7bd7a59 2024-05-07 15:34:53 +00:00 Compare
fyrchik force-pushed update-marshaling from 59d7bd7a59 to 2b11ab4974 2024-05-07 15:35:53 +00:00 Compare
fyrchik force-pushed update-marshaling from 2b11ab4974 to e990204c95 2024-05-14 12:04:18 +00:00 Compare
fyrchik force-pushed update-marshaling from e990204c95 to dcbbd4fdcd 2024-05-22 10:41:12 +00:00 Compare
fyrchik force-pushed update-marshaling from dcbbd4fdcd to 7f903df1e1 2024-06-04 12:46:43 +00:00 Compare
fyrchik force-pushed update-marshaling from 7f903df1e1 to 5bae7bf6a9 2024-08-01 13:10:08 +00:00 Compare
fyrchik force-pushed update-marshaling from 5bae7bf6a9 to 03759317ef 2024-08-06 13:56:45 +00:00 Compare
fyrchik changed title from WIP: use custom protoc plugin for codegen to use custom protoc plugin for codegen 2024-08-06 13:57:07 +00:00
fyrchik changed title from use custom protoc plugin for codegen to Use custom protoc plugin for codegen 2024-08-06 13:57:21 +00:00
fyrchik commented 2024-08-06 13:57:41 +00:00
Author
Owner
Copy link

frostfs-cli + frostfs-node + frostfs-ir + frostfs-devenv works, caught a bug with tracing exporter.

frostfs-cli + frostfs-node + frostfs-ir + frostfs-devenv works, caught a bug with tracing exporter.
fyrchik force-pushed update-marshaling from 03759317ef to 180f0bd106 2024-08-07 11:50:34 +00:00 Compare
fyrchik force-pushed update-marshaling from 180f0bd106 to 6dc55b3fca 2024-08-08 12:35:36 +00:00 Compare
fyrchik force-pushed update-marshaling from 6dc55b3fca to 3a1a6c72f3 2024-08-08 13:57:00 +00:00 Compare
fyrchik force-pushed update-marshaling from 3a1a6c72f3 to c225e95c49 2024-08-08 14:00:57 +00:00 Compare
fyrchik force-pushed update-marshaling from c225e95c49 to 30bf64441d 2024-08-09 06:57:32 +00:00 Compare
fyrchik force-pushed update-marshaling from 30bf64441d to 84a552d150 2024-08-09 14:29:14 +00:00 Compare
fyrchik force-pushed update-marshaling from 84a552d150 to 209f5ecace 2024-08-09 15:11:45 +00:00 Compare
fyrchik force-pushed update-marshaling from 209f5ecace to 4932274d34 2024-08-12 14:06:13 +00:00 Compare
acid-ant reviewed 2024-08-12 14:51:09 +00:00
rpc/message/test/message.go Outdated
@ -44,6 +44,7 @@ func TestRPCMessage(t *testing.T, msgGens ...func(empty bool) message.Message) {
if jm, ok := msg.(jsonMessage); ok {
t.Run(fmt.Sprintf("JSON_%T", msg), func(t *testing.T) {
data, err := jm.MarshalJSON()
fmt.Println(string(data))
acid-ant commented 2024-08-12 14:51:09 +00:00
Member
Copy link

Added for debugging?

Added for debugging?
fyrchik commented 2024-08-13 14:29:02 +00:00
Author
Owner
Copy link

yes, removed

yes, removed
acid-ant marked this conversation as resolved
acid-ant reviewed 2024-08-12 14:54:57 +00:00
util/proto/marshal_test.go Outdated
@ -57,0 +60,4 @@
buf := make([]byte, 12)
buf = buf[:proto.Int32Marshal(201, buf, tc.input.FieldD)]
spew.Dump(buf)
spew.Dump(r)
acid-ant commented 2024-08-12 14:54:57 +00:00
Member
Copy link

Remove please.

Remove please.
fyrchik commented 2024-08-13 14:28:53 +00:00
Author
Owner
Copy link

Removed

Removed
acid-ant marked this conversation as resolved
fyrchik force-pushed update-marshaling from 4932274d34 to b1f18a1c69 2024-08-13 07:19:15 +00:00 Compare
fyrchik force-pushed update-marshaling from b1f18a1c69 to 6e8a0e09af 2024-08-13 07:23:11 +00:00 Compare
fyrchik force-pushed update-marshaling from 6e8a0e09af to 16920ed18d 2024-08-13 14:25:23 +00:00 Compare
fyrchik force-pushed update-marshaling from 16920ed18d to d2622dbdae 2024-08-13 14:28:39 +00:00 Compare
fyrchik requested review from storage-core-committers 2024-08-13 14:29:13 +00:00
fyrchik requested review from storage-core-developers 2024-08-13 14:29:13 +00:00
fyrchik force-pushed update-marshaling from d2622dbdae to bf048fe8d4 2024-08-15 10:54:15 +00:00 Compare
acid-ant approved these changes 2024-08-15 14:20:22 +00:00
rpc/message/encoding.go Outdated
@ -24,3 +21,4 @@
}
return m.FromGRPCMessage(gm)
// if err := proto.Unmarshal(data, gm); err != nil {
acid-ant commented 2024-08-15 14:11:56 +00:00
Member
Copy link

Looks like these lines no more needed.

Looks like these lines no more needed.
fyrchik commented 2024-08-16 08:09:06 +00:00
Author
Owner
Copy link

fixed

fixed
acid-ant marked this conversation as resolved
rpc/message/encoding.go Outdated
@ -34,2 +33,2 @@
m.ToGRPCMessage().(proto.Message),
)
return json.Marshal(m.ToGRPCMessage())
// return protojson.MarshalOptions{
acid-ant commented 2024-08-15 14:12:03 +00:00
Member
Copy link

Looks like these lines no more needed.

Looks like these lines no more needed.
fyrchik commented 2024-08-16 08:09:14 +00:00
Author
Owner
Copy link

fixed

fixed
acid-ant marked this conversation as resolved
rpc/message/encoding.go Outdated
@ -45,3 +48,3 @@
}
return m.FromGRPCMessage(gm)
// if err := protojson.Unmarshal(data, gm); err != nil {
acid-ant commented 2024-08-15 14:12:09 +00:00
Member
Copy link

Looks like these lines no more needed.

Looks like these lines no more needed.
fyrchik commented 2024-08-16 08:09:22 +00:00
Author
Owner
Copy link

fixed

fixed
acid-ant marked this conversation as resolved
aarifullin approved these changes 2024-08-15 14:44:46 +00:00
fyrchik force-pushed update-marshaling from bf048fe8d4 to b6910f744d 2024-08-16 08:09:06 +00:00 Compare
fyrchik commented 2024-08-16 08:10:35 +00:00
Author
Owner
Copy link

Added JSON tests.

Added JSON tests.
fyrchik force-pushed update-marshaling from b6910f744d to cc0199d82b 2024-08-16 08:10:55 +00:00 Compare
acid-ant approved these changes 2024-08-16 09:44:14 +00:00
fyrchik force-pushed update-marshaling from cc0199d82b to 8544f6dacf 2024-08-16 14:09:12 +00:00 Compare
fyrchik force-pushed update-marshaling from 8544f6dacf to 01be4470e6 2024-08-16 14:11:19 +00:00 Compare
fyrchik force-pushed update-marshaling from 01be4470e6 to 038e475ba3 2024-08-16 14:17:41 +00:00 Compare
acid-ant approved these changes 2024-08-16 14:25:49 +00:00
fyrchik force-pushed update-marshaling from 038e475ba3 to 24b312c3c7 2024-08-16 14:27:34 +00:00 Compare
acid-ant approved these changes 2024-08-19 07:12:51 +00:00
fyrchik force-pushed update-marshaling from 24b312c3c7 to a43110e363 2024-08-19 07:47:16 +00:00 Compare
fyrchik merged commit a43110e363 into master 2024-08-19 15:25:03 +00:00
fyrchik deleted branch update-marshaling 2024-08-19 15:25:07 +00:00
Sign in to join this conversation.
Reviewers
No reviewers
aarifullin
acid-ant
Labels
Clear labels   
P0

Highest

  
P1

High

  
P2

Medium

  
P3

Low

  
good first issue

Good for newcomers

  
Infrastructure

Infrastructure, Automation, CI/CD or DevOps related stuff

  
blocked

The issue or PR is blocked by something

  
bug

Something is not working

  
config

Configuration format update or breaking change

  
discussion

Talking about something in order to reach a decision or to exchange ideas

  
documentation

Documentation related

  
duplicate

This issue or pull request already exists

  
enhancement

New feature

  
go

Language features adoption

  
help wanted

Extra attention is needed

  
internal

   
invalid

Something is wrong

  
kludge

This is a kludge and we know it

  
observability

Related to metrics, tracing and logs

  
perfomance

Perfomance related

  
question

More information is needed

  
refactoring

Refactoring

  
wontfix

This won't be fixed

No labels
P0
P1
P2
P3
good first issue
Infrastructure
blocked
bug
config
discussion
documentation
duplicate
enhancement
go
help wanted
internal
invalid
kludge
observability
perfomance
question
refactoring
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
3 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: TrueCloudLab/frostfs-api-go#77
No description provided.
Delete branch "fyrchik/frostfs-api-go:update-marshaling"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?