prost icon indicating copy to clipboard operation
prost copied to clipboard

Optimization opportunity: `encoded_len` gets computed twice when encoding a message

Open ivoanjo opened this issue 1 year ago • 8 comments

Hey :wave:! I work at Datadog and we use prost in our libdatadog rust component.

While looking at some performance profiling of our protobuf encoding, I noticed that there's quite a bit of repeated work between encode and encode_raw since both need encoded_len, and it gets computed twice:

image

Specifically, Message::encode computes the encoded_len to check if the buffer has enough space, and then throws this info away, and then encode_raw gets the number again as it needs to record the field size in the pprof.

Computing encoded_len is around 5% of the time of this benchmark, so there would be a nice gain in encoding speed if it was computed only once.

We're happy users of prost so I decided to report this in the spirit of "see something, say something".

Thanks again for your amazing work so far :pray:

ivoanjo avatar Jul 30 '24 11:07 ivoanjo

Thanks, that is great that you found this. Do you have a suggestion on how to fix this? Are you willing to create a PR for that?

caspermeijn avatar Jul 31 '24 06:07 caspermeijn

Thanks, that is great that you found this. Do you have a suggestion on how to fix this?

I would probably go for either introducing a encode_raw_with_len that receives the len as an argument or modifying the existing encode_raw to always then the len as an argument (I'm not sure where else it gets used).

Are you willing to create a PR for that?

The million dollar question :sweat_smile:. I'm working on a few other bigger optimizations on our codebase first, and I'm not sure when I'll have enough time to circle back to this one.

So I'll keep it on my radar, but it may take a while -- this is why I decided to report, so it wasn't forgotten/and in case someone wanted to pick it up faster than I can :)

ivoanjo avatar Jul 31 '24 07:07 ivoanjo

Hey guys,

I am willing to try to solve this problem :)

Athishpranav2003 avatar Jun 13 '25 05:06 Athishpranav2003

That would be really cool!

ivoanjo avatar Jun 13 '25 07:06 ivoanjo

@ivoanjo can u share the scripts u used to do the benchmarking

Would be really nice if I got to see the backtrace and try running stuff in gdb

Athishpranav2003 avatar Jun 13 '25 08:06 Athishpranav2003

Unfortunately I didn't keep most of the stuff as I was doing a bunch of experiments at the time. But I believe this will happen with any protobuf encoding, so setting up a simple message (with some amount of nested fields) and repeatedly encoding it should reveal the issue.

ivoanjo avatar Jun 13 '25 09:06 ivoanjo

For profiling which tool do u use @ivoanjo

Athishpranav2003 avatar Jun 13 '25 09:06 Athishpranav2003

The screenshot above was from the beta datadog native profiler. Since it's in beta, it's still free to use ;)

The underlying data source is linux perf, so using perf directly or any profiler that gets data from it should get you the same results.

ivoanjo avatar Jun 13 '25 10:06 ivoanjo