Opened 10 years ago

Closed 10 years ago

#22867 closed Cleanup/optimization (fixed)

Pickling models is slow in alpha versions

Reported by: Alexander Schepanovski Owned by: nobody
Component: Utilities Version: dev
Severity: Normal Keywords:
Cc: loic84 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

When pickling Django models in alpha version get_version() creates a git subprocess, which is very slow. I suggest memoizing get_version().

Change History (12)

comment:1 by Alexander Schepanovski, 10 years ago

comment:2 by loic84, 10 years ago

Cc: loic84 added
Patch needs improvement: set
Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

+1 although I'd only wrap get_git_changeset with lru_cache.

Marking as a release blocker for 1.8 since this is a regression from #21430.

comment:3 by anonymous, 10 years ago

Moved @lru_cache() to get_git_changeset().

comment:4 by Tim Graham <timograham@…>, 10 years ago

Resolution: fixed
Status: newclosed

In 80f4487d17a0040e9be35e7ee6ac478bafe6504a:

Fixed #22867 -- Memoized django.utils.version.get_git_changeset().

This improves pickling speed in prelease versions of Django; refs #21430.

comment:5 by Aymeric Augustin <aymeric.augustin@…>, 10 years ago

In 01399fa0aaa817281c77bbb4b7d82c6c8487c916:

Revert "Fixed #22867 -- Memoized django.utils.version.get_git_changeset()."

This reverts commit 80f4487 temporarily, because that commit prevented
the djangoproject.com server from building the docs, because it still
uses Python 2.6.

comment:6 by Aymeric Augustin, 10 years ago

Resolution: fixed
Status: closednew

While this commit was correct, I had to revert it because it broke the docs builds, which are still running on Python 2.6.

We need to upgrade the djangoproject.com server before we can push this change. Sorry for the inconvenience.

comment:7 by loic84, 10 years ago

Upgrading Python 2.6 which is EOL and recommitting this fix is the ideal course of action.

However if we don't manage to do so within a few of days, I'd suggest we go with a stopgap solution like https://gist.github.com/313430836d20006916c4f.

The regression could be quite severe to anyone caching large querysets since we'd be starting a process for each model.

Version 0, edited 10 years ago by loic84 (next)

comment:8 by Loic Bistuer <loic.bistuer@…>, 10 years ago

Resolution: fixed
Status: newclosed

In f07735c619c121d8785082972acd8fbad8b236af:

Fixed #22867 -- Memoized django.utils.version.get_git_changeset().

This follows commits 80f4487 and 01399fa; original patch had to be
reverted because it wasn't Python 2.6 compatible and we need it to
be in order to build docs on the djangoproject.com server.

This fix should be replaced by @lru_cache as soon as we drop
Python 2.6 compatibility.

Thanks Florian Apolloner for the review and Alexander Schepanovski
for the original patch.

comment:9 by loic84, 10 years ago

Easy pickings: unset
Patch needs improvement: unset
Severity: Release blockerNormal

Reopening as a "Cleanup/optimization" so we don't lose track of replacing this temporary fix with lru_cache.

comment:10 by loic84, 10 years ago

Resolution: fixed
Status: closednew

comment:11 by Tim Graham, 10 years ago

Triage Stage: AcceptedReady for checkin

We are just waiting on the new server.

comment:12 by Tim Graham <timograham@…>, 10 years ago

Resolution: fixed
Status: newclosed

In 2c681e8a8c453b1691219eace7ed5dc636e37948:

Fixed #22867 -- Memoized django.utils.version.get_git_changeset().

Restored original fix that had to be removed because the old djangoproject
server was still using Python 2.6.

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