Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#31732 closed Cleanup/optimization (fixed)

Cache function signatures in django.utils.inspect.

Reported by: Adam Johnson Owned by: Tim Park
Component: Utilities Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I found a bit of a performance regression in Django 2.1 and 2.2, gone in 3.0. There, SQLCompiler.get_converters() ran func_supports_parameter() for every field converter

Unfortunately the information isn't cached and inspect.signature() isn't particularly fast.

Using py-spy on a client test suite, I found func_supports_parameter() taking 0.25% of the total run time. This isn't much absolutely, but it was reachable only through pathways using Query.get_aggregation() (2.1%) and Query.get_count() (0.63%). So from those paths, func_supports_parameter() took 0.25 / (2.1 + .63) = ~10% of the time to execute aggregates, for easily cached information.

Given that function objects are immutable, I think all the use of inspect.signature in django.utils.inspect could go through a wrapper that caches with a WeakKeyDictionary or functools.lru_cache to avoid re-inspecting the same signatures.

It doesn't look like there is any usage of those functions in hot paths in Django 3.0+ but it could be useful to have this protection for any future function argument deprecations.

Change History (9)

comment:1 by Mariusz Felisiak, 4 years ago

Summary: Cache function signaturesCache function signatures in django.utils.inspect.
Triage Stage: UnreviewedAccepted

comment:2 by Tim Park, 4 years ago

Owner: changed from nobody to Tim Park
Status: newassigned

comment:3 by Mariusz Felisiak, 4 years ago

Has patch: set

comment:4 by Mariusz Felisiak, 4 years ago

Triage Stage: AcceptedReady for checkin

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

Resolution: fixed
Status: assignedclosed

In d1409f51:

Fixed #31732 -- Cached callables signatures in django.utils.inspect methods.

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

In 56289803:

Refs #31732 -- Fixed django.utils.inspect caching for bound methods.

Thanks Alexandr Artemyev for the report, and Simon Charette for the
original patch.

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

In 15a85183:

[3.2.x] Refs #31732 -- Fixed django.utils.inspect caching for bound methods.

Thanks Alexandr Artemyev for the report, and Simon Charette for the
original patch.

Backport of 562898034f65e17bcdd2d951ac5236a1ec8ea690 from main

comment:8 by Mariusz Felisiak, 4 years ago

In ac72a216:

(The changeset message doesn't reference this ticket)

comment:9 by Mariusz Felisiak, 4 years ago

In 2420fd2:

(The changeset message doesn't reference this ticket)

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