Migrate off of gogo/protobuf

[kzys]

gogo/protobuf has been deprecated since 2022. This PR uses Google's official code generators instead of gogo's.

Relates to #3119

[kzys]

The fsutil change is in tonistiigi/fsutil#171.

[tonistiigi]

Is there a guarantee that this does not break old-new client-daemon combinations?

[kzys]

Yes. gogo/protobuf didn't modify the wire protocol. Their changes are primarily around code generators to make it more Go-idiomatic and efficient. I did a similar refactorting for containerd (see containerd/containerd#6564), and it has been released as containerd 1.7.0. We haven't heard any complaints regarding the compatibility.

[thaJeztah]

Seeing some test failures; are those expected?

=== FAIL: solver/llbsolver TestRecomputeDigests (0.00s)
    vertex_test.go:54: 
        	Error Trace:	D:/a/buildkit/buildkit/solver/llbsolver/vertex_test.go:54
        	Error:      	Not equal: 
        	            	expected: digest.Digest("sha256:06c243a9083f537e33d82b0af0379737187f7fbba73f852fe968b592c36bd8b5")
        	            	actual  : string("sha256:06c243a9083f537e33d82b0af0379737187f7fbba73f852fe968b592c36bd8b5")

[kzys]

Seeing some test failures; are those expected?

No. It should be fixed in the last revision, but the PR is still work-in-progress...

[kzys]

By fixing all golangci-lint errors, I ended up getting failed to load cache key: docker.io/library/busybox:latest@sha256:b28d6f3d483bfa5b30562287e1197fca960ca5dd8211d8cbf1536eb937e21af8: not found which I couldn't figure out why. So I'm re-doing this PR again with much smaller commits.

[tonistiigi]

Do you have benchmarks for Buildkit itself?

I think for this we would do micro-benchmarks for marshaling, unmarshal. If we start running containers then this would skew the timing too much. In fsutil there was also a benchmark script that maybe can be used. Maybe LLB marshal and load is a nice candidate.

The biggest possible performance issue would be with the session endpoint and the context and build result transfer. This is where 100s of MB can potentially be transferred and encoded/decoded. If session uses gRPC dialer then it also uses protobuf for raw data framing. I believe previously some of these decoding could avoid allocating more memory because the Unmarshal code was reusing the buffer by resetting it with [:0]. For these big data transfers, we should avoid new memory allocations that grow with amount of transferred data.

[profnandaa]

Just wondering, coz of the movements in the codebase, we are bound to have a lot of merge conflict as we go. Is possible to split this into some self-contained chunks like perhaps sub-packages and chain the PRs?

[jsternberg]

I did a small subset of this change locally and added some benchmarks to some of the solver ops specifically and it seems like the google protobuf will definitely cause a regression.

benchmark                        old ns/op     new ns/op     delta
BenchmarkExecOp_Marshal-10       148           475           +220.46%
BenchmarkSourceOp_Marshal-10     234           608           +160.39%
BenchmarkFileOp_Marshal-10       143           475           +231.63%
BenchmarkMergeOp_Marshal-10      36.0          152           +320.64%
BenchmarkDiffOp_Marshal-10       31.2          152           +385.12%

It seems like there now exists vtprotobuf which I'll do a quick test of to see if that is a feasible alternative.

[jsternberg]

Some additional numbers comparing gogo protobuf with vtprotobuf:

benchmark                        old ns/op     new ns/op     delta
BenchmarkExecOp_Marshal-10       148           118           -20.59%
BenchmarkSourceOp_Marshal-10     234           128           -45.01%
BenchmarkFileOp_Marshal-10       143           100.0         -30.17%
BenchmarkMergeOp_Marshal-10      36.0          35.2          -2.25%
BenchmarkDiffOp_Marshal-10       31.2          27.2          -13.02%

Seems pretty easy to integrate so we might just be able to do that.

[stevvooe]

I'd favor moving off of gogo rather than introducing another dependency. We can deal with performance issues separately from the move off of gogo.

[kzys]

Superseded by #5342.