30/03/2025 10:09, Andrew Rybchenko: > On 3/27/25 20:15, Bruce Richardson wrote: > > On Wed, Feb 26, 2025 at 03:59:22PM +0000, Morten Brørup wrote: > >> The comparisons lcore_id < RTE_MAX_LCORE and lcore_id != LCORE_ID_ANY are > >> equivalent, but the latter compiles to fewer bytes of code space. > >> Similarly for lcore_id >= RTE_MAX_LCORE and lcore_id == LCORE_ID_ANY. > >> > >> The rte_mempool_get_ops() function is also used in the fast path, so > >> RTE_VERIFY() was replaced by RTE_ASSERT(). > >> > >> Compilers implicitly consider comparisons of variable == 0 likely, so > >> unlikely() was added to the check for no mempool cache (mp->cache_size == > >> 0) in the rte_mempool_default_cache() function. > >> > >> The rte_mempool_do_generic_put() function for adding objects to a mempool > >> was refactored as follows: > >> - The comparison for the request itself being too big, which is considered > >> unlikely, was moved down and out of the code path where the cache has > >> sufficient room for the added objects, which is considered the most > >> likely code path. > >> - Added __rte_assume() about the cache length, size and threshold, for > >> compiler optimization when "n" is compile time constant. > >> - Added __rte_assume() about "ret" being zero, so other functions using > >> the value returned by this function can be potentially optimized by the > >> compiler; especially when it merges multiple sequential code paths of > >> inlined code depending on the return value being either zero or > >> negative. > >> - The refactored source code (with comments) made the separate comment > >> describing the cache flush/add algorithm superfluous, so it was removed. > >> > >> A few more likely()/unlikely() were added. > > > > In general not a big fan of using likely/unlikely, but if they give a perf > > benefit, we should probably take them. > > > > Few more comments inline below. > > > >> A few comments were improved for readability. > >> > >> Some assertions, RTE_ASSERT(), were added. Most importantly to assert that > >> the return values of the mempool drivers' enqueue and dequeue operations > >> are API compliant, i.e. 0 (for success) or negative (for failure), and > >> never positive. > >> > >> Signed-off-by: Morten Brørup <m...@smartsharesystems.com> > > > > Acked-by: Bruce Richardson <bruce.richard...@intel.com> > > Acked-by: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
Applied, thanks.
