From 22918dc90a1443574a76df93535a0f78e2431d27 Mon Sep 17 00:00:00 2001 From: Michael Sullivan Date: Tue, 16 Jan 2018 15:11:11 -0800 Subject: [PATCH 1/8] Support loading from cache in fine-grained incremental mdoe --- mypy/build.py | 7 +++++++ mypy/dmypy_server.py | 5 ++++- mypy/main.py | 2 ++ mypy/options.py | 1 + mypy/server/update.py | 29 ++++++++++++++++++++++------- 5 files changed, 36 insertions(+), 8 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index 18958903a12f..4c6f17d91b14 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -2383,6 +2383,13 @@ def process_graph(graph: Graph, manager: BuildManager) -> None: manager.log("Processing SCC of size %d (%s) as %s" % (size, scc_str, fresh_msg)) process_stale_scc(graph, scc, manager) + # If we are running in fine-grained incremental mode with caching, + # we need to always process fresh SCCs. + if manager.options.use_fine_grained_cache: + for prev_scc in fresh_scc_queue: + process_fresh_scc(graph, prev_scc, manager) + fresh_scc_queue = [] + sccs_left = len(fresh_scc_queue) nodes_left = sum(len(scc) for scc in fresh_scc_queue) manager.add_stats(sccs_left=sccs_left, nodes_left=nodes_left) diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index c05d43eced1f..a480e9686bb6 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -99,13 +99,16 @@ def __init__(self, flags: List[str]) -> None: sys.exit("dmypy: start/restart should not disable incremental mode") if options.quick_and_dirty: sys.exit("dmypy: start/restart should not specify quick_and_dirty mode") + if options.use_fine_grained_cache and not options.fine_grained_incremental: + sys.exit("dmypy: fine-grained cache can only be used in experimental mode") self.options = options if os.path.isfile(STATUS_FILE): os.unlink(STATUS_FILE) if self.fine_grained: options.incremental = True options.show_traceback = True - options.cache_dir = os.devnull + if not options.use_fine_grained_cache: + options.cache_dir = os.devnull def serve(self) -> None: """Serve requests, synchronously (no thread or fork).""" diff --git a/mypy/main.py b/mypy/main.py index 10d8d1251b77..cbcab24d2b01 100644 --- a/mypy/main.py +++ b/mypy/main.py @@ -395,6 +395,8 @@ def add_invertible_flag(flag: str, if server_options: parser.add_argument('--experimental', action='store_true', dest='fine_grained_incremental', help="enable fine-grained incremental mode") + parser.add_argument('--use-fine-grained-cache', action='store_true', + help="use the cache in fine-grained incremental mode") report_group = parser.add_argument_group( title='report generation', diff --git a/mypy/options.py b/mypy/options.py index c62520075d34..cb7871778f29 100644 --- a/mypy/options.py +++ b/mypy/options.py @@ -144,6 +144,7 @@ def __init__(self) -> None: self.skip_version_check = False self.fine_grained_incremental = False self.cache_fine_grained = False + self.use_fine_grained_cache = False # Paths of user plugins self.plugins = [] # type: List[str] diff --git a/mypy/server/update.py b/mypy/server/update.py index 6b0a9721c833..0b279887ba51 100644 --- a/mypy/server/update.py +++ b/mypy/server/update.py @@ -281,9 +281,10 @@ def update_single(self, module: str, path: str) -> Tuple[List[str], print('triggered:', sorted(filtered)) self.triggered.extend(triggered | self.previous_targets_with_errors) collect_dependencies({module: tree}, self.deps, graph) - propagate_changes_using_dependencies(manager, graph, self.deps, triggered, - {module}, - self.previous_targets_with_errors) + remaining += propagate_changes_using_dependencies( + manager, graph, self.deps, triggered, + {module}, + self.previous_targets_with_errors) # Preserve state needed for the next update. self.previous_targets_with_errors = manager.errors.targets() @@ -318,6 +319,9 @@ def mark_all_meta_as_memory_only(graph: Dict[str, State], def get_all_dependencies(manager: BuildManager, graph: Dict[str, State], options: Options) -> Dict[str, Set[str]]: """Return the fine-grained dependency map for an entire build.""" + if not options.use_fine_grained_cache: + for state in graph.values(): + state.compute_fine_grained_deps() deps = {} # type: Dict[str, Set[str]] collect_dependencies(manager.modules, deps, graph) return deps @@ -441,6 +445,7 @@ def update_single_isolated(module: str, # Perform type checking. state.type_check_first_pass() state.type_check_second_pass() + state.compute_fine_grained_deps() state.finish_passes() # TODO: state.write_cache()? # TODO: state.mark_as_rechecked()? @@ -654,7 +659,6 @@ def collect_dependencies(new_modules: Mapping[str, Optional[MypyFile]], for id, node in new_modules.items(): if node is None: continue - graph[id].compute_fine_grained_deps() for trigger, targets in graph[id].fine_grained_deps.items(): deps.setdefault(trigger, set()).update(targets) @@ -711,9 +715,10 @@ def propagate_changes_using_dependencies( deps: Dict[str, Set[str]], triggered: Set[str], up_to_date_modules: Set[str], - targets_with_errors: Set[str]) -> None: + targets_with_errors: Set[str]) -> List[Tuple[str, str]]: # TODO: Multiple type checking passes num_iter = 0 + remaining_modules = [] # Propagate changes until nothing visible has changed during the last # iteration. @@ -737,7 +742,13 @@ def propagate_changes_using_dependencies( # TODO: Preserve order (set is not optimal) for id, nodes in sorted(todo.items(), key=lambda x: x[0]): assert id not in up_to_date_modules - triggered |= reprocess_nodes(manager, graph, id, nodes, deps) + # TODO: Is there a better way to detect that the file isn't loaded? + if not manager.modules[id].defs: + # We haven't actually loaded this file! Add it to the + # queue of files that need to be processed fully. + remaining_modules.append((id, manager.modules[id].path)) + else: + triggered |= reprocess_nodes(manager, graph, id, nodes, deps) # Changes elsewhere may require us to reprocess modules that were # previously considered up to date. For example, there may be a # dependency loop that loops back to an originally processed module. @@ -746,6 +757,8 @@ def propagate_changes_using_dependencies( if DEBUG: print('triggered:', list(triggered)) + return remaining_modules + def find_targets_recursive( triggers: Set[str], @@ -993,4 +1006,6 @@ def lookup_target(modules: Dict[str, MypyFile], target: str) -> List[DeferredNod def extract_type_maps(graph: Graph) -> Dict[str, Dict[Expression, Type]]: - return {id: state.type_map() for id, state in graph.items()} + # This is used to export information used only by the testmerge harness. + return {id: state.type_map() for id, state in graph.items() + if state.tree} From e5afefdc3696b33a4e473eacf2c668a01ba29e2f Mon Sep 17 00:00:00 2001 From: Michael Sullivan Date: Wed, 31 Jan 2018 17:56:52 -0800 Subject: [PATCH 2/8] Do a fine-grained incremental run when loading from cache --- mypy/build.py | 5 +++++ mypy/dmypy_server.py | 24 +++++++++++++++++++++--- mypy/fswatcher.py | 3 +++ 3 files changed, 29 insertions(+), 3 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index 4c6f17d91b14..bc3b3a30bfe5 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -1131,6 +1131,11 @@ def validate_meta(meta: Optional[CacheMeta], id: str, path: Optional[str], if not stat.S_ISREG(st.st_mode): manager.log('Metadata abandoned for {}: file {} does not exist'.format(id, path)) return None + + if manager.options.use_fine_grained_cache: + manager.log('Using potentially stale metadata for {}'.format(id)) + return meta + size = st.st_size if size != meta.size: manager.log('Metadata abandoned for {}: file {} has different size'.format(id, path)) diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index a480e9686bb6..8a7c5aa45ddc 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -24,7 +24,7 @@ from mypy.dmypy_util import STATUS_FILE, receive from mypy.gclogger import GcLogger from mypy.fscache import FileSystemCache -from mypy.fswatcher import FileSystemWatcher +from mypy.fswatcher import FileSystemWatcher, FileData def daemonize(func: Callable[[], None], log_file: Optional[str] = None) -> int: @@ -266,11 +266,29 @@ def initialize_fine_grained(self, sources: List[mypy.build.BuildSource]) -> Dict manager = result.manager graph = result.graph self.fine_grained_manager = mypy.server.update.FineGrainedBuildManager(manager, graph) - status = 1 if messages else 0 - self.previous_messages = messages[:] self.fine_grained_initialized = True self.previous_sources = sources self.fscache.flush() + + # If we are using the fine-grained cache, build hasn't actually done + # the typechecking on the updated files yet. + # Run a fine-grained update starting from the cached data + 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 + self.fswatcher.set_file_data( + meta.path, + FileData(st_mtime=float(meta.mtime), st_size=meta.size, md5=meta.hash)) + + # Run an update + changed = self.find_changed(sources) + if changed: + messages += self.fine_grained_manager.update(changed) + + status = 1 if messages else 0 + self.previous_messages = messages[:] return {'out': ''.join(s + '\n' for s in messages), 'err': '', 'status': status} def fine_grained_increment(self, sources: List[mypy.build.BuildSource]) -> Dict[str, Any]: diff --git a/mypy/fswatcher.py b/mypy/fswatcher.py index c6eeae2ddb83..d4efc10d6105 100644 --- a/mypy/fswatcher.py +++ b/mypy/fswatcher.py @@ -36,6 +36,9 @@ def __init__(self, fs: FileSystemCache) -> None: def paths(self) -> AbstractSet[str]: return self._paths + def set_file_data(self, path: str, data: FileData) -> None: + self._file_data[path] = data + def add_watched_paths(self, paths: Iterable[str]) -> None: for path in paths: if path not in self._paths: From b1a5a25236448cae2fc05cf17dca0fcba9456a33 Mon Sep 17 00:00:00 2001 From: Michael Sullivan Date: Mon, 5 Feb 2018 16:10:16 -0800 Subject: [PATCH 3/8] Fix some bugs in fg cache loading --- mypy/dmypy_server.py | 3 ++- mypy/server/update.py | 5 +++-- test-data/unit/fine-grained-modules.test | 1 - 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index 8a7c5aa45ddc..19264944749f 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -265,6 +265,7 @@ def initialize_fine_grained(self, sources: List[mypy.build.BuildSource]) -> Dict messages = result.errors manager = result.manager graph = result.graph + manager.options.cache_dir = os.devnull # XXX: HACK self.fine_grained_manager = mypy.server.update.FineGrainedBuildManager(manager, graph) self.fine_grained_initialized = True self.previous_sources = sources @@ -285,7 +286,7 @@ def initialize_fine_grained(self, sources: List[mypy.build.BuildSource]) -> Dict # Run an update changed = self.find_changed(sources) if changed: - messages += self.fine_grained_manager.update(changed) + messages = self.fine_grained_manager.update(changed) status = 1 if messages else 0 self.previous_messages = messages[:] diff --git a/mypy/server/update.py b/mypy/server/update.py index 0b279887ba51..ce43f85e9802 100644 --- a/mypy/server/update.py +++ b/mypy/server/update.py @@ -378,7 +378,7 @@ def update_single_isolated(module: str, sources = get_sources(previous_modules, [(module, path)]) invalidate_stale_cache_entries(manager.saved_cache, [(module, path)]) - manager.missing_modules = set() + manager.missing_modules.clear() try: graph = load_graph(sources, manager) except CompileError as err: @@ -497,7 +497,8 @@ def delete_module(module_id: str, # TODO: Remove deps for the module (this only affects memory use, not correctness) assert module_id not in graph new_graph = graph.copy() - del manager.modules[module_id] + if module_id in manager.modules: + del manager.modules[module_id] if module_id in manager.saved_cache: del manager.saved_cache[module_id] components = module_id.split('.') diff --git a/test-data/unit/fine-grained-modules.test b/test-data/unit/fine-grained-modules.test index 0a254e2ae51f..698e863be76b 100644 --- a/test-data/unit/fine-grained-modules.test +++ b/test-data/unit/fine-grained-modules.test @@ -245,7 +245,6 @@ from p import q main:1: error: Cannot find module named 'p.q' -- TODO: The following messages are different compared to non-incremental mode main:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) -main:1: error: Module 'p' has no attribute 'q' == [case testDeletionOfSubmoduleTriggersImportFrom2] From e3868203e6a4bfb620d296573dd09032e06b7e14 Mon Sep 17 00:00:00 2001 From: Michael Sullivan Date: Mon, 5 Feb 2018 17:48:19 -0800 Subject: [PATCH 4/8] Run fine-grained tests using a cache --- mypy/test/testfinegrained.py | 45 +++++++++++++++++++---- test-data/unit/fine-grained-blockers.test | 9 ++++- test-data/unit/fine-grained-cycles.test | 5 ++- test-data/unit/fine-grained-modules.test | 9 ++++- test-data/unit/fine-grained.test | 4 +- 5 files changed, 58 insertions(+), 14 deletions(-) diff --git a/mypy/test/testfinegrained.py b/mypy/test/testfinegrained.py index a442f1052e11..261749fa49e8 100644 --- a/mypy/test/testfinegrained.py +++ b/mypy/test/testfinegrained.py @@ -43,15 +43,30 @@ class FineGrainedSuite(DataSuite): optional_out = True def run_case(self, testcase: DataDrivenTestCase) -> None: + self.run_case_inner(testcase, cache=False) + + # Reset the test case and run it again with caching on + testcase.teardown() + testcase.setup() + self.run_case_inner(testcase, cache=True) + + def run_case_inner(self, testcase: DataDrivenTestCase, cache: bool) -> None: + # TODO: In caching mode we currently don't well support + # starting from cached states with errors in them. + if cache and testcase.output and testcase.output[0] != '==': return + main_src = '\n'.join(testcase.input) sources_override = self.parse_sources(main_src) - messages, manager, graph = self.build(main_src, testcase, sources_override) - + print("Testing with cache: ", cache) + messages, manager, graph = self.build(main_src, testcase, sources_override, + build_cache=cache, enable_cache=cache) a = [] if messages: a.extend(normalize_messages(messages)) - fine_grained_manager = FineGrainedBuildManager(manager, graph) + fine_grained_manager = None + if not cache: + fine_grained_manager = FineGrainedBuildManager(manager, graph) steps = testcase.find_steps() all_triggered = [] @@ -70,6 +85,15 @@ def run_case(self, testcase: DataDrivenTestCase) -> None: modules = [(module, path) for module, path in sources_override if any(m == module for m, _ in modules)] + + # If this is the second iteration and we are using a + # cache, now we need to set it up + if fine_grained_manager is None: + messages, manager, graph = self.build(main_src, testcase, sources_override, + build_cache=False, enable_cache=cache) + manager.options.cache_dir = os.devnull # XXX: HACK + fine_grained_manager = FineGrainedBuildManager(manager, graph) + new_messages = fine_grained_manager.update(modules) all_triggered.append(fine_grained_manager.triggered) new_messages = normalize_messages(new_messages) @@ -80,10 +104,11 @@ def run_case(self, testcase: DataDrivenTestCase) -> None: # Normalize paths in test output (for Windows). a = [line.replace('\\', '/') for line in a] + modestr = "in cache mode " if cache else "" assert_string_arrays_equal( testcase.output, a, - 'Invalid output ({}, line {})'.format(testcase.file, - testcase.line)) + 'Invalid output {}({}, line {})'.format( + modestr, testcase.file, testcase.line)) if testcase.triggered: assert_string_arrays_equal( @@ -95,14 +120,18 @@ def run_case(self, testcase: DataDrivenTestCase) -> None: def build(self, source: str, testcase: DataDrivenTestCase, - sources_override: Optional[List[Tuple[str, str]]]) -> Tuple[List[str], - BuildManager, - Graph]: + sources_override: Optional[List[Tuple[str, str]]], + build_cache: bool, + enable_cache: bool) -> Tuple[List[str], BuildManager, Graph]: # This handles things like '# flags: --foo'. options = parse_options(source, testcase, incremental_step=1) options.incremental = True options.use_builtins_fixtures = True options.show_traceback = True + options.fine_grained_incremental = not build_cache + options.use_fine_grained_cache = enable_cache and not build_cache + options.cache_fine_grained = enable_cache + main_path = os.path.join(test_temp_dir, 'main') with open(main_path, 'w') as f: f.write(source) diff --git a/test-data/unit/fine-grained-blockers.test b/test-data/unit/fine-grained-blockers.test index 9eaf25eeea05..ccd0879fd7fe 100644 --- a/test-data/unit/fine-grained-blockers.test +++ b/test-data/unit/fine-grained-blockers.test @@ -237,7 +237,9 @@ a.py:1: error: invalid syntax b.py:3: error: Too many arguments for "f" a.py:3: error: Too many arguments for "g" -[case testDeleteFileWithBlockingError] +[case testDeleteFileWithBlockingError-skip] +-- Disabled due to cache/no-cache mismatch: +-- Error message ordering differs import a import b [file a.py] @@ -308,7 +310,10 @@ import blocker2 == a.py:1: error: "int" not callable -[case testFixingBlockingErrorTriggersDeletion1] +[case testFixingBlockingErrorTriggersDeletion1-skip] +-- Disabled due to cache/no-cache mismatch: +-- Cache mode fails to produce the error in the final step, but this is +-- a manifestation of a bug that can occur in no-cache mode also. import a def g(x: a.A) -> None: diff --git a/test-data/unit/fine-grained-cycles.test b/test-data/unit/fine-grained-cycles.test index eeac0b427b00..47129c9c32e1 100644 --- a/test-data/unit/fine-grained-cycles.test +++ b/test-data/unit/fine-grained-cycles.test @@ -174,7 +174,10 @@ def h() -> None: == a.py:1: error: Module 'b' has no attribute 'C' -[case testReferenceToTypeThroughCycleAndReplaceWithFunction] +[case testReferenceToTypeThroughCycleAndReplaceWithFunction-skip] +-- Disabled due to cache/no-cache mismatch: +-- Cache mode has a "need type annotation" message (like coarse incremental does) + import a [file a.py] diff --git a/test-data/unit/fine-grained-modules.test b/test-data/unit/fine-grained-modules.test index 698e863be76b..a4cccc98670d 100644 --- a/test-data/unit/fine-grained-modules.test +++ b/test-data/unit/fine-grained-modules.test @@ -234,7 +234,10 @@ main:1: error: Cannot find module named 'a' main:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) == -[case testDeletionOfSubmoduleTriggersImportFrom1] +[case testDeletionOfSubmoduleTriggersImportFrom1-skip] +-- Disabled due to cache/no-cache mismatch: +-- Cache mode matches the message from regular mode and no-cache mode +-- matches the message from coarse incremental mode... from p import q [file p/__init__.py] [file p/q.py] @@ -730,7 +733,9 @@ a/b.py:3: error: Revealed type is 'Any' == a/b.py:3: error: Unsupported operand types for + ("int" and "str") -[case testDeleteModuleWithinPackageInitIgnored] +[case testDeleteModuleWithinPackageInitIgnored-skip] +-- Disabled due to cache/no-cache mismatch: +-- Having deleted files specified on command line seems dodgy, though. # cmd: mypy x.py a/b.py # flags: --follow-imports=skip --ignore-missing-imports [file x.py] diff --git a/test-data/unit/fine-grained.test b/test-data/unit/fine-grained.test index b97b3eeeb5c4..f6a8e6ac6a16 100644 --- a/test-data/unit/fine-grained.test +++ b/test-data/unit/fine-grained.test @@ -286,7 +286,9 @@ main:5: error: Module has no attribute "B" == main:5: error: Module has no attribute "B" -[case testContinueToReportErrorAtTopLevel] +[case testContinueToReportErrorAtTopLevel-skip] +-- Disabled due to cache/no-cache mismatch: +-- Error message ordering differs import n import m m.A().f() From 39c270917bd84b3ad0634715eab4d9ca40613f00 Mon Sep 17 00:00:00 2001 From: Michael Sullivan Date: Tue, 6 Feb 2018 12:28:08 -0800 Subject: [PATCH 5/8] Some code/comment cleanup --- mypy/build.py | 11 +++++++++-- mypy/dmypy_server.py | 1 - mypy/nodes.py | 3 +++ mypy/server/update.py | 21 ++++++++++++++------- mypy/test/testdmypy.py | 1 + mypy/test/testfinegrained.py | 1 - mypy/test/testmerge.py | 1 + 7 files changed, 28 insertions(+), 11 deletions(-) diff --git a/mypy/build.py b/mypy/build.py index bc3b3a30bfe5..82ae17b9419d 100644 --- a/mypy/build.py +++ b/mypy/build.py @@ -1132,6 +1132,12 @@ def validate_meta(meta: Optional[CacheMeta], id: str, path: Optional[str], manager.log('Metadata abandoned for {}: file {} does not exist'.format(id, path)) return None + # When we are using a fine-grained cache, we want our initial + # build() to load all of the cache information and then do a + # fine-grained incremental update to catch anything that has + # changed since the cache was generated. We *don't* want to do a + # coarse-grained incremental rebuild, so we accept the cache + # metadata even if it doesn't match the source file. if manager.options.use_fine_grained_cache: manager.log('Using potentially stale metadata for {}'.format(id)) return meta @@ -2389,7 +2395,8 @@ def process_graph(graph: Graph, manager: BuildManager) -> None: process_stale_scc(graph, scc, manager) # If we are running in fine-grained incremental mode with caching, - # we need to always process fresh SCCs. + # we always process fresh SCCs so that we have all of the symbol + # tables and fine-grained dependencies available. if manager.options.use_fine_grained_cache: for prev_scc in fresh_scc_queue: process_fresh_scc(graph, prev_scc, manager) @@ -2581,7 +2588,7 @@ def process_stale_scc(graph: Graph, scc: List[str], manager: BuildManager) -> No graph[id].transitive_error = True for id in stale: graph[id].finish_passes() - if manager.options.cache_fine_grained: + if manager.options.cache_fine_grained or manager.options.fine_grained_incremental: graph[id].compute_fine_grained_deps() graph[id].generate_unused_ignore_notes() manager.flush_errors(manager.errors.file_messages(graph[id].xpath), False) diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index 19264944749f..f1f6bae1584a 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -265,7 +265,6 @@ def initialize_fine_grained(self, sources: List[mypy.build.BuildSource]) -> Dict messages = result.errors manager = result.manager graph = result.graph - manager.options.cache_dir = os.devnull # XXX: HACK self.fine_grained_manager = mypy.server.update.FineGrainedBuildManager(manager, graph) self.fine_grained_initialized = True self.previous_sources = sources diff --git a/mypy/nodes.py b/mypy/nodes.py index 6ea5c91a5017..c6c0c412be45 100644 --- a/mypy/nodes.py +++ b/mypy/nodes.py @@ -203,6 +203,8 @@ class MypyFile(SymbolNode): ignored_lines = None # type: Set[int] # Is this file represented by a stub file (.pyi)? is_stub = False + # Is this loaded from the cache and thus missing the actual body of the file? + is_cache_skeleton = False def __init__(self, defs: List[Statement], @@ -249,6 +251,7 @@ def deserialize(cls, data: JsonDict) -> 'MypyFile': tree.names = SymbolTable.deserialize(data['names']) tree.is_stub = data['is_stub'] tree.path = data['path'] + tree.is_cache_skeleton = True return tree diff --git a/mypy/server/update.py b/mypy/server/update.py index ce43f85e9802..e59ae60b0d8a 100644 --- a/mypy/server/update.py +++ b/mypy/server/update.py @@ -170,6 +170,10 @@ def __init__(self, self.blocking_error = None # type: Optional[Tuple[str, str]] # Module that we haven't processed yet but that are known to be stale. self.stale = [] # type: List[Tuple[str, str]] + # Disable the cache so that load_graph doesn't try going back to disk + # 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) self.type_maps = extract_type_maps(graph) @@ -319,9 +323,7 @@ def mark_all_meta_as_memory_only(graph: Dict[str, State], def get_all_dependencies(manager: BuildManager, graph: Dict[str, State], options: Options) -> Dict[str, Set[str]]: """Return the fine-grained dependency map for an entire build.""" - if not options.use_fine_grained_cache: - for state in graph.values(): - state.compute_fine_grained_deps() + # Deps for each module were computed during build() or loaded from the cache. deps = {} # type: Dict[str, Set[str]] collect_dependencies(manager.modules, deps, graph) return deps @@ -717,6 +719,11 @@ def propagate_changes_using_dependencies( triggered: Set[str], up_to_date_modules: Set[str], targets_with_errors: Set[str]) -> List[Tuple[str, str]]: + """Transitively rechecks targets based on triggers and the dependency map. + + Returns a list (module id, path) tuples representing modules that contain + a target that needs to be reprocessed but that has not been parsed yet.""" + # TODO: Multiple type checking passes num_iter = 0 remaining_modules = [] @@ -743,10 +750,10 @@ def propagate_changes_using_dependencies( # TODO: Preserve order (set is not optimal) for id, nodes in sorted(todo.items(), key=lambda x: x[0]): assert id not in up_to_date_modules - # TODO: Is there a better way to detect that the file isn't loaded? - if not manager.modules[id].defs: - # We haven't actually loaded this file! Add it to the - # queue of files that need to be processed fully. + if manager.modules[id].is_cache_skeleton: + # We have only loaded the cache for this file, not the actual file, + # so we can't access the nodes to reprocess. + # Add it to the queue of files that need to be processed fully. remaining_modules.append((id, manager.modules[id].path)) else: triggered |= reprocess_nodes(manager, graph, id, nodes, deps) diff --git a/mypy/test/testdmypy.py b/mypy/test/testdmypy.py index ef2f87e64d81..6423b2ee06ef 100644 --- a/mypy/test/testdmypy.py +++ b/mypy/test/testdmypy.py @@ -120,6 +120,7 @@ def run_case_once(self, testcase: DataDrivenTestCase, incremental_step: int) -> server_options = [] # type: List[str] if 'fine-grained' in testcase.file: server_options.append('--experimental') + options.fine_grained_incremental = True self.server = dmypy_server.Server(server_options) # TODO: Fix ugly API self.server.options = options diff --git a/mypy/test/testfinegrained.py b/mypy/test/testfinegrained.py index 261749fa49e8..7224d192eba6 100644 --- a/mypy/test/testfinegrained.py +++ b/mypy/test/testfinegrained.py @@ -91,7 +91,6 @@ def run_case_inner(self, testcase: DataDrivenTestCase, cache: bool) -> None: if fine_grained_manager is None: messages, manager, graph = self.build(main_src, testcase, sources_override, build_cache=False, enable_cache=cache) - manager.options.cache_dir = os.devnull # XXX: HACK fine_grained_manager = FineGrainedBuildManager(manager, graph) new_messages = fine_grained_manager.update(modules) diff --git a/mypy/test/testmerge.py b/mypy/test/testmerge.py index 7cbf8041b78a..d51288ced3cf 100644 --- a/mypy/test/testmerge.py +++ b/mypy/test/testmerge.py @@ -99,6 +99,7 @@ def run_case(self, testcase: DataDrivenTestCase) -> None: def build(self, source: str) -> Tuple[List[str], Optional[BuildManager], Dict[str, State]]: options = Options() options.incremental = True + options.fine_grained_incremental = True options.use_builtins_fixtures = True options.show_traceback = True main_path = os.path.join(test_temp_dir, 'main') From 22dd19a59f28d66fa5ca2051feb5727dc66ce3db Mon Sep 17 00:00:00 2001 From: Michael Sullivan Date: Tue, 6 Feb 2018 12:20:49 -0800 Subject: [PATCH 6/8] Make the fg-cache test a separate suite. --- mypy/test/testfinegrained.py | 45 ++++++++++++++--------- mypy/test/testfinegrainedcache.py | 12 ++++++ runtests.py | 1 + test-data/unit/fine-grained-blockers.test | 29 +++++++++++++-- test-data/unit/fine-grained-cycles.test | 39 +++++++++++++++++++- test-data/unit/fine-grained-modules.test | 23 ++++++++++-- test-data/unit/fine-grained.test | 29 ++++++++++++++- 7 files changed, 148 insertions(+), 30 deletions(-) create mode 100644 mypy/test/testfinegrainedcache.py diff --git a/mypy/test/testfinegrained.py b/mypy/test/testfinegrained.py index 7224d192eba6..1adc013bc596 100644 --- a/mypy/test/testfinegrained.py +++ b/mypy/test/testfinegrained.py @@ -30,6 +30,7 @@ from mypy.test.testtypegen import ignore_node from mypy.types import TypeStrVisitor, Type from mypy.util import short_type +import pytest # type: ignore # no pytest in typeshed class FineGrainedSuite(DataSuite): @@ -41,31 +42,40 @@ class FineGrainedSuite(DataSuite): ] base_path = test_temp_dir optional_out = True + # Whether to use the fine-grained cache in the testing. This is overridden + # by a trivial subclass to produce a suite that uses the cache. + use_cache = False + + # Decide whether to skip the test. This could have been structured + # as a filter() classmethod also, but we want the tests reported + # as skipped, not just elided. + def should_skip(self, testcase: DataDrivenTestCase) -> bool: + if self.use_cache: + if testcase.name.endswith("-skip-cache"): return True + # TODO: In caching mode we currently don't well support + # starting from cached states with errors in them. + if testcase.output and testcase.output[0] != '==': return True + else: + if testcase.name.endswith("-skip-nocache"): return True - def run_case(self, testcase: DataDrivenTestCase) -> None: - self.run_case_inner(testcase, cache=False) - - # Reset the test case and run it again with caching on - testcase.teardown() - testcase.setup() - self.run_case_inner(testcase, cache=True) + return False - def run_case_inner(self, testcase: DataDrivenTestCase, cache: bool) -> None: - # TODO: In caching mode we currently don't well support - # starting from cached states with errors in them. - if cache and testcase.output and testcase.output[0] != '==': return + def run_case(self, testcase: DataDrivenTestCase) -> None: + if self.should_skip(testcase): + pytest.skip() + return main_src = '\n'.join(testcase.input) sources_override = self.parse_sources(main_src) - print("Testing with cache: ", cache) messages, manager, graph = self.build(main_src, testcase, sources_override, - build_cache=cache, enable_cache=cache) + build_cache=self.use_cache, + enable_cache=self.use_cache) a = [] if messages: a.extend(normalize_messages(messages)) fine_grained_manager = None - if not cache: + if not self.use_cache: fine_grained_manager = FineGrainedBuildManager(manager, graph) steps = testcase.find_steps() @@ -90,7 +100,7 @@ def run_case_inner(self, testcase: DataDrivenTestCase, cache: bool) -> None: # cache, now we need to set it up if fine_grained_manager is None: messages, manager, graph = self.build(main_src, testcase, sources_override, - build_cache=False, enable_cache=cache) + build_cache=False, enable_cache=True) fine_grained_manager = FineGrainedBuildManager(manager, graph) new_messages = fine_grained_manager.update(modules) @@ -103,11 +113,10 @@ def run_case_inner(self, testcase: DataDrivenTestCase, cache: bool) -> None: # Normalize paths in test output (for Windows). a = [line.replace('\\', '/') for line in a] - modestr = "in cache mode " if cache else "" assert_string_arrays_equal( testcase.output, a, - 'Invalid output {}({}, line {})'.format( - modestr, testcase.file, testcase.line)) + 'Invalid output ({}, line {})'.format( + testcase.file, testcase.line)) if testcase.triggered: assert_string_arrays_equal( diff --git a/mypy/test/testfinegrainedcache.py b/mypy/test/testfinegrainedcache.py new file mode 100644 index 000000000000..78701016ef22 --- /dev/null +++ b/mypy/test/testfinegrainedcache.py @@ -0,0 +1,12 @@ +"""Tests for fine-grained incremental checking using the cache. + +All of the real code for this lives in testfinegrained.py. +""" + +# We can't "import FineGrainedSuite from ..." because that will cause pytest +# to collect the non-caching tests when running this file. +import mypy.test.testfinegrained + + +class FineGrainedCacheSuite(mypy.test.testfinegrained.FineGrainedSuite): + use_cache = True diff --git a/runtests.py b/runtests.py index e9d9a000c695..e663aea8400b 100755 --- a/runtests.py +++ b/runtests.py @@ -209,6 +209,7 @@ def test_path(*names: str): 'testdeps', 'testdiff', 'testfinegrained', + 'testfinegrainedcache', 'testmerge', 'testtransform', 'testtypegen', diff --git a/test-data/unit/fine-grained-blockers.test b/test-data/unit/fine-grained-blockers.test index ccd0879fd7fe..3a746e1ea367 100644 --- a/test-data/unit/fine-grained-blockers.test +++ b/test-data/unit/fine-grained-blockers.test @@ -237,8 +237,8 @@ a.py:1: error: invalid syntax b.py:3: error: Too many arguments for "f" a.py:3: error: Too many arguments for "g" -[case testDeleteFileWithBlockingError-skip] --- Disabled due to cache/no-cache mismatch: +[case testDeleteFileWithBlockingError-skip-cache] +-- Different cache/no-cache tests because: -- Error message ordering differs import a import b @@ -258,6 +258,27 @@ main:1: error: Cannot find module named 'a' main:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) b.py:1: error: Cannot find module named 'a' +[case testDeleteFileWithBlockingError-skip-nocache] +-- Different cache/no-cache tests because: +-- Error message ordering differs +import a +import b +[file a.py] +def f() -> None: pass +[file b.py] +import a +a.f() +[file a.py.2] +x x +[delete a.py.3] +[out] +== +a.py:1: error: invalid syntax +== +b.py:1: error: Cannot find module named 'a' +b.py:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) +main:1: error: Cannot find module named 'a' + [case testModifyFileWhileBlockingErrorElsewhere] import a import b @@ -310,8 +331,8 @@ import blocker2 == a.py:1: error: "int" not callable -[case testFixingBlockingErrorTriggersDeletion1-skip] --- Disabled due to cache/no-cache mismatch: +[case testFixingBlockingErrorTriggersDeletion1-skip-cache] +-- Disabled in cache mdode: -- Cache mode fails to produce the error in the final step, but this is -- a manifestation of a bug that can occur in no-cache mode also. import a diff --git a/test-data/unit/fine-grained-cycles.test b/test-data/unit/fine-grained-cycles.test index 47129c9c32e1..737d0ed0d4eb 100644 --- a/test-data/unit/fine-grained-cycles.test +++ b/test-data/unit/fine-grained-cycles.test @@ -174,8 +174,8 @@ def h() -> None: == a.py:1: error: Module 'b' has no attribute 'C' -[case testReferenceToTypeThroughCycleAndReplaceWithFunction-skip] --- Disabled due to cache/no-cache mismatch: +[case testReferenceToTypeThroughCycleAndReplaceWithFunction-skip-cache] +-- Different cache/no-cache tests because: -- Cache mode has a "need type annotation" message (like coarse incremental does) import a @@ -208,6 +208,41 @@ def h() -> None: == 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) + +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" +b.py:6: error: Need type annotation for 'c' + -- TODO: More import cycle: -- -- * "from x import y" through cycle diff --git a/test-data/unit/fine-grained-modules.test b/test-data/unit/fine-grained-modules.test index a4cccc98670d..43f13657a6b3 100644 --- a/test-data/unit/fine-grained-modules.test +++ b/test-data/unit/fine-grained-modules.test @@ -234,8 +234,8 @@ main:1: error: Cannot find module named 'a' main:1: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help) == -[case testDeletionOfSubmoduleTriggersImportFrom1-skip] --- Disabled due to cache/no-cache mismatch: +[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... from p import q @@ -250,6 +250,21 @@ 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... +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: Module 'p' has no attribute 'q' +== + + [case testDeletionOfSubmoduleTriggersImportFrom2] from p.q import f f() @@ -733,8 +748,8 @@ a/b.py:3: error: Revealed type is 'Any' == a/b.py:3: error: Unsupported operand types for + ("int" and "str") -[case testDeleteModuleWithinPackageInitIgnored-skip] --- Disabled due to cache/no-cache mismatch: +[case testDeleteModuleWithinPackageInitIgnored-skip-cache] +-- Disabled in cache mode because incorrect behavior: -- Having deleted files specified on command line seems dodgy, though. # cmd: mypy x.py a/b.py # flags: --follow-imports=skip --ignore-missing-imports diff --git a/test-data/unit/fine-grained.test b/test-data/unit/fine-grained.test index f6a8e6ac6a16..a0838915910d 100644 --- a/test-data/unit/fine-grained.test +++ b/test-data/unit/fine-grained.test @@ -286,8 +286,8 @@ main:5: error: Module has no attribute "B" == main:5: error: Module has no attribute "B" -[case testContinueToReportErrorAtTopLevel-skip] --- Disabled due to cache/no-cache mismatch: +[case testContinueToReportErrorAtTopLevel-skip-cache] +-- Different cache/no-cache tests because: -- Error message ordering differs import n import m @@ -311,6 +311,31 @@ n.py:2: error: "A" has no attribute "g" == n.py:2: error: "A" has no attribute "g" +[case testContinueToReportErrorAtTopLevel-skip-nocache] +-- Different cache/no-cache tests because: +-- Error message ordering differs +import n +import m +m.A().f() +[file n.py] +import m +m.A().g() +[file m.py] +class A: + def f(self) -> None: pass + def g(self) -> None: pass +[file m.py.2] +class A: pass +[file m.py.3] +class A: + def f(self) -> None: pass +[out] +== +n.py:2: error: "A" has no attribute "g" +main:3: error: "A" has no attribute "f" +== +n.py:2: error: "A" has no attribute "g" + [case testContinueToReportErrorInMethod] import m class C: From bbd49a23129bd54133188e6df312306c51b5b576 Mon Sep 17 00:00:00 2001 From: Michael Sullivan Date: Tue, 6 Feb 2018 16:18:50 -0800 Subject: [PATCH 7/8] Avoid needing to specify --cache-fine-grained with --use-fine-grained-cache --- mypy/dmypy_server.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/mypy/dmypy_server.py b/mypy/dmypy_server.py index f1f6bae1584a..5428ad7969a0 100644 --- a/mypy/dmypy_server.py +++ b/mypy/dmypy_server.py @@ -107,7 +107,9 @@ def __init__(self, flags: List[str]) -> None: if self.fine_grained: options.incremental = True options.show_traceback = True - if not options.use_fine_grained_cache: + if options.use_fine_grained_cache: + options.cache_fine_grained = True # set this so that cache options match + else: options.cache_dir = os.devnull def serve(self) -> None: From 376db149b44b901a668753aa876c4d9e13238bf7 Mon Sep 17 00:00:00 2001 From: Michael Sullivan Date: Wed, 7 Feb 2018 10:44:30 -0800 Subject: [PATCH 8/8] style tweaks --- mypy/test/testfinegrained.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/mypy/test/testfinegrained.py b/mypy/test/testfinegrained.py index 1adc013bc596..5cd5c04320ae 100644 --- a/mypy/test/testfinegrained.py +++ b/mypy/test/testfinegrained.py @@ -51,12 +51,15 @@ class FineGrainedSuite(DataSuite): # as skipped, not just elided. def should_skip(self, testcase: DataDrivenTestCase) -> bool: if self.use_cache: - if testcase.name.endswith("-skip-cache"): return True + if testcase.name.endswith("-skip-cache"): + return True # TODO: In caching mode we currently don't well support # starting from cached states with errors in them. - if testcase.output and testcase.output[0] != '==': return True + if testcase.output and testcase.output[0] != '==': + return True else: - if testcase.name.endswith("-skip-nocache"): return True + if testcase.name.endswith("-skip-nocache"): + return True return False