Fixed #25750 -- Made Options._expire_cache faster #5649
Conversation
|
There are so many typos in that commit message that I'll amend it to not embarass myself any further. |
Made Options._expire_cache much faster by avoiding unnecessary list operations and by avoiding `delattr` calls that would result in an exception being raised as it involves call frame reconstruction which is very costly. Especially so in a function that is called millions of times.
| REVERSE_PROPERTIES = ('related_objects', 'fields_map', '_relation_tree') | ||
| FORWARD_PROPERTIES = {'fields', 'many_to_many', 'concrete_fields', | ||
| 'local_concrete_fields', '_forward_fields_map'} | ||
| REVERSE_PROPERTIES = {'related_objects', 'fields_map', '_relation_tree'} |
There was a problem hiding this comment.
For what I've read about performance, sets perform better when testing membership and lists perform better when iterating over (which seems the case here). To be checked...
There was a problem hiding this comment.
This is not a matter of performance. We care about a particular set of values, not about their particular order. Set is the idiomatic Python type to use here.
There was a problem hiding this comment.
Fair, but we are still in a critical section of code performance-wise.
There was a problem hiding this comment.
While my tests suggest that iterating a set is ~8% slower in this case, it is a negligible difference once you look at the whole _expire_cache function.
There was a problem hiding this comment.
OK, let's ignore my comment, then. Thanks for testing.
|
merged in 7628f87, thanks! |
Made Options._expire_cache much faster by avoiding unnecessary list operations and by avoiding
delattrcalls that would result in an exception being raised as it involves call frame reconstruction which is very costly. Especially so in a function that is called millions of times.Relevant ticket: https://code.djangoproject.com/ticket/25750