Opened 4 years ago

Closed 4 years ago

#31680 closed Cleanup/optimization (fixed)

Preventing DeferredAttribute.__ get__() unnecessary calls.

Reported by: Sultan Owned by: Sultan
Component: Database layer (models, ORM) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description (last modified by Sultan)

To retrieve a deferred model attributes, the get method is called twice. This is because it uses the getattr() function, which in turn causes the get method to be called again.

    def __get__(self, instance, cls=None):
        """
        Retrieve and caches the value from the datastore on the first lookup.
        Return the cached value.
        """
        if instance is None:
            return self
        data = instance.__dict__
        field_name = self.field.attname
        if field_name not in data:
            # Let's see if the field is part of the parent chain. If so we
            # might be able to reuse the already loaded value. Refs #18343.
            val = self._check_parent_chain(instance)
            if val is None:
                instance.refresh_from_db(fields=[field_name])
                **val = getattr(instance, field_name)**
            data[field_name] = val
        return data[field_name]

To prevent this unnecessary call, we can simply extract the value from the instance dict (at that moment it already contains the reloaded value):

    def __get__(self, instance, cls=None):
        """
        Retrieve and caches the value from the datastore on the first lookup.
        Return the cached value.
        """
        if instance is None:
            return self
        data = instance.__dict__
        field_name = self.field.attname
        if field_name not in data:
            # Let's see if the field is part of the parent chain. If so we
            # might be able to reuse the already loaded value. Refs #18343.
            val = self._check_parent_chain(instance)
            if val is None:
                instance.refresh_from_db(fields=[field_name])
                # Now the instance dict contains the reloaded data.
                # Using getattr() will do extra call to __get__ method.
                **val = data[field_name]**
            data[field_name] = val
        return data[field_name]

This reduces the number of method calls.

Change History (6)

comment:1 by Sultan, 4 years ago

Easy pickings: set
Has patch: set

comment:2 by Sultan, 4 years ago

Description: modified (diff)

comment:3 by Mariusz Felisiak, 4 years ago

Owner: changed from nobody to Sultan
Status: newassigned
Summary: Preventing DeferredAttribute .__ get__ method unnecessary callsPreventing DeferredAttribute .__ get__ method unnecessary calls.
Triage Stage: UnreviewedAccepted
Version: 3.1master

It looks that val = getattr(instance, field_name) is redundant.

comment:4 by Mariusz Felisiak, 4 years ago

Summary: Preventing DeferredAttribute .__ get__ method unnecessary calls.Preventing DeferredAttribute.__ get__() unnecessary calls.

in reply to:  4 comment:5 by Sultan, 4 years ago

Replying to felixxm: Yes you are right. I changed the code. Now it has become much easier and better. Thank you.

    def __get__(self, instance, cls=None):
        """
        Retrieve and caches the value from the datastore on the first lookup.
        Return the cached value.
        """
        if instance is None:
            return self
        data = instance.__dict__
        field_name = self.field.attname
        if field_name not in data:
            # Let's see if the field is part of the parent chain. If so we
            # might be able to reuse the already loaded value. Refs #18343.
            val = self._check_parent_chain(instance)
            if val is None:
                instance.refresh_from_db(fields=[field_name])
            else:
                data[field_name] = val
        return data[field_name]

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

Resolution: fixed
Status: assignedclosed

In 678c8df:

Fixed #31680 -- Removed unnecessary getattr() call in DeferredAttribute.get().

refresh_from_db() loads fields values.

Note: See TracTickets for help on using tickets.
Back to Top