[Twisted-Python] log.callWithLogger not used - slows down reactor?

[Glyph]

> On Jun 3, 2020, at 2:51 AM, Barry Scott <barry.scott at forcepoint.com> wrote:
> 
> On Wednesday, 3 June 2020 08:07:37 BST Glyph wrote:
>>> On Jun 2, 2020, at 4:54 AM, Barry Scott <barry.scott at forcepoint.com>
>>> wrote:
>>> 
>>> I'm hunting down performance issue in our code and spotted this in
>>> passing. As far I can tell nothing seems to need callWithLogger.
>>> 
>>> I ran our 6k+ tests with this patch applied and everything worked.
>>> 
>>> Does anything in the twisted world need it?
>>> 
>>> Barry
>>> 
>>> diff --git a/src/twisted/internet/pollreactor.py b/src/twisted/internet/
>>> pollreactor.py
>>> index 6db1660b9..6901e5c95 100644
>>> --- a/src/twisted/internet/pollreactor.py
>>> +++ b/src/twisted/internet/pollreactor.py
>>> @@ -165,7 +165,7 @@ class PollReactor(posixbase.PosixReactorBase,
>>> 
>>> posixbase._PollLikeMixin):
>>>                # Handles the infrequent case where one selectable's
>>>                # handler disconnects another.
>>>                continue
>>> 
>>> -            log.callWithLogger(selectable, _drdw, selectable, fd, event)
>>> +            _drdw(selectable, fd, event)
>>> 
>>>    doIteration = doPoll
>> 
>> I think that we may have eliminated all the dependency on it.  Do your logs
>> look any different with this change applied?  If you can demonstrate its
>> impact on speed.twistedmatrix.com <http://speed.twistedmatrix.com/> maybe
>> we can go ahead and do this.  The logger used to be far more dependent on
>> this, but the "system" portion of the log message is now based more on who
>> is doing the logging than what socket provoked the event (which is what
>> this is tracking, effectively).
> 
> Curious, if its dead code why do you need a speed test? For your comment
> it seems you planned to stop using this.

Because we only may have eliminated the dependency on it :).  It's always safest not to change things, so it's good to have a clear repeatable demonstration of the benefit.  If you're sure that we have no dependencies on it, we can probably get rid of it without a super clear benchmark.

> What is it I would need to do for a speed test?

Commit some code to this repository: https://github.com/twisted-infra/twisted-benchmarks <https://github.com/twisted-infra/twisted-benchmarks>

I am not sure how to get speed.twistedmatrix.com <http://speed.twistedmatrix.com/> to run on a branch, but we can figure it out :)

-glyph
-------------- next part --------------
An HTML attachment was scrubbed...
URL: </pipermail/twisted-python/attachments/20200607/edbe135d/attachment.htm>