Skip to content

Commit 4cc3292

Browse files
committed
perf: avoid needless sql operations. #1538
If the set of arcs is empty, skip the SQL operations. We also need to allow setting a file tracer for an unmeasured file, to avoid the Cython problem whose fix caused the performance issue in the first place. TBH, I don't know why we had to prevent file tracers on unmeasured files. Perhaps pytest-cov has changed to avoid the behavior that caused problems.
1 parent 674204f commit 4cc3292

File tree

3 files changed

+23
-20
lines changed

3 files changed

+23
-20
lines changed

Diff for: CHANGES.rst

+8-1
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,20 @@ development at the same time, such as 4.5.x and 5.0.
2020
Unreleased
2121
----------
2222

23+
- Performance: fixed a slow-down with dynamic contexts that appeared in 6.4.3,
24+
closing `issue 1538`_. Hopefully this doesn't break the `Cython change`_
25+
that fixed `issue 972`_. Thanks to Mathieu Kniewallner for the deep
26+
investigative work and comprehensive issue report.
27+
2328
- Added: the debug output file can now be specified with ``[run] debug_file``
2429
in the configuration file. Closes `issue 1319`_.
2530

2631
- Typing: all product and test code has type annotations.
2732

33+
.. _Cython change: https://github.com/nedbat/coveragepy/pull/1347
34+
.. _issue 972: https://github.com/nedbat/coveragepy/issues/972
2835
.. _issue 1319: https://github.com/nedbat/coveragepy/issues/1319
29-
36+
.. _issue 1538: https://github.com/nedbat/coveragepy/issues/1538
3037

3138
.. scriv-start-here
3239

Diff for: coverage/sqldata.py

+7-9
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,8 @@ def add_arcs(self, arc_data: Mapping[str, Collection[TArc]]) -> None:
528528
with self._connect() as con:
529529
self._set_context_id()
530530
for filename, arcs in arc_data.items():
531+
if not arcs:
532+
continue
531533
file_id = self._file_id(filename, add=True)
532534
data = [(file_id, self._current_context_id, fromno, tono) for fromno, tono in arcs]
533535
con.executemany_void(
@@ -571,12 +573,7 @@ def add_file_tracers(self, file_tracers: Mapping[str, str]) -> None:
571573
self._start_using()
572574
with self._connect() as con:
573575
for filename, plugin_name in file_tracers.items():
574-
file_id = self._file_id(filename)
575-
if file_id is None:
576-
raise DataError(
577-
f"Can't add file tracer data for unmeasured file '{filename}'"
578-
)
579-
576+
file_id = self._file_id(filename, add=True)
580577
existing_plugin = self.file_tracer(filename)
581578
if existing_plugin:
582579
if existing_plugin != plugin_name:
@@ -1213,10 +1210,9 @@ def execute_one(self, sql: str, parameters: Iterable[Any] = ()) -> Optional[Tupl
12131210
else:
12141211
raise AssertionError(f"SQL {sql!r} shouldn't return {len(rows)} rows")
12151212

1216-
def _executemany(self, sql: str, data: Iterable[Any]) -> sqlite3.Cursor:
1213+
def _executemany(self, sql: str, data: List[Any]) -> sqlite3.Cursor:
12171214
"""Same as :meth:`python:sqlite3.Connection.executemany`."""
12181215
if self.debug.should("sql"):
1219-
data = list(data)
12201216
final = ":" if self.debug.should("sqldata") else ""
12211217
self.debug.write(f"Executing many {sql!r} with {len(data)} rows{final}")
12221218
if self.debug.should("sqldata"):
@@ -1233,7 +1229,9 @@ def _executemany(self, sql: str, data: Iterable[Any]) -> sqlite3.Cursor:
12331229

12341230
def executemany_void(self, sql: str, data: Iterable[Any]) -> None:
12351231
"""Same as :meth:`python:sqlite3.Connection.executemany` when you don't need the cursor."""
1236-
self._executemany(sql, data).close()
1232+
data = list(data)
1233+
if data:
1234+
self._executemany(sql, data).close()
12371235

12381236
def executescript(self, script: str) -> None:
12391237
"""Same as :meth:`python:sqlite3.Connection.executescript`."""

Diff for: tests/test_data.py

+8-10
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,14 @@ def test_ok_to_add_arcs_twice(self) -> None:
187187
assert_line_counts(covdata, SUMMARY_3_4)
188188
assert_measured_files(covdata, MEASURED_FILES_3_4)
189189

190+
def test_ok_to_add_empty_arcs(self) -> None:
191+
covdata = DebugCoverageData()
192+
covdata.add_arcs(ARCS_3)
193+
covdata.add_arcs(ARCS_4)
194+
covdata.add_arcs(dict.fromkeys(ARCS_3, set()))
195+
assert_line_counts(covdata, SUMMARY_3_4)
196+
assert_measured_files(covdata, MEASURED_FILES_3_4)
197+
190198
@pytest.mark.parametrize("klass", [CoverageData, DebugCoverageData])
191199
def test_cant_add_arcs_with_lines(self, klass: TCoverageData) -> None:
192200
covdata = klass()
@@ -350,16 +358,6 @@ def test_ok_to_set_empty_file_tracer(self) -> None:
350358
assert covdata.file_tracer("p1.foo") == "p1.plugin"
351359
assert covdata.file_tracer("main.py") == ""
352360

353-
def test_cant_file_tracer_unmeasured_files(self) -> None:
354-
covdata = DebugCoverageData()
355-
msg = "Can't add file tracer data for unmeasured file 'p1.foo'"
356-
with pytest.raises(DataError, match=msg):
357-
covdata.add_file_tracers({"p1.foo": "p1.plugin"})
358-
359-
covdata.add_lines({"p2.html": [10, 11, 12]})
360-
with pytest.raises(DataError, match=msg):
361-
covdata.add_file_tracers({"p1.foo": "p1.plugin"})
362-
363361
def test_cant_change_file_tracer_name(self) -> None:
364362
covdata = DebugCoverageData()
365363
covdata.add_lines({"p1.foo": [1, 2, 3]})

0 commit comments

Comments
 (0)