Implement cache meachanism to speed up module search #175

hltbra · 2017-03-06T20:55:22Z

This should fix the performance issues reported in issue #69.

I'd like to hear from other people that experienced the slowdowns to see if this pull request will fix their issues.

coveralls · 2017-03-06T20:58:21Z

Coverage increased (+0.6%) to 96.456% when pulling b23129e on feature/speed-up-module-search into 88f6990 on master.

Deimos · 2017-03-07T00:16:02Z

Works without any modifications necessary now, results seem about the same as I mentioned in there with the previous version - about 0.25s duration on the first test, and then it's difficult to tell if there's any change to other ones. Seems to just add about 0.25s to the run time in total, from a few unscientific runs.

spulec · 2017-03-07T02:05:12Z

freezegun/api.py

+    if _get_module_attributes_hash(module) == module_hash:
+        return cached_attrs
+    else:
+        return _get_module_attributes(module)


Do we want to be setting these back on global_modules_cache to update the cache?

hltbra · 2017-03-07T16:02:25Z

freezegun/api.py

+    result = {}
+    for mod_name, module in list(sys.modules.items()):
+        # ignore this module
+        if mod_name == __name__:


@spulec: in the _freeze_time.start method I saw a similar logic (if mod_name is None or module is None) but I didn't understand the None checks and didn't put them here. When are they None?

answering myself: module is None when it's a freezegun module.

coveralls · 2017-03-16T19:56:47Z

Coverage increased (+0.6%) to 96.474% when pulling 199c655 on feature/speed-up-module-search into 88f6990 on master.

coveralls · 2017-03-16T19:56:47Z

Coverage increased (+0.6%) to 96.474% when pulling 199c655 on feature/speed-up-module-search into 88f6990 on master.

coveralls · 2017-03-16T20:06:13Z

Coverage increased (+0.6%) to 96.474% when pulling 8d5193e on feature/speed-up-module-search into 88f6990 on master.

hltbra · 2017-03-20T17:05:57Z

I will test this against moto later. I think there's a bug after running this against a work (private) project.

nolanlum · 2017-05-09T22:34:11Z

Any update on merging this PR? I'm also interested in the speedups this provides, but I'm running into a strange bug with the module cache:

module = <AliasModule 'py.error' for 'py._error.error'>

    def _setup_module_cache(module):
        global _GLOBAL_MODULES_CACHE
        date_attrs = []
        all_module_attributes = _get_module_attributes(module)
        for attribute_name, attribute_value in all_module_attributes:
            if id(attribute_value) in _real_time_object_ids:
                date_attrs.append((attribute_name, attribute_value))
>       _GLOBAL_MODULES_CACHE[module.__name__] = (_get_module_attributes_hash(module), date_attrs)
E       AttributeError: __name__

/usr/local/lib/python2.7/site-packages/freezegun/api.py:85: AttributeError

azmeuk · 2017-08-29T19:15:28Z

freezegun/api.py

+    for attribute_name, attribute_value in all_module_attributes:
+        if id(attribute_value) in _real_time_object_ids:
+            date_attrs.append((attribute_name, attribute_value))
+    _GLOBAL_MODULES_CACHE[module.__name__] = (_get_module_attributes_hash(module), date_attrs)


I encountered this bug:

local.virtualenv/lib/python2.7/site-packages/freezegun/api.py:557: in wrapper with self: local.virtualenv/lib/python2.7/site-packages/freezegun/api.py:434: in __enter__ return self.start() local.virtualenv/lib/python2.7/site-packages/freezegun/api.py:490: in start module_attrs = _get_cached_module_attributes(mod_name, module) local.virtualenv/lib/python2.7/site-packages/freezegun/api.py:96: in _get_cached_module_attributes global_modules_cache = _get_global_modules_cache() local.virtualenv/lib/python2.7/site-packages/freezegun/api.py:53: in _get_global_modules_cache _setup_modules_cache() local.virtualenv/lib/python2.7/site-packages/freezegun/api.py:75: in _setup_modules_cache _setup_module_cache(module) _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ module = <AliasModule 'py.error' for 'py._error.error'> def _setup_module_cache(module): global _GLOBAL_MODULES_CACHE #if not hasattr(module, "__name__"): # return date_attrs = [] all_module_attributes = _get_module_attributes(module) for attribute_name, attribute_value in all_module_attributes: if id(attribute_value) in _real_time_object_ids: date_attrs.append((attribute_name, attribute_value)) > _GLOBAL_MODULES_CACHE[module.__name__] = (_get_module_attributes_hash(module), date_attrs) E AttributeError: __name__ all_module_attributes = [] date_attrs = [] module = <AliasModule 'py.error' for 'py._error.error'> local.virtualenv/lib/python2.7/site-packages/freezegun/api.py:88: AttributeError

It seems ok with the following code:

def _setup_modules_cache(): for mod_name, module in list(sys.modules.items()): if mod_name == __name__ or not mod_name or not module or not hasattr(module, "__name__"): continue ...

azmeuk · 2017-08-29T19:20:38Z

@hltbra @spulec I am very interested in improving the performances of freezegun. Is there something I can do to help with this PR?

Fixed version of spulec#175

spulec · 2018-03-06T01:17:58Z

Thanks!

Chronial · 2018-03-06T07:24:02Z

freezegun/api.py

+
+
+def _get_module_attributes_hash(module):
+    return '{0}-{1}'.format(id(module), hashlib.md5(','.join(dir(module)).encode('utf-8')).hexdigest())


In line 63, you catch a TypeError when you call dir(module), here you don't. If a TypeError is raised there, that doesn't seem to prevent this function from being called. Is that an issue?
Why don't you pass the already existing all_module_attributes to this function and extract the names from that?

Thanks, fixed with 689a7bb.

mariuszthecoder · 2018-03-06T14:34:35Z

freezegun/api.py

@@ -66,7 +66,7 @@ def _setup_modules_cache():
    # FIXME: move this definition to be at the top-level
    real_time_object_ids = set(id(obj) for obj in real_date_objects)
    result = {}
-    for mod_name, module in sys.modules.items():
+    for mod_name, module in list(sys.modules.items()):


it's unnecessary - if sys.modules is dict , then sys.modules.items() is iterable containing pairs of key and value in both py2 and py3
casting it to list will not break it, but slow down a bit

Without creating the copy, I'm occasionally getting a RuntimeError: dictionary changed size during iteration, which makes sense since additional modules can get loaded.

Ideas?

I would assume that the size of that collection is small enough so that casting to list once does not cause a relevant slowdown.

so commit message is misleading

No, the PR title is about an architectural optimization that saves seconds of run time, not microoptimizations that make a difference of a few microseconds.

I meant this particular commit, not entire PullRequest
Reading Make sys.modules.items() compatible with python 3 I expect some py3 adjustment, but I get runtime error prevention.
That was my point, sir :)

The commit message is not misleading. In python 2, items() returns a list (copy), which is what is needed here. But in python 3 it returns an iterator, which causes faulty behavior. So it is casted to list for python 3 support.

pzelnip · 2018-03-07T21:58:09Z

With 0.3.10 over 0.3.9 I'm seeing about a 16% speedup. Not bad.

mariuszthecoder · 2018-03-09T13:30:40Z

How do you measure speed improvement?

pzelnip · 2018-03-09T17:17:40Z

I ran the tests and looked at the time that nose said it took to run them. Did that 4 times and took the average. Then uninstalled 0.3.9 & installed 0.3.10 and did the same. The difference between the two averages was about 16%.

hltbra added 4 commits March 6, 2017 11:46

Speed up module attribute search (proof of concept). Refs #69

e4072e4

Refactor code around module attribute search (refs e4072e4)

f5f30fd

Make sys.modules.items() compatible with python 3

9d0d22a

Make string formatting compatible with python 2.6

b23129e

spulec reviewed Mar 7, 2017

View reviewed changes

hltbra commented Mar 7, 2017

View reviewed changes

Cache modules before returning from _get_cached_module_attributes()

8d5193e

hltbra force-pushed the feature/speed-up-module-search branch from 199c655 to 8d5193e Compare March 16, 2017 20:00

azmeuk reviewed Aug 29, 2017

View reviewed changes

nolanlum pushed a commit to lyft/freezegun that referenced this pull request Mar 1, 2018

Implement cache mechanism to speed up module search.

b8fc6b8

Fixed version of spulec#175

spulec added 2 commits March 5, 2018 20:15

Fix edge cases.

63cee42

Fix merge conflicts.

1654a14

spulec merged commit 652bb7d into master Mar 6, 2018

spulec deleted the feature/speed-up-module-search branch March 6, 2018 01:17

This was referenced Mar 6, 2018

Optimize api. #224

Closed

0.2.3 performance issue #69

Closed

Chronial reviewed Mar 6, 2018

View reviewed changes

mariuszthecoder reviewed Mar 6, 2018

View reviewed changes

iloveagent57 mentioned this pull request Mar 22, 2018

EDUCATOR-2540 | More efficient query; bump freezegun version to speed up tests openedx/completion#13

Merged

12 tasks

spulec mentioned this pull request Jun 1, 2018

Freezing of custom datetime classes #243

Open

charettes mentioned this pull request Sep 30, 2018

Faked time shouldn't always be the same. #89

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement cache meachanism to speed up module search #175

Implement cache meachanism to speed up module search #175

hltbra commented Mar 6, 2017

coveralls commented Mar 6, 2017 •

edited

Deimos commented Mar 7, 2017

spulec Mar 7, 2017

hltbra Mar 7, 2017

hltbra Mar 7, 2017

hltbra Mar 16, 2017

coveralls commented Mar 16, 2017

coveralls commented Mar 16, 2017 •

edited

coveralls commented Mar 16, 2017 •

edited

hltbra commented Mar 20, 2017

nolanlum commented May 9, 2017

azmeuk Aug 29, 2017 •

edited

azmeuk commented Aug 29, 2017

spulec commented Mar 6, 2018

Chronial Mar 6, 2018

spulec Mar 6, 2018

mariuszthecoder Mar 6, 2018

spulec Mar 6, 2018

Chronial Mar 6, 2018

mariuszthecoder Mar 7, 2018

Chronial Mar 7, 2018

mariuszthecoder Mar 9, 2018

Chronial Mar 12, 2018

mariuszthecoder Mar 14, 2018

pzelnip commented Mar 7, 2018

mariuszthecoder commented Mar 9, 2018

pzelnip commented Mar 9, 2018



		def _get_module_attributes_hash(module):
		return '{0}-{1}'.format(id(module), hashlib.md5(','.join(dir(module)).encode('utf-8')).hexdigest())

Implement cache meachanism to speed up module search #175

Implement cache meachanism to speed up module search #175

Conversation

hltbra commented Mar 6, 2017

coveralls commented Mar 6, 2017 • edited

Deimos commented Mar 7, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Mar 16, 2017

coveralls commented Mar 16, 2017 • edited

coveralls commented Mar 16, 2017 • edited

hltbra commented Mar 20, 2017

nolanlum commented May 9, 2017

azmeuk Aug 29, 2017 • edited

Choose a reason for hiding this comment

azmeuk commented Aug 29, 2017

spulec commented Mar 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pzelnip commented Mar 7, 2018

mariuszthecoder commented Mar 9, 2018

pzelnip commented Mar 9, 2018

coveralls commented Mar 6, 2017 •

edited

coveralls commented Mar 16, 2017 •

edited

coveralls commented Mar 16, 2017 •

edited

azmeuk Aug 29, 2017 •

edited