Opened 14 years ago
Closed 8 years ago
#14297 closed Cleanup/optimization (duplicate)
Accessing settings.FOO in hot spots cause performance problems
Reported by: | Anssi Kääriäinen | Owned by: | |
---|---|---|---|
Component: | Core (Other) | Version: | dev |
Severity: | Normal | Keywords: | settings, performance |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
Trying to access settings.FOO (no matter if it exists or not) will cause performance problems if used in hot spots of code. The reason is that settings is actually LazyObject, and each access goes through getattr. The problem is well illustrated in ticket #14290, where performance is increased considerably if a setting is cached in a global attribute.
I am trying to fix this by refactoring LazySettings in the following way: Make the new LazySettings be a regular Python object, but with a special getattr method: When first accessed, it will set up the settings in the objects own dict. After this, each access to settings.FOO, where FOO exists, will not use getattr. When trying to access settings.BAR, where BAR does not exist, the access will still need to go through getattr. This should not be a performance problem, though.
The performance increase on my laptop (using the test from #14290, with USE_L10N = False) is from 1.1 to 0.8 seconds, or 0.3 seconds. This is the cost of accessing settings.USE_L10N 60000 times and settings.USE_I18N 10000 times, plus fifteen other accesses. (The test is the http://localhost:8000/test/10000?timing one).
Attached patch passes all tests (I know how to run).
Attachments (2)
Change History (13)
by , 14 years ago
Attachment: | settings_optimization.diff added |
---|
comment:1 by , 14 years ago
Patch needs improvement: | set |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 14 years ago
Another idea how to handle this. Currently, settings is a LazySettings
object, which in turn is a LazyObject
. LazyObject
works like this: when an attribute lookup comes in to __getattr__
, then if the LazyObject
has a initialized _wrapped
object, the __getattr__
will be passed to the _wrapped
object. Now, this could be changed (for settings only) so that when something comes to __getattr__
, the value is looked in the _wrapped
object, and then set into the LazyObject
itself. Thus, any subsequent lookup will not go through __getattr__
, but instead be a direct attribute lookup. This will of course have the problem that changing settings when testing will not work. But as far as I understand there should be a better solution (to be invented) to the changing settings for testing problem. That is, settings can be changed only for testing, and otherwise they should be static throughout the whole life of the Django instance.
Just to make sure the idea is understood:
In django.conf.__init__.py
, instead of having class LazySetting(LazyObject)
, have class LazySetting(LazySettingWrapper)
. Leave everything else as is. LazySettingsWrapper
is defined as follows:
class LazySettingsWrapper(LazyObject): def __getattr__(self, name): if self._wrapped is None: self._setup() val = getattr(self._wrapped, name) # Set the attribute so that subsequent calls will # get the value directly from this object, instead # of going through __getattr__. setattr(self, name, val) return val
But as said before, this still has the ugly "changing settings for testing" problem. Maybe the best solution to this would be to have a flush_settings method in the settings object. This method could also handle clearing possibly settings-dependent cached values.
comment:3 by , 14 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
I don't know what you mean by the "changing settings for testing" problem - it's perfectly fixable by implementing setattr properly. Or, even better, a complete re-implementation of LazySettings that dodges most of the problems: when __getattr__
is first accessed, we run _setup()
which loads all the values from the actual settings object in the LazySettings dict.
I've implemented in the attached patch. I had to fix a few tests that were naughtily accessing the ._wrapped
attribute, which no longer exists, but otherwise no changes will be necessary - this is backwards compatible.
For performance, this approach might give a very slight performance hit on startup, since the whole dictionary of settings has to be copied as soon as the settings are configured. But afterwards it should eliminate a fair bit of overhead when settings are used in a loop. I've created a djangobench benchmark and can verify a 30% improvement with this patch. Other benchmarks give a small improvement (about 2%) or an insignificant change.
by , 14 years ago
Attachment: | ticket_14297.diff added |
---|
follow-up: 5 comment:4 by , 14 years ago
This latest patch works, except one problem: if the underlying module is changed it won't be reflected. Not sure if that's a semantic that was guaranteed before or not.
comment:5 by , 14 years ago
Replying to Alex:
This latest patch works, except one problem: if the underlying module is changed it won't be reflected. Not sure if that's a semantic that was guaranteed before or not.
I'm pretty sure we didn't guarantee that - the docs don't imply it, and state that you shouldn't change the settings at run time at all.
comment:6 by , 14 years ago
Another small semantic change: if you changed a settings on the settings
object, it would be reflected on the underlying module, same with deletion.
comment:7 by , 13 years ago
Severity: | → Normal |
---|---|
Type: | → Cleanup/optimization |
comment:10 by , 11 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
Accepting this to acknowledge that this could be optimized. The attached patch seems rather obscure though.