>> >> I did some testing with my patch and these are the findings >> >> | Avg. Poll Intervals | Total >> test time | northd RSS >> ------------------------------ +------------------------+--------------- >> Last week | 1.5 seconds | 1005 seconds >> | 2.5 GB >> This week | 6 seconds | 2246 seconds >> | 8.5 GB >> Ilya's Patch | 2.5 seconds | 1170 seconds >> | 3.1 GB >> Numan's patch (run1) | 1.6 seconds | 992 seconds | 2.43 GB >> Numan's patch (run2) | 1.7 seconds | 1022 seconds | 2.43 GB >> >> Seems like removing dp ref cnt all together is more efficient and very close >> to >> last week's results. >> >> I'll submit the patch tomorrow after some more testing. >> >> Feel free to provide any review comments or feedback if you've any. >> >> Thanks >> Numan >> >> > > >> > > Regarding the cluster density 500 node test with Ilya's fix, I think >> > > this seems o.k to me since >> > > the test falls back to recompute a lot and there is some cost now with >> > > the lflow I-P patches. >> > > This can be improved by adding port group I-P. I can start looking >> > > into the port-group I-P if you >> > > think it would be worth it. Let me know. >> > > >> > >> > Adding more I-P would be helpful, but it still doesn't explain the >> > observation from Ilya regarding the 66% increase in recompute time and 25% >> > increase in memory. My test cases couldn't reproduce such significant >> > increases (it is ~10-20% in my test). I think we'd better figure out what >> > caused such big increases - is it still related to ref count or is it >> > something else. I think the lflow_ref might have contributed to it, >> > especially the use of hmap instead of list. It is totally fine if the >> > lflow_ref adds some cost, which is required for the I-P to work, but it is >> > better to understand where the cost comes from and justify it (or optimize >> > it). >> >> Looks like from my patch results, dp_ref_cnt is contributing to this >> increase. >> Is it possible for you to trigger another test with my patch - >> https://github.com/numansiddique/ovn/commit/92f6563d9e7b1e6c8f2b924dea7a220781e10a05 >> >> <https://github.com/numansiddique/ovn/commit/92f6563d9e7b1e6c8f2b924dea7a220781e10a05> >> > It is great to see that removing ref count resolved the performance gap. The > test result is actually very interesting. I also had a test with more LBs > (10k instead of the earlier 1k) in the large LBG, trying to make the cost of > bitmap scan more obvious, but still, the difference between your patch and > Ilya's patch is not significant in my test. The latency difference is within > 5%, and the memory <1%. So I wonder why in the ovn-heater test it is so > different. Is it possible that in the ovn-heater test it does create a lot of > duplicated lflows so ref count hmap operations are actually triggered for a > significant amount of times, which might have contributed to the major cost > even with Ilya's patch, and with your patch it means it fall back to > recompute much more often?
FWIW, in my testing with the cluster-density 500node database there are 12 M refcounts allocated. With 32 bytes each, it's about 370 MB of RAM. We should also add a fair share of allocation overhead since we're allocating a huge number of very small objects. Numan also gets rid of all the space allocated for hash maps that hold all these refcounts, and these were taking ~25% of all allocations. Getting rid of these allocations likely saves a lot of CPU cycles as well, since they are very heavy. P.S. Almost all of these 12M refcounts are for: build_lrouter_defrag_flows_for_lb():ovn_lflow_add_with_dp_group() All of these have refcount == 3. There is one with refcount 502: build_egress_delivery_flows_for_lrouter_port():ovn_lflow_add_default_drop() And there are 500-ish with refcount of 2: build_lswitch_rport_arp_req_flow():ovn_lflow_add_with_hint() Best regards, Ilya Maximets. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
