Skip to content

Conversation

kezabelle
Copy link
Contributor

Ticket is 33124, following on from ticket 33025 which was PR 14769.

  • Repeated access to connections[alias] should be cached into a local variable wherever possible to avoid asking the Local for the value twice.
  • Asking for connections[alias] a single time should be deferred until absolutely necessary (ie: local to the if branch, or as the last part of an if chained comparison, etc)

@felixxm
Copy link
Member

felixxm commented Sep 24, 2021

@kezabelle These changes looks good, thanks 👍 Is there anything left for this patch? If not, I can squash & merge.

@kezabelle
Copy link
Contributor Author

I think this was all of the ones (outside of tests) which made sense, except for the change which Nick pointed out was problematic.

I think there's a couple of others that could be done, like eg: django.contrib.sites.management.create_default_site but they are "one and done" operations, which won't net any positive change and just muddy the git annotation history...

I can do another pass to find and do those if you'd prefer, though.

@felixxm
Copy link
Member

felixxm commented Sep 24, 2021

I can do another pass to find and do those if you'd prefer, though.

No need, thanks 👍

@felixxm felixxm marked this pull request as ready for review September 24, 2021 10:15
…ecessary.

Follow up to bf5abf1.

This also caches the __getitem__ access.
@felixxm felixxm changed the title Refs #33124 -- Avoided touching ConnectionsHandler.__getitem__ until strictly necessary. Fixed #33124 -- Avoided accessing the database connections when not necessary. Sep 24, 2021
@felixxm felixxm merged commit 06c50ce into django:main Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants