Skip to content
This repository has been archived by the owner on Jun 1, 2022. It is now read-only.

Terrible performance on the county history diff page #225

Closed
simonw opened this issue Apr 9, 2021 · 4 comments
Closed

Terrible performance on the county history diff page #225

simonw opened this issue Apr 9, 2021 · 4 comments
Labels
code cleanup Refactoring and other code maintenance help wanted Extra attention is needed performance Site and API performance optimizations wontfix This will not be worked on

Comments

@simonw
Copy link
Collaborator

simonw commented Apr 9, 2021

Spotted this while working on #218:

Compare_Kern___VIAL_admin

We don't want to track the locations that belong to the county every time we edit the county - it will store a whole bunch of pointless data in our revision tables, plus it has a significant performance impact on the pages that show the differences between different county versions.

@simonw simonw added bug Something isn't working performance Site and API performance optimizations labels Apr 9, 2021
@simonw
Copy link
Collaborator Author

simonw commented Apr 9, 2021

There's a follow= option in the registration API which should be able to fix this: https://django-reversion.readthedocs.io/en/latest/api.html#registration-api

@simonw
Copy link
Collaborator Author

simonw commented Apr 9, 2021

Here's how to customize that when registering reversion using the VersionAdmin mixin: https://django-reversion.readthedocs.io/en/stable/admin.html#versionadmin-register

@simonw
Copy link
Collaborator Author

simonw commented Apr 9, 2021

It looks like I misunderstood this: from digging into the database it doesn't look like those location lists are being included in the reversion serialized JSON records.

Instead this looks to be a performance issue with django-reversion-compare:

Compare_Kern____VIAL_admin

We can address this some point in the future.

@simonw simonw added this to the Post-California launch milestone Apr 9, 2021
@simonw simonw added code cleanup Refactoring and other code maintenance help wanted Extra attention is needed and removed bug Something isn't working labels Apr 9, 2021
@simonw simonw changed the title django-reversion is pointlessly tracking lists of locations against counties Terrible performance on the county history diff page Apr 9, 2021
@simonw
Copy link
Collaborator Author

simonw commented Apr 9, 2021

Relevant issue: jedie/django-reversion-compare#95

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
code cleanup Refactoring and other code maintenance help wanted Extra attention is needed performance Site and API performance optimizations wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants