osrg / gobgp

BGP implemented in the Go Programming Language
https://osrg.github.io/gobgp/
Apache License 2.0
3.59k stars 684 forks source link

improve MonitorTable performance #2519

Open bganne opened 2 years ago

bganne commented 2 years ago

GoBGP is a great project, very versatile, thanks for the hard work. While doing some performance profiling, I noticed that a lot of time was spent in the protobuf serialization/deserialization. When integrating GoBGP as a library and using the MonitorTable() API, this can account for up to 50% of the CPU usage when processing route updates. I prototyped a MonitorTableInternal() API (lacking a better name) to investigate whether it could be possible to avoid this cost here: https://github.com/bganne/gobgp/commit/c7fb637856c45e0a46b441dfc8030a3380f71993 What do you think? Is it worth pursuing?

fujita commented 2 years ago

Path structure includes some private data structures so we need a new Path struct for the API and some conversion. Also MonitorTable API was removed in 3.X versions; we also need several new structs to be exported.

bganne commented 2 years ago

Path structure includes some private data structures so we need a new Path struct for the API and some conversion.

Yes, that was basically what I started doing, define a new struct Path for API and export that. In my tests, this allowed me to process 30K routes/s vs 15K routes/s by avoiding protobuf marshalling/unmarshalling. I think not all fields are of equal interests so maybe we could start exporting a subset of those fields 1st and see which one are really useful based on feedback?

Also MonitorTable API was removed in 3.X versions; we also need several new structs to be exported.

Right, but as far as I can tell this could be done with WatchEvent() in a similar way.

I can give it a shot and propose a pull-request, what do you think?