> > > Hi Dharmik, > > > > > >>> > > >>>> Current mempool per core cache implementation stores pointers to mbufs > > >>>> On 64b architectures, each pointer consumes 8B > > >>>> This patch replaces it with index-based implementation, > > >>>> where in each buffer is addressed by (pool base address + index) > > >>>> It reduces the amount of memory/cache required for per core cache > > >>>> > > >>>> L3Fwd performance testing reveals minor improvements in the cache > > >>>> performance (L1 and L2 misses reduced by 0.60%) > > >>>> with no change in throughput > > >>> > > >>> I feel really sceptical about that patch and the whole idea in general: > > >>> - From what I read above there is no real performance improvement > > >>> observed. > > >>> (In fact on my IA boxes mempool_perf_autotest reports ~20% slowdown, > > >>> see below for more details). > > >> > > >> Currently, the optimizations (loop unroll and vectorization) are only > > >> implemented for ARM64. > > >> Similar optimizations can be implemented for x86 platforms which should > > >> close the performance gap > > >> and in my understanding should give better performance for a bulk size > > >> of 32. > > > > > > Might be, but I still don't see the reason for such effort. > > > As you mentioned there is no performance improvement in 'real' apps: > > > l3fwd, etc. > > > on ARM64 even with vectorized version of the code. > > > > > > > IMO, even without performance improvement, it is advantageous because the > > same performance is being achieved > > with less memory and cache utilization using the patch. > > > > >>> - Space utilization difference looks neglectable too. > > >> > > >> Sorry, I did not understand this point. > > > > > > As I understand one of the expectations from that patch was: > > > reduce memory/cache required, which should improve cache utilization > > > (less misses, etc.). > > > Though I think such improvements would be neglectable and wouldn't > > > cause any real performance gain. > > > > The cache utilization performance numbers are for the l3fwd app, which > > might not be bottlenecked at the mempool per core cache. > > Theoretically, this patch enables storing twice the number of objects in > > the cache as compared to the original implementation. > > It saves you 4 just bytes per mbuf. > Even for simple l2fwd-like workload we access ~100 bytes per mbuf. > Let's do a simplistic estimation of number of affected cache-lines l for > l2fwd. > For bulk of 32 packets, assuming 64B per cache-line and 16B per HW desc: > > number > of cache-lines accessed > cache with > pointers / cache with indexes > mempool_get: (32*8)/64=4 > / (32*4)/64=2 > RX (read HW desc): (32*16)/64=8 / > (32*16)/64=8 > RX (write mbuf fields, 1st cache line): (32*64)/64=3 / > (32*64)/64=32
Should be: RX (write mbuf fields, 1st cache line): (32*64)/64=32 / (32*64)/64=32 off course > update mac addrs: (32*64)/64=32 / > (32*64)/64=32 > TX (write HW desc): (32*16)/64=8 / > (32*16)/64=8 > free mbufs (read 2nd mbuf cache line): (32*64)/64=32 / (32*64)/64=32 > mempool_put: (32*8)/64=4 / > (32*4)/64=2 > total: 120 > 116 > > So, if my calculations are correct, max estimated gain for cache utilization > would be: > (120-116)*100/120=3.33% > Note that numbers are for over-simplistic usage scenario. > In more realistic ones, when we have to touch more cache-lines per packet, > that difference would be even less noticeable. > So I really doubt we will see some noticeable improvements in terms of cache > utilization > with that patch. > > > > > > >>> - The change introduces a new build time config option with a major > > >>> limitation: > > >>> All memzones in a pool have to be within the same 4GB boundary. > > >>> To address it properly, extra changes will be required in > > >>> init(/populate) part of the code. > > >> > > >> I agree to the above mentioned challenges and I am currently working on > > >> resolving these issues. > > > > > > I still think that to justify such changes some really noticeable > > > performance > > > improvement needs to be demonstrated: double-digit speedup for > > > l3fwd/ipsec-secgw/... > > > Otherwise it just not worth the hassle. > > > > > > > Like I mentioned earlier, the app might not be bottlenecked at the mempool > > per core cache. > > That could be the reason the numbers with l3fwd don’t fully show the > > advantage of the patch. > > As I said above, I don’t think we'll see any real advantage here. > But feel free to pick-up different app and prove me wrong. > After all we have plenty of sample apps that do provide enough > pressure on the cache: l3fwd-acl, ipsec-secgw. > Or you can even apply these patches from Sean: > https://patches.dpdk.org/project/dpdk/list/?series=20999 > to run l3fwd with configurable routes. > That should help you to make it cache-bound. > > > I’m seeing double-digit improvement with mempool_perf_autotest which should > > not be ignored. > > And for other we are seeing double digit degradation. > So far the whole idea doesn't look promising at all, at least to me. > Konstantin
