Eliminate serializing into and out of SavedCache

Author: msullivan

mypy/build.py

@@ -2186,13 +2186,16 @@ def dump_graph(graph: Graph) -> None:
     print("[" + ",\n ".join(node.dumps() for node in nodes) + "\n]")
 
 
-def load_graph(sources: List[BuildSource], manager: BuildManager) -> Graph:
+def load_graph(sources: List[BuildSource], manager: BuildManager,
+               old_graph: Optional[Graph] = None) -> Graph:
     """Given some source files, load the full dependency graph.
 
     As this may need to parse files, this can raise CompileError in case
     there are syntax errors.
     """
-    graph = {}  # type: Graph
+
+    graph = old_graph or {}  # type: Graph
+
     # The deque is used to implement breadth-first traversal.
     # TODO: Consider whether to go depth-first instead.  This may
     # affect the order in which we process files within import cycles.

mypy/checker.py

@@ -216,6 +216,16 @@ def __init__(self, errors: Errors, modules: Dict[str, MypyFile], options: Option
         # for processing module top levels in fine-grained incremental mode.
         self.recurse_into_functions = True
 
+    def reset(self) -> None:
+        """Cleanup stale state that might be left over from a typechecking run.
+
+        This allows us to reuse TypeChecker objects in fine-grained
+        incremental mode.
+        """
+        self.partial_reported.clear()
+        assert self.partial_types == []
+        assert self.deferred_nodes == []
+
     def check_first_pass(self) -> None:
         """Type check the entire file, but defer functions with unresolved references.
 

mypy/dmypy_server.py

@@ -282,10 +282,11 @@ def initialize_fine_grained(self, sources: List[mypy.build.BuildSource]) -> Dict
         if self.options.use_fine_grained_cache:
             # Pull times and hashes out of the saved_cache and stick them into
             # the fswatcher, so we pick up the changes.
-            for meta, mypyfile, type_map in manager.saved_cache.values():
-                if meta.mtime is None: continue
+            for state in self.fine_grained_manager.graph.values():
+                meta = state.meta
+                if meta is None: continue
                 self.fswatcher.set_file_data(
-                    mypyfile.path,
+                    state.xpath,
                     FileData(st_mtime=float(meta.mtime), st_size=meta.size, md5=meta.hash))
 
             # Run an update

mypy/server/update.py

@@ -161,6 +161,7 @@ def __init__(self,
         self.previous_modules = get_module_to_path_map(manager)
         self.deps = get_all_dependencies(manager, graph, self.options)
         self.previous_targets_with_errors = manager.errors.targets()
+        self.graph = graph
         # Module, if any, that had blocking errors in the last run as (id, path) tuple.
         # TODO: Handle blocking errors in the initial build
         self.blocking_error = None  # type: Optional[Tuple[str, str]]
@@ -170,8 +171,7 @@ def __init__(self,
         # for the cache. This is kind of a hack and it might be better to have
         # this directly reflected in load_graph's interface.
         self.options.cache_dir = os.devnull
-        mark_all_meta_as_memory_only(graph, manager)
-        manager.saved_cache = preserve_full_cache(graph, manager)
+        manager.saved_cache = {}
         self.type_maps = extract_type_maps(graph)
         # Active triggers during the last update
         self.triggered = []  # type: List[str]
@@ -261,7 +261,7 @@ def update_single(self, module: str, path: str) -> Tuple[List[str],
             old_snapshots[module] = snapshot
 
         manager.errors.reset()
-        result = update_single_isolated(module, path, manager, previous_modules)
+        result = update_single_isolated(module, path, manager, previous_modules, self.graph)
         if isinstance(result, BlockedUpdate):
             # Blocking error -- just give up
             module, path, remaining, errors = result
@@ -294,23 +294,13 @@ def update_single(self, module: str, path: str) -> Tuple[List[str],
             if state.tree is None and id in manager.saved_cache:
                 meta, tree, type_map = manager.saved_cache[id]
                 state.tree = tree
-        mark_all_meta_as_memory_only(graph, manager)
-        manager.saved_cache = preserve_full_cache(graph, manager)
         self.previous_modules = get_module_to_path_map(manager)
         self.type_maps = extract_type_maps(graph)
 
-        return manager.errors.new_messages(), remaining, (module, path), False
-
+        # XXX: I want us to not need this
+        self.graph = graph
 
-def mark_all_meta_as_memory_only(graph: Dict[str, State],
-                                 manager: BuildManager) -> None:
-    for id, state in graph.items():
-        if id in manager.saved_cache:
-            # Don't look at disk.
-            old = manager.saved_cache[id]
-            manager.saved_cache[id] = (old[0]._replace(memory_only=True),
-                                       old[1],
-                                       old[2])
+        return manager.errors.new_messages(), remaining, (module, path), False
 
 
 def get_all_dependencies(manager: BuildManager, graph: Dict[str, State],
@@ -350,7 +340,8 @@ def get_all_dependencies(manager: BuildManager, graph: Dict[str, State],
 def update_single_isolated(module: str,
                            path: str,
                            manager: BuildManager,
-                           previous_modules: Dict[str, str]) -> UpdateResult:
+                           previous_modules: Dict[str, str],
+                           graph: Graph) -> UpdateResult:
     """Build a new version of one changed module only.
 
     Don't propagate changes to elsewhere in the program. Raise CompleError on
@@ -371,11 +362,11 @@ def update_single_isolated(module: str,
 
     old_modules = dict(manager.modules)
     sources = get_sources(previous_modules, [(module, path)])
-    invalidate_stale_cache_entries(manager.saved_cache, [(module, path)])
+    invalidate_stale_cache_entries(manager.saved_cache, graph, [(module, path)])
 
     manager.missing_modules.clear()
     try:
-        graph = load_graph(sources, manager)
+        load_graph(sources, manager, graph)
     except CompileError as err:
         # Parse error somewhere in the program -- a blocker
         assert err.module_with_blocker
@@ -437,6 +428,7 @@ def update_single_isolated(module: str,
     replace_modules_with_new_variants(manager, graph, old_modules, new_modules)
 
     # Perform type checking.
+    state.type_checker().reset()
     state.type_check_first_pass()
     state.type_check_second_pass()
     state.compute_fine_grained_deps()
@@ -488,8 +480,9 @@ def delete_module(module_id: str,
     manager.log_fine_grained('delete module %r' % module_id)
     # TODO: Deletion of a package
     # TODO: Remove deps for the module (this only affects memory use, not correctness)
-    assert module_id not in graph
     new_graph = graph.copy()
+    if module_id in new_graph:
+        del new_graph[module_id]
     if module_id in manager.modules:
         del manager.modules[module_id]
     if module_id in manager.saved_cache:
@@ -529,9 +522,11 @@ def get_sources(modules: Dict[str, str],
     sources = [BuildSource(path, id, None)
                for id, path in items
                if os.path.isfile(path)]
+    sources = []
     for id, path in changed_modules:
-        if os.path.isfile(path) and id not in modules:
+        if os.path.isfile(path):# and id not in modules:
             sources.append(BuildSource(path, id, None))
+    # print(changed_modules, sources)
     return sources
 
 
@@ -549,75 +544,14 @@ def get_all_changed_modules(root_module: str,
     return changed_modules
 
 
-def preserve_full_cache(graph: Graph, manager: BuildManager) -> SavedCache:
-    """Preserve every module with an AST in the graph, including modules with errors."""
-    saved_cache = {}
-    for id, state in graph.items():
-        assert state.id == id
-        if state.tree is not None:
-            meta = state.meta
-            if meta is None:
-                # No metadata, likely because of an error. We still want to retain the AST.
-                # There is no corresponding JSON so create partial "memory-only" metadata.
-                assert state.path
-                dep_prios = state.dependency_priorities()
-                dep_lines = state.dependency_lines()
-                meta = memory_only_cache_meta(
-                    id,
-                    state.path,
-                    state.dependencies,
-                    state.suppressed,
-                    list(state.child_modules),
-                    dep_prios,
-                    dep_lines,
-                    state.source_hash,
-                    state.ignore_all,
-                    manager)
-            else:
-                meta = meta._replace(memory_only=True)
-            saved_cache[id] = (meta, state.tree, state.type_map())
-    return saved_cache
-
-
-def memory_only_cache_meta(id: str,
-                           path: str,
-                           dependencies: List[str],
-                           suppressed: List[str],
-                           child_modules: List[str],
-                           dep_prios: List[int],
-                           dep_lines: List[int],
-                           source_hash: str,
-                           ignore_all: bool,
-                           manager: BuildManager) -> CacheMeta:
-    """Create cache metadata for module that doesn't have a JSON cache files.
-
-    JSON cache files aren't written for modules with errors, but we want to still
-    cache them in fine-grained incremental mode.
-    """
-    options = manager.options.clone_for_module(id)
-    # Note that we omit attributes related to the JSON files.
-    meta = {'id': id,
-            'path': path,
-            'memory_only': True,  # Important bit: don't expect JSON files to exist
-            'hash': source_hash,
-            'dependencies': dependencies,
-            'suppressed': suppressed,
-            'child_modules': child_modules,
-            'options': options.select_options_affecting_cache(),
-            'dep_prios': dep_prios,
-            'dep_lines': dep_lines,
-            'interface_hash': '',
-            'version_id': manager.version_id,
-            'ignore_all': ignore_all,
-            }
-    return cache_meta_from_dict(meta, '')
-
-
 def invalidate_stale_cache_entries(cache: SavedCache,
+                                   graph: Graph,
                                    changed_modules: List[Tuple[str, str]]) -> None:
     for name, _ in changed_modules:
         if name in cache:
             del cache[name]
+        if name in graph:
+            del graph[name]
 
 
 def verify_dependencies(state: State, manager: BuildManager) -> None:
@@ -809,8 +743,8 @@ def reprocess_nodes(manager: BuildManager,
 
     Return fired triggers.
     """
-    if module_id not in manager.saved_cache or module_id not in graph:
-        manager.log_fine_grained('%s not in saved cache or graph (blocking errors or deleted?)' %
+    if module_id not in graph:
+        manager.log_fine_grained('%s not in graph (blocking errors or deleted?)' %
                     module_id)
         return set()
 
@@ -863,10 +797,8 @@ def key(node: DeferredNode) -> int:
             merge_asts(file_node, old_symbols[name], file_node, new_symbols[name])
 
     # Type check.
-    meta, file_node, type_map = manager.saved_cache[module_id]
-    graph[module_id].tree = file_node
-    graph[module_id].type_checker().type_map = type_map
     checker = graph[module_id].type_checker()
+    checker.reset()
     # We seem to need additional passes in fine-grained incremental mode.
     checker.pass_num = 0
     checker.last_pass = 3

mypy/test/testcheck.py

@@ -149,6 +149,7 @@ def run_case_once(self, testcase: DataDrivenTestCase, incremental_step: int = 0)
         options = parse_options(original_program_text, testcase, incremental_step)
         options.use_builtins_fixtures = True
         options.show_traceback = True
+        options.verbosity = 1
         if 'optional' in testcase.file:
             options.strict_optional = True
         if incremental_step:

test-data/unit/fine-grained-cycles.test

@@ -174,44 +174,7 @@ def h() -> None:
 ==
 a.py:1: error: Module 'b' has no attribute 'C'
 
-[case testReferenceToTypeThroughCycleAndReplaceWithFunction-skip-cache]
--- Different cache/no-cache tests because:
--- Cache mode has a "need type annotation" message (like coarse incremental does)
-
-import a
-
-[file a.py]
-from b import C
-
-def f() -> C: pass
-
-[file b.py]
-import a
-
-class C:
-    def g(self) -> None: pass
-
-def h() -> None:
-    c = a.f()
-    c.g()
-
-[file b.py.2]
-import a
-
-def C() -> int: pass
-
-def h() -> None:
-    c = a.f()
-    c.g()
-
-[out]
-==
-a.py:3: error: Invalid type "b.C"
-
-[case testReferenceToTypeThroughCycleAndReplaceWithFunction-skip-nocache]
--- Different cache/no-cache tests because:
--- Cache mode has a "need type annotation" message (like coarse incremental does)
-
+[case testReferenceToTypeThroughCycleAndReplaceWithFunction]
 import a
 
 [file a.py]

test-data/unit/fine-grained-modules.test

@@ -236,24 +236,23 @@ main:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports"
 
 [case testDeletionOfSubmoduleTriggersImportFrom1-skip-cache]
 -- Different cache/no-cache tests because:
--- Cache mode matches the message from regular mode and no-cache mode
--- matches the message from coarse incremental mode...
+-- missing module error message mismatch
 from p import q
 [file p/__init__.py]
 [file p/q.py]
 [delete p/q.py.2]
 [file p/q.py.3]
 [out]
 ==
-main:1: error: Cannot find module named 'p.q'
+main:1: error: Module 'p' has no attribute 'q'
 -- TODO: The following messages are different compared to non-incremental mode
+main:1: error: Cannot find module named 'p.q'
 main:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help)
 ==
 
 [case testDeletionOfSubmoduleTriggersImportFrom1-skip-nocache]
 -- Different cache/no-cache tests because:
--- Cache mode matches the message from regular mode and no-cache mode
--- matches the message from coarse incremental mode...
+-- missing module error message mismatch
 from p import q
 [file p/__init__.py]
 [file p/q.py]

test-data/unit/fine-grained.test

@@ -1108,9 +1108,9 @@ def f() -> Iterator[None]:
 [out]
 main:2: error: Revealed type is 'contextlib.GeneratorContextManager[builtins.None]'
 ==
+main:2: error: Revealed type is 'contextlib.GeneratorContextManager[builtins.None]'
 a.py:3: error: Cannot find module named 'b'
 a.py:3: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help)
-main:2: error: Revealed type is 'contextlib.GeneratorContextManager[builtins.None]'
 ==
 main:2: error: Revealed type is 'contextlib.GeneratorContextManager[builtins.None]'
 
@@ -2355,7 +2355,7 @@ def f() -> None:
 d.py:3: error: Argument 1 to "A" has incompatible type "str"; expected "int"
 ==
 
-[case testNonePartialType]
+[case testNonePartialType1]
 import a
 a.y