Opened 2 years ago

Last modified 2 years ago

#9680 defect new

twisted.logger.Logger creates too many new instances when declared within a class

Reported by: L. Daniel Burr Owned by: L. Daniel Burr
Priority: normal Milestone:
Component: core Keywords:
Cc: Branch:
Author:

Description (last modified by L. Daniel Burr)

Situation
In https://github.com/twisted/twisted/blob/trunk/src/twisted/logger/_logger.py#L100 a new instance of the Logger is created each and every time it is accessed as a descriptor. This can result in a lot of unnecessary object creation.

Target
When a Logger instance is created within a class declaration, all subsequent attribute accesses should return the same Logger instance.

Proposal
As a first step, stop creating a new instance of the Logger, and instead set the namespace, source, and observer attributes on the existing Logger instance. This will be sufficient for purposes of stemming the object creation tide, but will still be doing pointless work, in that the namespace, source, and observer never change.

As a second step, add an internal flag to the Logger class, which the __get__ method can check to see if it has already configured the Logger instance.

Lastly, adjust the private _loggerFor function to not rely on Logger.__get__ to create a Logger instance.

Change History (11)

comment:1 Changed 2 years ago by L. Daniel Burr

Description: modified (diff)

comment:2 Changed 2 years ago by L. Daniel Burr

Description: modified (diff)

comment:3 Changed 2 years ago by L. Daniel Burr

Description: modified (diff)

comment:4 Changed 2 years ago by L. Daniel Burr

Description: modified (diff)

comment:5 Changed 2 years ago by Glyph

Isn't log_source set to the instance every time? We could still cache the Logger object on the instance if that's faster (it does save an allocation and creates less garbage).

comment:6 in reply to:  5 Changed 2 years ago by L. Daniel Burr

Replying to Glyph:

Isn't log_source set to the instance every time? We could still cache the Logger object on the instance if that's faster (it does save an allocation and creates less garbage).

log_source is set to self.source at https://github.com/twisted/twisted/blob/trunk/src/twisted/logger/_logger.py#L138. For the case where an instance of Logger is an attribute of a class, your statement should be correct, since the call to __get__ will have occurred prior to the call to emit.

I think the right first step is to do something like this:

def __get__(self, oself, type=None):
    if oself is None:
        source = type
    else:
        source = oself

    self.namespace = ".".join([type.__module__, type.__name__])
    self.source = source
    return self

And then adjust _loggerFor like so:

def _loggerFor(obj):
    cls = obj.__class__
    n = ".".join([cls.__module__, cls.__name__])
    return Logger(namespace=n, observer=None, source=obj)

Thoughts?

comment:7 Changed 2 years ago by L. Daniel Burr

comment:8 Changed 2 years ago by L. Daniel Burr

Keywords: review added
Owner: set to Glyph
Type: enhancementdefect

comment:9 Changed 2 years ago by L. Daniel Burr

Owner: Glyph deleted

comment:10 Changed 2 years ago by Glyph

Keywords: review removed

comment:11 Changed 2 years ago by Glyph

Owner: set to L. Daniel Burr
Note: See TracTickets for help on using tickets.