p4lang / p4runtime

Specification documents for the P4Runtime control-plane API
Apache License 2.0
142 stars 88 forks source link

Enabling arenas in runtime proto #348

Closed mkruskal-google closed 3 years ago

mkruskal-google commented 3 years ago

This will be enabled by default in future versions, and is necessary to generate protobuf arena methods.

mkruskal-google commented 3 years ago

Looking at when this was flipped on by default back in early 2020, it looks like the main drawback is that the binary size increases a bit. This should be marginal though, and this flag will be deprecated and defaulted to true in future protobuf versions.

Since you aren't using arenas, it basically just adds some dead generated code.

mkruskal-google commented 3 years ago

Are there any outstanding issues? I don't seem to have permission to merge

antoninbas commented 3 years ago

@mkruskal-google, no I was giving other folks a chance to review. I think we can proceed with the merge.

mkruskal-google commented 3 years ago

Great, thanks Antonin!

On Wed, Apr 21, 2021 at 5:20 PM Antonin Bas @.***> wrote:

Merged #348 https://github.com/p4lang/p4runtime/pull/348 into main.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/p4lang/p4runtime/pull/348#event-4627846841, or unsubscribe https://github.com/notifications/unsubscribe-auth/AO6CNUZ3HGJVOFVM52DWCCTTJ5TWLANCNFSM43JGRPDQ .