Re: [PATCH] mempool: micro optimizations

[Thomas Monjalon <thomas@monjalon.net>]

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 <mb@smartsharesystems.com>
> > 
> > Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>

Applied, thanks.