Skip to content

Commit

Permalink
perf: avoid needless sql operations. #1538
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
nedbat committed Jan 25, 2023
1 parent 674204f commit 4cc3292
Show file tree
Hide file tree
Showing 3 changed files with 23 additions and 20 deletions.
9 changes: 8 additions & 1 deletion CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,20 @@ development at the same time, such as 4.5.x and 5.0.
Unreleased
----------

- Performance: fixed a slow-down with dynamic contexts that appeared in 6.4.3,
closing `issue 1538`_. Hopefully this doesn't break the `Cython change`_
that fixed `issue 972`_. Thanks to Mathieu Kniewallner for the deep
investigative work and comprehensive issue report.

- Added: the debug output file can now be specified with ``[run] debug_file``
in the configuration file. Closes `issue 1319`_.

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

.. _Cython change: https://github.com/nedbat/coveragepy/pull/1347
.. _issue 972: https://github.com/nedbat/coveragepy/issues/972
.. _issue 1319: https://github.com/nedbat/coveragepy/issues/1319

.. _issue 1538: https://github.com/nedbat/coveragepy/issues/1538

.. scriv-start-here
Expand Down
16 changes: 7 additions & 9 deletions coverage/sqldata.py
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,8 @@ def add_arcs(self, arc_data: Mapping[str, Collection[TArc]]) -> None:
with self._connect() as con:
self._set_context_id()
for filename, arcs in arc_data.items():
if not arcs:
continue
file_id = self._file_id(filename, add=True)
data = [(file_id, self._current_context_id, fromno, tono) for fromno, tono in arcs]
con.executemany_void(
Expand Down Expand Up @@ -571,12 +573,7 @@ def add_file_tracers(self, file_tracers: Mapping[str, str]) -> None:
self._start_using()
with self._connect() as con:
for filename, plugin_name in file_tracers.items():
file_id = self._file_id(filename)
if file_id is None:
raise DataError(
f"Can't add file tracer data for unmeasured file '{filename}'"
)

file_id = self._file_id(filename, add=True)
existing_plugin = self.file_tracer(filename)
if existing_plugin:
if existing_plugin != plugin_name:
Expand Down Expand Up @@ -1213,10 +1210,9 @@ def execute_one(self, sql: str, parameters: Iterable[Any] = ()) -> Optional[Tupl
else:
raise AssertionError(f"SQL {sql!r} shouldn't return {len(rows)} rows")

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

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

def executescript(self, script: str) -> None:
"""Same as :meth:`python:sqlite3.Connection.executescript`."""
Expand Down
18 changes: 8 additions & 10 deletions tests/test_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,14 @@ def test_ok_to_add_arcs_twice(self) -> None:
assert_line_counts(covdata, SUMMARY_3_4)
assert_measured_files(covdata, MEASURED_FILES_3_4)

def test_ok_to_add_empty_arcs(self) -> None:
covdata = DebugCoverageData()
covdata.add_arcs(ARCS_3)
covdata.add_arcs(ARCS_4)
covdata.add_arcs(dict.fromkeys(ARCS_3, set()))
assert_line_counts(covdata, SUMMARY_3_4)
assert_measured_files(covdata, MEASURED_FILES_3_4)

@pytest.mark.parametrize("klass", [CoverageData, DebugCoverageData])
def test_cant_add_arcs_with_lines(self, klass: TCoverageData) -> None:
covdata = klass()
Expand Down Expand Up @@ -350,16 +358,6 @@ def test_ok_to_set_empty_file_tracer(self) -> None:
assert covdata.file_tracer("p1.foo") == "p1.plugin"
assert covdata.file_tracer("main.py") == ""

def test_cant_file_tracer_unmeasured_files(self) -> None:
covdata = DebugCoverageData()
msg = "Can't add file tracer data for unmeasured file 'p1.foo'"
with pytest.raises(DataError, match=msg):
covdata.add_file_tracers({"p1.foo": "p1.plugin"})

covdata.add_lines({"p2.html": [10, 11, 12]})
with pytest.raises(DataError, match=msg):
covdata.add_file_tracers({"p1.foo": "p1.plugin"})

def test_cant_change_file_tracer_name(self) -> None:
covdata = DebugCoverageData()
covdata.add_lines({"p1.foo": [1, 2, 3]})
Expand Down

0 comments on commit 4cc3292

Please sign in to comment.