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

Let AstroidManager.clear_cache act on other caches #1521

Merged
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 13 additions & 25 deletions astroid/inference.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
Union,
)

import wrapt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering if we could remove the dependency to wrapt following this change, Turns out that there are other uses elsewhere but not that much.


from astroid import bases, decorators, helpers, nodes, protocols, util
from astroid.context import (
CallContext,
Expand Down Expand Up @@ -1037,42 +1035,32 @@ def infer_ifexp(self, context=None):
nodes.IfExp._infer = infer_ifexp # type: ignore[assignment]


# pylint: disable=dangerous-default-value
@wrapt.decorator
def _cached_generator(
func, instance: _FunctionDefT, args, kwargs, _cache={} # noqa: B006
):
node = instance
try:
return iter(_cache[func, id(node)])
except KeyError:
result = func(*args, **kwargs)
# Need to keep an iterator around
original, copy = itertools.tee(result)
_cache[func, id(node)] = list(copy)
return original


# When inferring a property, we instantiate a new `objects.Property` object,
# which in turn, because it inherits from `FunctionDef`, sets itself in the locals
# of the wrapping frame. This means that every time we infer a property, the locals
# are mutated with a new instance of the property. This is why we cache the result
# of the function's inference.
@_cached_generator
def infer_functiondef(
self: _FunctionDefT, context: Optional[InferenceContext] = None
) -> Generator[Union["Property", _FunctionDefT], None, InferenceErrorInfo]:
if not self.decorators or not bases._is_property(self):
yield self
return InferenceErrorInfo(node=self, context=context)

# When inferring a property, we instantiate a new `objects.Property` object,
# which in turn, because it inherits from `FunctionDef`, sets itself in the locals
# of the wrapping frame. This means that every time we infer a property, the locals
# are mutated with a new instance of the property. To avoid this, we detect this
# scenario and avoid passing the `parent` argument to the constructor.
parent_frame = self.parent.frame(future=True)
property_already_in_parent_locals = self.name in parent_frame.locals and any(
isinstance(val, objects.Property) for val in parent_frame.locals[self.name]
)

prop_func = objects.Property(
function=self,
name=self.name,
lineno=self.lineno,
parent=self.parent,
parent=self.parent if not property_already_in_parent_locals else None,
col_offset=self.col_offset,
)
if property_already_in_parent_locals:
prop_func.parent = self.parent
prop_func.postinit(body=[], args=self.args, doc_node=self.doc_node)
yield prop_func
return InferenceErrorInfo(node=self, context=context)
Expand Down
2 changes: 2 additions & 0 deletions astroid/interpreter/objectmodel.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import os
import pprint
import types
from functools import lru_cache
from typing import TYPE_CHECKING, List, Optional

import astroid
Expand Down Expand Up @@ -100,6 +101,7 @@ def __get__(self, instance, cls=None):
def __contains__(self, name):
return name in self.attributes()

@lru_cache() # noqa
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved
def attributes(self) -> List[str]:
"""Get the attributes which are exported by this object model."""
return [o[LEN_OF_IMPL_PREFIX:] for o in dir(self) if o.startswith(IMPL_PREFIX)]
Expand Down
24 changes: 23 additions & 1 deletion astroid/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,14 @@
import os
import types
import zipimport
from functools import _lru_cache_wrapper
from typing import TYPE_CHECKING, ClassVar, List, Optional

from astroid.exceptions import AstroidBuildingError, AstroidImportError
from astroid.interpreter._import import spec
from astroid.modutils import (
NoSourceFile,
_cache_normalize_path_,
file_info_from_modpath,
get_source_file,
is_module_name_part_of_extension_package_whitelist,
Expand Down Expand Up @@ -59,6 +61,16 @@ class AstroidManager:
max_inferable_values: ClassVar[int] = 100

def __init__(self):
# pylint: disable=import-outside-toplevel
DanielNoord marked this conversation as resolved.
Show resolved Hide resolved
from astroid.interpreter.objectmodel import ObjectModel
from astroid.nodes.node_classes import LookupMixIn

self._lru_caches: List[_lru_cache_wrapper] = [
LookupMixIn.lookup,
_cache_normalize_path_,
ObjectModel.attributes,
]

# NOTE: cache entries are added by the [re]builder
self.astroid_cache = AstroidManager.brain["astroid_cache"]
self._mod_file_cache = AstroidManager.brain["_mod_file_cache"]
Expand Down Expand Up @@ -357,6 +369,16 @@ def bootstrap(self):
raw_building._astroid_bootstrapping()

def clear_cache(self):
"""Clear the underlying cache. Also bootstraps the builtins module."""
"""Clear the underlying caches. Also bootstraps the builtins module."""
from astroid.inference_tip import ( # pylint: disable=import-outside-toplevel
clear_inference_tip_cache,
)

clear_inference_tip_cache()

self.astroid_cache.clear()

for lru_cache in self._lru_caches:
lru_cache.cache_clear()

self.bootstrap()
17 changes: 8 additions & 9 deletions astroid/modutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,9 @@
import sys
import sysconfig
import types
from functools import lru_cache
from pathlib import Path
from typing import Dict, Set
from typing import Set

from astroid.const import IS_JYTHON, IS_PYPY
from astroid.interpreter._import import spec, util
Expand Down Expand Up @@ -138,21 +139,19 @@ def _handle_blacklist(blacklist, dirnames, filenames):
filenames.remove(norecurs)


_NORM_PATH_CACHE: Dict[str, str] = {}
@lru_cache()
def _cache_normalize_path_(path: str) -> str:
jacobtylerwalls marked this conversation as resolved.
Show resolved Hide resolved
return _normalize_path(path)


def _cache_normalize_path(path: str) -> str:
jacobtylerwalls marked this conversation as resolved.
Show resolved Hide resolved
"""Normalize path with caching."""
# _module_file calls abspath on every path in sys.path every time it's
# called; on a larger codebase this easily adds up to half a second just
# assembling path components. This cache alleviates that.
try:
return _NORM_PATH_CACHE[path]
except KeyError:
if not path: # don't cache result for ''
return _normalize_path(path)
result = _NORM_PATH_CACHE[path] = _normalize_path(path)
return result
if not path: # don't cache result for ''
return _normalize_path(path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This line is not covered, on the other hand are we really ever having something falsey here ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added coverage. We talked a bit about it here.

return _cache_normalize_path_(path)


def load_module_from_name(dotted_name: str) -> types.ModuleType:
Expand Down
2 changes: 1 addition & 1 deletion astroid/nodes/node_classes.py
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ def get_children(self):
class LookupMixIn:
"""Mixin to look up a name in the right scope."""

@lru_cache(maxsize=None) # pylint: disable=cache-max-size-none # noqa
@lru_cache() # noqa
def lookup(self, name: str) -> typing.Tuple[str, typing.List[NodeNG]]:
"""Lookup where the given variable is assigned.

Expand Down
38 changes: 38 additions & 0 deletions tests/unittest_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@
from astroid import manager, test_utils
from astroid.const import IS_JYTHON
from astroid.exceptions import AstroidBuildingError, AstroidImportError
from astroid.interpreter.objectmodel import ObjectModel
from astroid.modutils import is_standard_module
from astroid.nodes.scoped_nodes import ClassDef

from . import resources

Expand Down Expand Up @@ -315,5 +318,40 @@ def test_borg(self) -> None:
self.assertIs(built, second_built)


class ClearCacheTest(unittest.TestCase, resources.AstroidCacheSetupMixin):
def test_clear_cache(self) -> None:
# Get a baseline for the size of the cache after simply calling bootstrap()
baseline_cache_infos = [lru.cache_info() for lru in astroid.MANAGER._lru_caches]

# Generate some hits and misses
ClassDef().lookup("garbage")
is_standard_module("unittest", std_path=["garbage_path"])
ObjectModel().attributes()

# Did the hits or misses actually happen?
incremented_cache_infos = [
lru.cache_info() for lru in astroid.MANAGER._lru_caches
]
for incremented_cache, baseline_cache in zip(
incremented_cache_infos, baseline_cache_infos
):
with self.subTest(incremented_cache=incremented_cache):
self.assertGreater(
incremented_cache.hits + incremented_cache.misses,
baseline_cache.hits + baseline_cache.misses,
)

astroid.MANAGER.clear_cache() # also calls bootstrap()

# The cache sizes are now as low or lower than the original baseline
cleared_cache_infos = [lru.cache_info() for lru in astroid.MANAGER._lru_caches]
for cleared_cache, baseline_cache in zip(
cleared_cache_infos, baseline_cache_infos
):
with self.subTest(cleared_cache=cleared_cache):
# less equal because the "baseline" might have had multiple calls to bootstrap()
self.assertLessEqual(cleared_cache.currsize, baseline_cache.currsize)


if __name__ == "__main__":
unittest.main()
2 changes: 0 additions & 2 deletions tests/unittest_scoped_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -1678,10 +1678,8 @@ def __init__(self):
"FinalClass",
"ClassB",
"MixinB",
"",
"ClassA",
"MixinA",
"",
jacobtylerwalls marked this conversation as resolved.
Show resolved Hide resolved
"Base",
"object",
],
Expand Down