On 5/29/25 11:37 PM, Dmitry Porokh wrote: > > Ilya Maximets <i.maxim...@ovn.org> writes: > >> External email: Use caution opening links or attachments >> >> >> On 5/13/25 7:23 PM, Dmitry Porokh via dev wrote: >>> The idea of this change is to reduce memory allocations/deallocations >>> for constant json objects like "uuid", "named_uuid", "map", etc. >> >> Hi, Dmitry. Thanks for the patch and sorry for delay. >> >> Do you have any performance numbers for this change to show the improvement? > > I don't believe that performance improvement from this change can be > really measured on full system. But probably I can do some > microbenchmark that shows that statics are better and adding additional > condition in if in deallocation can be ignored.
OK, I was just curious if you saw a noticeable performance improvement with this change, since I haven't in my tests. It may be worth mentioning in the commit message. > > Also, protect this change a bit I would ask: "Would you try to remove it if > it already had been there?". In addition, IMO constants in one place is > better than constants scattered across the code. Sure, I do think that it's a reasonable change overall and, as I said below, I even tried the very similar approach myself some time ago. So, I think, it's fine to get it accepted. I'll post some technical comments for the patch. Best regards, Ilya Maximets. > > But if your opinion is different I can just give up. Not fight worth having. > >> I did actually try a very similar approach last year and didn't see any real >> performance improvement, unfortunately, in neither real-world OVN databases >> nor synthetic ones. Tried with databases from ovn-kubernetes setups and with >> ovn-heater. >> >> One thing that may have skewed my testing is that I tried this optimization >> on top of inlining the short strings and arrays into the json struct: >> >> https://github.com/igsilya/ovs/commit/5f545e6a0b6cdfed40798efca054e67dafdd66b2 >> (I need to clean this up and post someday.) >> but I'm not sure. The impact of these pre-defined short strings seems >> negligible in comparison with the rest of memory allocations we do for json >> objects. >> >> Best regards, Ilya Maximets. > > Thanks, > Dmitry > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
