[RFC] Performance improvements
jra at samba.org
Wed Jun 20 22:09:21 UTC 2018
On Wed, Jun 20, 2018 at 10:51:58PM +0200, Swen Schillig wrote:
> ... and each packet is handled by an individually triggered callback.
OK, if this is the case then it's OK. I'm up
at the Redmond plugfest right now with limited
> > if part of the event processing schedules an
> > immediate event (or socket activation etc.) then you can
> > accidently trigger out-of-order event handling, which can
> > lead to horrible to find bugs.
> ...and there is NO out of order handling.
If you say so :-).
> It is totally transparent to tevent....one event ... one invocation,
> no messing with multiple events in one go.
Again, if you say so :-).
> > hat has great potential
> > to cause future bugs, so on those grounds I would NAK
> > this.
> I think you're a bit too quick with your judgement, is there
> a test-case which shows/proves the issue you saw in the past ?
No, no test case - just experience with tevent.
So long as you're not processing multiple events from
one callback you're OK. As soon as you do, you will
*certainly* get bugs.
I don't need a test case to prove this any more than
I need a test case to know that unchecked arithmetic
on network packets can lead to integer wrap security
bugs, or talloc_reference() leads to memory graphs
instead of trees.
Some things are so bad you just have to avoid them,
they'll mess you up good :-).
> well, since it's the "last" patch out of the 10 I posted,
> I guess it is not so difficult to have a start without it.
> ...I think I could separate out just this part from the last patch and
> keep the other updates again in an individual patch.
> Even though I cannot quantify the improvement of that patch I would
> still believe that it has its share on the reduction of the variation.
> Therefore, I would really like you to have a look at the code again and
> "verify" if the changes are as evil as you claim.
You do have some integer wrap and variable type
work to do first - so why not do that and then resubmit
with the "questionable" patch as the last entry
so it can be reviewed carefully individually ?
More information about the samba-technical