golang / protobuf

Go support for Google's protocol buffers
BSD 3-Clause "New" or "Revised" License
9.64k stars 1.58k forks source link

benchmarks: main protobuf repo no longer contains benchmark protos #1524

Open jhump opened 1 year ago

jhump commented 1 year ago

As of https://github.com/protocolbuffers/protobuf/commit/83c499de86224538e5d59adc3d0fa7fdb45b2c72, the main protobuf repo no longer contains the benchmark protos used by protocolbuffers-go repo.

When updating to latest version (https://github.com/protocolbuffers/protobuf-go/commit/bc1253ad37431ee26876db47cd8207cdec81993c#), the benchmark protos had to be left untouched.

The benchmarks for Go either need to be reconciled with the new benchmarks in fleetbench (which is apparently what the main protobuf repo now uses), or it needs to define the benchmark proto sources it needs locally (instead of expecting them to be in the main protobuf repo).

aktau commented 1 year ago

... the benchmark protos had to be left untouched.

Can you explain this? From what I understand the benchmark protos are (were) on the main protobuf repo. Are you saying this commit should've included a copy of the now deleted benchmark protos?

Also, I recently ran ./regenerate.bash and ./test.bash and didn't see any complaints. How do you run the benchmarks?

jhump commented 1 year ago

@aktau, the first link in the description is the commit where they were removed from the main repo. The comment there is that they have been superceded by fleetbench.

The scripts run without complaining because I commented out the code that was expecting them to be in the main repo. That's in the second link -- specifically this part. The benchmarks still successfully run, but they are using an older version of the benchmark protos, the one from Protobuf v21.5 (which is from the last time this repo was updated from a version of the main repo that still included those protos).

So the gist of this issue is that the commented out code should be removed and instead this repo should probably pull benchmark protos from fleetbench and generate Go code for those. And then the actual benchmark tests will need to be updated to reference those protos (which are different than the old benchmark protos that were in the main repo). OR, an alternative, would be to copy those benchmark proto sources into this repo and let this repo be their new home (instead of expecting them to be defined in the main repo).

I hope that makes sense.