Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade the ProtocolBuffer compiler. #213

Closed
wants to merge 2 commits into from
Closed

Conversation

L3n41c
Copy link
Member

@L3n41c L3n41c commented Dec 22, 2022

  • protoc has been upgraded from 3.5.1 to 21.12.
  • gogo/protobuf fork has been replaced by upstream protocolbuffers/protobuf because gogo/protobuf is deprecated.

I had to do this upgrade because I encountered some troubles while trying to compile the CycloneDX SBOM definition in #212.

Here are some examples of error I got when trying to compile the upstream CycloneDX SBOM definition without the deprecation version of gogo/protobuf:

Generating sbom proto
proto/deps/github.com/CycloneDX/specification/schema/bom-1.4.proto:9:12: Explicit 'optional' labels are disallowed in the Proto3 syntax. To define 'optional' fields in Proto3, simply remove the 'optional' label, as fields are 'optional' by default.
proto/deps/github.com/CycloneDX/specification/schema/bom-1.4.proto:11:12: Explicit 'optional' labels are disallowed in the Proto3 syntax. To define 'optional' fields in Proto3, simply remove the 'optional' label, as fields are 'optional' by default.
proto/deps/github.com/CycloneDX/specification/schema/bom-1.4.proto:20:12: Explicit 'optional' labels are disallowed in the Proto3 syntax. To define 'optional' fields in Proto3, simply remove the 'optional' label, as fields are 'optional' by default.
proto/deps/github.com/CycloneDX/specification/schema/bom-1.4.proto:22:12: Explicit 'optional' labels are disallowed in the Proto3 syntax. To define 'optional' fields in Proto3, simply remove the 'optional' label, as fields are 'optional' by default.
proto/deps/github.com/CycloneDX/specification/schema/bom-1.4.proto:24:12: Explicit 'optional' labels are disallowed in the Proto3 syntax. To define 'optional' fields in Proto3, simply remove the 'optional' label, as fields are 'optional' by default.
proto/deps/github.com/CycloneDX/specification/schema/bom-1.4.proto:61:12: Explicit 'optional' labels are disallowed in the Proto3 syntax. To define 'optional' fields in Proto3, simply remove the 'optional' label, as fields are 'optional' by default.
proto/deps/github.com/CycloneDX/specification/schema/bom-1.4.proto:63:12: Explicit 'optional' labels are disallowed in the Proto3 syntax. To define 'optional' fields in Proto3, simply remove the 'optional' label, as fields are 'optional' by default.
…
# github.com/DataDog/agent-payload/v5/cyclonedx_v1_4
cyclonedx_v1_4/bom-1.4.pb.go:3522:55: m.Timestamp.Size undefined (type *timestamppb.Timestamp has no field or method Size)
cyclonedx_v1_4/bom-1.4.pb.go:3523:26: m.Timestamp.MarshalTo undefined (type *timestamppb.Timestamp has no field or method MarshalTo)
cyclonedx_v1_4/bom-1.4.pb.go:3762:55: m.Timestamp.Size undefined (type *timestamppb.Timestamp has no field or method Size)
cyclonedx_v1_4/bom-1.4.pb.go:3763:27: m.Timestamp.MarshalTo undefined (type *timestamppb.Timestamp has no field or method MarshalTo)
cyclonedx_v1_4/bom-1.4.pb.go:4585:55: m.Timestamp.Size undefined (type *timestamppb.Timestamp has no field or method Size)
cyclonedx_v1_4/bom-1.4.pb.go:4586:27: m.Timestamp.MarshalTo undefined (type *timestamppb.Timestamp has no field or method MarshalTo)
cyclonedx_v1_4/bom-1.4.pb.go:4773:53: m.Created.Size undefined (type *timestamppb.Timestamp has no field or method Size)
cyclonedx_v1_4/bom-1.4.pb.go:4774:25: m.Created.MarshalTo undefined (type *timestamppb.Timestamp has no field or method MarshalTo)
cyclonedx_v1_4/bom-1.4.pb.go:4783:55: m.Published.Size undefined (type *timestamppb.Timestamp has no field or method Size)
cyclonedx_v1_4/bom-1.4.pb.go:4784:27: m.Published.MarshalTo undefined (type *timestamppb.Timestamp has no field or method MarshalTo)
cyclonedx_v1_4/bom-1.4.pb.go:4784:27: too many errors

I ran the benchmark before and after the upgrade.

  • Before the upgrade:
goos: linux
goarch: amd64
pkg: github.com/DataDog/agent-payload/v5/process
cpu: Intel(R) Xeon(R) CPU @ 2.20GHz
BenchmarkDNSDecode
BenchmarkDNSDecode/samples.txt
BenchmarkDNSDecode/samples.txt-8                    4323            271051 ns/op           45016 B/op       1161 allocs/op
BenchmarkDNSDecode/big_ips.txt
BenchmarkDNSDecode/big_ips.txt-8                   26408             45198 ns/op            6240 B/op        142 allocs/op
BenchmarkDNSDecode/big_entries.txt
BenchmarkDNSDecode/big_entries.txt-8               25825             45620 ns/op            6792 B/op        160 allocs/op
BenchmarkDNSEncode
BenchmarkDNSEncode/samples.txt
BenchmarkDNSEncode/samples.txt-8                    1958            564821 ns/op             48106 bytes          128338 B/op       1149 allocs/op
BenchmarkDNSEncode/big_ips.txt
BenchmarkDNSEncode/big_ips.txt-8                   26799             45494 ns/op              2429 bytes            9296 B/op        112 allocs/op
BenchmarkDNSEncode/big_entries.txt
BenchmarkDNSEncode/big_entries.txt-8               25825             46958 ns/op              2257 bytes            7755 B/op        102 allocs/op
BenchmarkDNSV2Decode
BenchmarkDNSV2Decode/samples.txt
BenchmarkDNSV2Decode/samples.txt-8                  6421            182514 ns/op             502 B/op        124 allocs/op
BenchmarkDNSV2Decode/big_ips.txt
BenchmarkDNSV2Decode/big_ips.txt-8                 35188             33940 ns/op              57 B/op         13 allocs/op
BenchmarkDNSV2Decode/big_entries.txt
BenchmarkDNSV2Decode/big_entries.txt-8             34520             34845 ns/op             114 B/op         27 allocs/op
BenchmarkDNSV2Encode
BenchmarkDNSV2Encode/samples.txt
BenchmarkDNSV2Encode/samples.txt-8                  3880            299718 ns/op             17169 bytes           62712 B/op       1086 allocs/op
BenchmarkDNSV2Encode/big_ips.txt
BenchmarkDNSV2Encode/big_ips.txt-8                 41476             28877 ns/op              2131 bytes            8496 B/op        111 allocs/op
BenchmarkDNSV2Encode/big_entries.txt
BenchmarkDNSV2Encode/big_entries.txt-8             38641             31480 ns/op              1961 bytes            7488 B/op        102 allocs/op
BenchmarkTagEncode
BenchmarkTagEncode-8                               43138             29505 ns/op           66913 B/op         17 allocs/op
BenchmarkTagEncoders
BenchmarkTagEncoders/low_dups_v2
BenchmarkTagEncoders/low_dups_v2-8                186968              6225 ns/op              2253 bytes            2371 B/op          2 allocs/op
BenchmarkTagEncoders/low_dups_v1
BenchmarkTagEncoders/low_dups_v1-8                340032              3310 ns/op              2072 bytes            6297 B/op          6 allocs/op
BenchmarkTagEncoders/high_dups_v2
BenchmarkTagEncoders/high_dups_v2-8                14582             83532 ns/op             26601 bytes           28365 B/op          7 allocs/op
BenchmarkTagEncoders/high_dups_v1
BenchmarkTagEncoders/high_dups_v1-8                10000            104431 ns/op             51227 bytes          248986 B/op         22 allocs/op
BenchmarkTagEncoders/high_dups_2_v2
BenchmarkTagEncoders/high_dups_2_v2-8              30013             39533 ns/op             13022 bytes           13765 B/op          2 allocs/op
BenchmarkTagEncoders/high_dups_2_v1
BenchmarkTagEncoders/high_dups_2_v1-8              22963             47993 ns/op             28509 bytes          115289 B/op         18 allocs/op
BenchmarkTagEncoders/combined_v2
BenchmarkTagEncoders/combined_v2-8                  7879            134738 ns/op             41866 bytes           49325 B/op          2 allocs/op
BenchmarkTagEncoders/combined_v1
BenchmarkTagEncoders/combined_v1-8                  7098            166963 ns/op             81806 bytes          395035 B/op         28 allocs/op
PASS
ok      github.com/DataDog/agent-payload/v5/process     32.124s

After the upgrade:

goos: linux
goarch: amd64
pkg: github.com/DataDog/agent-payload/v5/process
cpu: Intel(R) Xeon(R) CPU @ 2.20GHz
BenchmarkDNSDecode
BenchmarkDNSDecode/samples.txt
BenchmarkDNSDecode/samples.txt-8                    4309            273284 ns/op           45016 B/op       1161 allocs/op
BenchmarkDNSDecode/big_ips.txt
BenchmarkDNSDecode/big_ips.txt-8                   26301             44662 ns/op            6240 B/op        142 allocs/op
BenchmarkDNSDecode/big_entries.txt
BenchmarkDNSDecode/big_entries.txt-8               25369             46798 ns/op            6792 B/op        160 allocs/op
BenchmarkDNSEncode
BenchmarkDNSEncode/samples.txt
BenchmarkDNSEncode/samples.txt-8                    2187            555840 ns/op             48108 bytes          128365 B/op       1149 allocs/op
BenchmarkDNSEncode/big_ips.txt
BenchmarkDNSEncode/big_ips.txt-8                   25695             45922 ns/op              2432 bytes            9296 B/op        112 allocs/op
BenchmarkDNSEncode/big_entries.txt
BenchmarkDNSEncode/big_entries.txt-8               25502             46688 ns/op              2262 bytes            7756 B/op        102 allocs/op
BenchmarkDNSV2Decode
BenchmarkDNSV2Decode/samples.txt
BenchmarkDNSV2Decode/samples.txt-8                  6728            177686 ns/op             502 B/op        124 allocs/op
BenchmarkDNSV2Decode/big_ips.txt
BenchmarkDNSV2Decode/big_ips.txt-8                 35605             34115 ns/op              57 B/op         13 allocs/op
BenchmarkDNSV2Decode/big_entries.txt
BenchmarkDNSV2Decode/big_entries.txt-8             35160             34007 ns/op             113 B/op         27 allocs/op
BenchmarkDNSV2Encode
BenchmarkDNSV2Encode/samples.txt
BenchmarkDNSV2Encode/samples.txt-8                  3820            310290 ns/op             17169 bytes           62712 B/op       1086 allocs/op
BenchmarkDNSV2Encode/big_ips.txt
BenchmarkDNSV2Encode/big_ips.txt-8                 38322             30682 ns/op              2131 bytes            8496 B/op        111 allocs/op
BenchmarkDNSV2Encode/big_entries.txt
BenchmarkDNSV2Encode/big_entries.txt-8             36840             32334 ns/op              1961 bytes            7488 B/op        102 allocs/op
BenchmarkTagEncode
BenchmarkTagEncode-8                               30900             39176 ns/op           66913 B/op         17 allocs/op
BenchmarkTagEncoders
BenchmarkTagEncoders/low_dups_v2
BenchmarkTagEncoders/low_dups_v2-8                178010              6279 ns/op              2253 bytes            2372 B/op          2 allocs/op
BenchmarkTagEncoders/low_dups_v1
BenchmarkTagEncoders/low_dups_v1-8                301112              3943 ns/op              2072 bytes            6297 B/op          6 allocs/op
BenchmarkTagEncoders/high_dups_v2
BenchmarkTagEncoders/high_dups_v2-8                13999             85104 ns/op             26601 bytes           28388 B/op          7 allocs/op
BenchmarkTagEncoders/high_dups_v1
BenchmarkTagEncoders/high_dups_v1-8                 8462            143283 ns/op             51227 bytes          248987 B/op         22 allocs/op
BenchmarkTagEncoders/high_dups_2_v2
BenchmarkTagEncoders/high_dups_2_v2-8              29270             41123 ns/op             13022 bytes           13769 B/op          2 allocs/op
BenchmarkTagEncoders/high_dups_2_v1
BenchmarkTagEncoders/high_dups_2_v1-8              19120             65599 ns/op             28509 bytes          115290 B/op         18 allocs/op
BenchmarkTagEncoders/combined_v2
BenchmarkTagEncoders/combined_v2-8                  8128            138573 ns/op             41866 bytes           49321 B/op          2 allocs/op
BenchmarkTagEncoders/combined_v1
BenchmarkTagEncoders/combined_v1-8                  4926            221495 ns/op             81806 bytes          395037 B/op         28 allocs/op
PASS
ok      github.com/DataDog/agent-payload/v5/process     32.908s

In short:

  • the memory allocation looks pretty much the same with both versions.
  • as far as the CPU consumption is concerned, the order of magnitude looks similar and which version is faster depends on the use case.

@L3n41c
Copy link
Member Author

L3n41c commented Dec 23, 2022

Here are the changes needed on datadog-agent side to use this new version: DataDog/datadog-agent#14844.

@L3n41c L3n41c marked this pull request as ready for review December 23, 2022 15:20
@L3n41c L3n41c requested a review from a team as a code owner December 23, 2022 15:20
@L3n41c L3n41c marked this pull request as draft December 23, 2022 15:21
@L3n41c L3n41c force-pushed the lenaic/update_protoc branch 2 times, most recently from 61bd4d3 to 44e9184 Compare December 23, 2022 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant