> > Hi Morten, > > > > > PING mempool maintainers. > > > > > > If you don't provide ACK or Review to patches, how should Thomas know > > that it is ready for merging? > > > > > > -Morten > > > > The code changes itself looks ok to me. > > Though I don't think it would really bring any noticeable speedup. > > But yes, it looks a bit nicer this way, especially with extra comments. > > Agree, removing the compare and branch instructions from the likely code path > provides no noticeable speedup, but makes the code > more clean. > > > One question though, why do you feel this assert: > > RTE_ASSERT(cache->len <= cache->flushthresh); > > is necessary? > > I could have written it into the comment above it, but instead chose to add > the assertion as an improved comment. > > > I can't see any way it could happen, unless something is totally broken > > within the app. > > Agree. These are the circumstances where assertions can come into action. > With more assertions in the code, it is likely that less code > executes before hitting an assertion, making it easier to find the root cause > of such errors. > > Please note that RTE_ASSERTs are omitted unless built with RTE_ENABLE_ASSERT, > so this assertion is omitted and thus has no > performance impact for a normal build.
I am aware that RTE_ASSERT is not enabled by default. My question was more about - why do you feel assert() is necessary in that case in particular. Does it guard for some specific scenario or so. Anyway, I am ok either way: with and without assert() here. Acked-by: Konstantin Ananyev <konstantin.anan...@huawei.com>
