diff options
author | Ned Batchelder <ned@nedbatchelder.com> | 2023-01-23 19:44:41 -0500 |
---|---|---|
committer | Ned Batchelder <ned@nedbatchelder.com> | 2023-01-24 19:23:48 -0500 |
commit | 4cc32922685c6971275f522304b3754ad1a233c1 (patch) | |
tree | 59ab9a76180a22a20fbd34b5d2188191daee4c0b | |
parent | 674204fc958f30815fe482fe1ed36d01eb74c489 (diff) | |
download | python-coveragepy-git-4cc32922685c6971275f522304b3754ad1a233c1.tar.gz |
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.
-rw-r--r-- | CHANGES.rst | 9 | ||||
-rw-r--r-- | coverage/sqldata.py | 16 | ||||
-rw-r--r-- | tests/test_data.py | 18 |
3 files changed, 23 insertions, 20 deletions
diff --git a/CHANGES.rst b/CHANGES.rst index 185f20bf..745564c5 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -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 diff --git a/coverage/sqldata.py b/coverage/sqldata.py index da66ad09..1cb8abe4 100644 --- a/coverage/sqldata.py +++ b/coverage/sqldata.py @@ -528,6 +528,8 @@ class CoverageData(AutoReprMixin): 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( @@ -571,12 +573,7 @@ class CoverageData(AutoReprMixin): 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: @@ -1213,10 +1210,9 @@ class SqliteDb(AutoReprMixin): 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"): @@ -1233,7 +1229,9 @@ class SqliteDb(AutoReprMixin): 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`.""" diff --git a/tests/test_data.py b/tests/test_data.py index ab4e03f7..5953ba36 100644 --- a/tests/test_data.py +++ b/tests/test_data.py @@ -187,6 +187,14 @@ class CoverageDataTest(CoverageTest): 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() @@ -350,16 +358,6 @@ class CoverageDataTest(CoverageTest): 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]}) |