[PATCH v2] mempool: micro-optimize put function
Konstantin Ananyev
konstantin.ananyev at huawei.com
Fri Dec 23 17:34:09 CET 2022
> > 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.ananyev at huawei.com>
More information about the dev
mailing list