Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance improvement #417

Merged
merged 2 commits into from
May 17, 2017

Conversation

Proper-Job
Copy link
Contributor

@Proper-Job Proper-Job commented May 16, 2017

After pouring over the imagekit code I believe I have found the culprit that causes the major performance problems voiced in: #325, #375, #256, #250
Starting with Django >= 1.7 every call to retrieve a generator would erroneously run the imagekit.utils.autodiscover routine. This fix allows the routine to be run only once.

…over() only once on Django > 1.7 as it was intended.
@vstoykov
Copy link
Collaborator

Thank you for your contribution!

Actually the problem that you are fixing is a regression in ImageKit 4 introduced by me on 22 March 2017 (7551936) which is long time after mentioned issues where made.

Now when I'm looking again at that code I'm wondering if you can fix it a little bit more. Remove the logic for checking _autodiscovered in _autodiscover_modules_fallback and setting _autodiscovered to True to be made only in autodiscover() no mather which autodiscover_modules function was called.

We need to continue investigating the performance issues described in mentioned above issues.

@Proper-Job
Copy link
Contributor Author

Hi @vstoykov
I made the changes you requested.
Even though this doesn't fix the original problem, I was able to cut my current response times in half using this fix. I believe this fix alone merits a maintenance release. What do you think?
I guess we'll just have to keep looking for the original problem after this ;)

@vstoykov
Copy link
Collaborator

Yes I need to release a new version because this is bad regression but easy fixable.

Thank you for your contribution and I will make new release ASAP.

@vstoykov vstoykov merged commit 499e9e1 into matthewwithanm:develop May 17, 2017
@Proper-Job
Copy link
Contributor Author

@vstoykov thank you

@matthewwithanm
Copy link
Owner

❤️

@vstoykov
Copy link
Collaborator

There is new release 4.0.1 in PyPi

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.

None yet

3 participants