Skip to content

Commit 4b76823

Browse files
committed
Modify incremental to check public interface
This commit finally enables the build process to throttle back incremental mode when the public interface of a method is not changed.
1 parent 4af183b commit 4b76823

File tree

1 file changed

+62
-10
lines changed

1 file changed

+62
-10
lines changed

mypy/build.py

Lines changed: 62 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,6 +1022,9 @@ class State:
10221022
# If caller_state is set, the line number in the caller where the import occurred
10231023
caller_line = 0
10241024

1025+
# If True, indicates that the public interface is unchanged.
1026+
externally_same = False
1027+
10251028
# If True, indicates that module must be checked later to see if it is
10261029
# stale and if the public interface has been changed.
10271030
maybe_stale = False
@@ -1122,6 +1125,24 @@ def __init__(self,
11221125
self.suppressed = []
11231126
self.child_modules = set()
11241127

1128+
def check_interface(self) -> None:
1129+
if self.has_new_submodules():
1130+
self.manager.trace("Module {} has new submodules".format(self.id))
1131+
self.parse_file()
1132+
self.externally_same = False
1133+
elif self.maybe_stale:
1134+
self.parse_file()
1135+
if self.interface_hash == self.meta.interface_hash:
1136+
self.manager.trace("Module {} was changed but has same interface".format(self.id))
1137+
self.externally_same = True
1138+
self.meta = None
1139+
else:
1140+
self.manager.trace("Module {} has different interface".format(self.id))
1141+
self.externally_same = False
1142+
elif self.meta is not None:
1143+
self.manager.trace("Module {} is unchanged".format(self.id))
1144+
self.externally_same = True
1145+
11251146
def skipping_ancestor(self, id: str, path: str, ancestor_for: 'State') -> None:
11261147
# TODO: Read the path (the __init__.py file) and return
11271148
# immediately if it's empty or only contains comments.
@@ -1168,16 +1189,28 @@ def is_fresh(self) -> bool:
11681189
# suppression by --silent-imports. However when a suppressed
11691190
# dependency is added back we find out later in the process.
11701191
return (self.meta is not None
1192+
and self.externally_same
11711193
and self.dependencies == self.meta.dependencies
11721194
and self.child_modules == set(self.meta.child_modules))
11731195

1196+
def is_interface_fresh(self) -> bool:
1197+
return self.externally_same
1198+
11741199
def has_new_submodules(self) -> bool:
11751200
"""Return if this module has new submodules after being loaded from a warm cache."""
11761201
return self.meta is not None and self.child_modules != set(self.meta.child_modules)
11771202

1178-
def mark_stale(self) -> None:
1179-
"""Throw away the cache data for this file, marking it as stale."""
1203+
def mark_stale(self, interface_is_same=False) -> None:
1204+
"""Throw away the cache data for this file, marking it as stale.
1205+
1206+
If interface_is_same is True, treat the module's interface as being
1207+
fresh -- only the module's content should be considered stale."""
1208+
if interface_is_same:
1209+
self.manager.trace("Marking {} as stale (but interface is fresh)".format(self.id))
1210+
else:
1211+
self.manager.trace("Marking {} as stale (interface is also stale)".format(self.id))
11801212
self.meta = None
1213+
self.externally_same = interface_is_same
11811214
self.manager.stale_modules.add(self.id)
11821215

11831216
def check_blockers(self) -> None:
@@ -1397,8 +1430,7 @@ def load_graph(sources: List[BuildSource], manager: BuildManager) -> Graph:
13971430
if dep in st.ancestors and dep in graph:
13981431
graph[dep].child_modules.add(st.id)
13991432
for id, g in graph.items():
1400-
if g.has_new_submodules():
1401-
g.parse_file()
1433+
g.check_interface()
14021434
return graph
14031435

14041436

@@ -1437,8 +1469,24 @@ def process_graph(graph: Graph, manager: BuildManager) -> None:
14371469
for id in scc:
14381470
deps.update(graph[id].dependencies)
14391471
deps -= ascc
1440-
stale_deps = {id for id in deps if not graph[id].is_fresh()}
1472+
stale_deps = {id for id in deps if not graph[id].is_interface_fresh()}
14411473
fresh = fresh and not stale_deps
1474+
1475+
# TODO: Currently, the interface-based approach to incremental mode will regard
1476+
# absolutely any interface changes in our dependencies as a sign that our
1477+
# interface has also changed. This is often over-conservative, but is
1478+
# an easy way of making sure we preserve correctness.
1479+
#
1480+
# This unfortunately does mean that an interface change will often result back
1481+
# to the worst-case behavior for incremental mode -- changing an interface causes
1482+
# a cascade of changes through a large subset of the import graph.
1483+
#
1484+
# The ideal behavior would be for an interface change to propagate only only one
1485+
# or two levels through the import DAG, but this requires us to track dependencies
1486+
# on a more finer-grained level then we currently do.
1487+
interface_stale_scc = {id for id in scc if not graph[id].is_interface_fresh()}
1488+
interface_fresh = len(interface_stale_scc) == 0 and len(stale_deps) == 0
1489+
14421490
undeps = set()
14431491
if fresh:
14441492
# Check if any dependencies that were suppressed according
@@ -1453,9 +1501,10 @@ def process_graph(graph: Graph, manager: BuildManager) -> None:
14531501
# All cache files are fresh. Check that no dependency's
14541502
# cache file is newer than any scc node's cache file.
14551503
oldest_in_scc = min(graph[id].meta.data_mtime for id in scc)
1456-
newest_in_deps = 0 if not deps else max(graph[dep].meta.data_mtime for dep in deps)
1504+
viable = {id for id in deps if not graph[id].is_interface_fresh()}
1505+
newest_in_deps = max(graph[dep].meta.data_mtime for dep in viable) if viable else 0
14571506
if manager.options.verbosity >= 3: # Dump all mtimes for extreme debugging.
1458-
all_ids = sorted(ascc | deps, key=lambda id: graph[id].meta.data_mtime)
1507+
all_ids = sorted(ascc | viable, key=lambda id: graph[id].meta.data_mtime)
14591508
for id in all_ids:
14601509
if id in scc:
14611510
if graph[id].meta.data_mtime < newest_in_deps:
@@ -1481,17 +1530,20 @@ def process_graph(graph: Graph, manager: BuildManager) -> None:
14811530
fresh_msg = "inherently stale (%s)" % " ".join(sorted(stale_scc))
14821531
if stale_deps:
14831532
fresh_msg += " with stale deps (%s)" % " ".join(sorted(stale_deps))
1533+
if interface_fresh:
1534+
fresh_msg += ", but interface is fresh"
14841535
else:
14851536
fresh_msg = "stale due to deps (%s)" % " ".join(sorted(stale_deps))
14861537
if len(scc) == 1:
14871538
manager.log("Processing SCC singleton (%s) as %s" % (" ".join(scc), fresh_msg))
14881539
else:
14891540
manager.log("Processing SCC of size %d (%s) as %s" %
14901541
(len(scc), " ".join(scc), fresh_msg))
1542+
14911543
if fresh:
14921544
process_fresh_scc(graph, scc)
14931545
else:
1494-
process_stale_scc(graph, scc)
1546+
process_stale_scc(graph, scc, interface_fresh)
14951547

14961548

14971549
def order_ascc(graph: Graph, ascc: AbstractSet[str], pri_max: int = PRI_ALL) -> List[str]:
@@ -1553,10 +1605,10 @@ def process_fresh_scc(graph: Graph, scc: List[str]) -> None:
15531605
graph[id].calculate_mros()
15541606

15551607

1556-
def process_stale_scc(graph: Graph, scc: List[str]) -> None:
1608+
def process_stale_scc(graph: Graph, scc: List[str], is_interface_fresh: bool) -> None:
15571609
"""Process the modules in one SCC from source code."""
15581610
for id in scc:
1559-
graph[id].mark_stale()
1611+
graph[id].mark_stale(is_interface_fresh)
15601612
for id in scc:
15611613
# We may already have parsed the module, or not.
15621614
# If the former, parse_file() is a no-op.

0 commit comments

Comments
 (0)