Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

COVERAGE_CORE=sysmon with branch coverage can be 2x slower than default in some cases #1812

Open
lesteve opened this issue Jul 10, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@lesteve
Copy link

lesteve commented Jul 10, 2024

Describe the bug
Using COVERAGE_CORE=sysmon with branch coverage can be 2x slower than default. Note the 2x likely depends on the code whose coverage is measured. Trying different variations of the Python snippet below I have seen vary from ~20% to 2x degradation roughly.

To Reproduce

Python script:

# test-coverage.py
from sklearn.datasets import load_breast_cancer
from sklearn.linear_model import LogisticRegression
from sklearn.model_selection import TunedThresholdClassifierCV

X, y = load_breast_cancer(return_X_y=True)
for i in range(10):
    model = TunedThresholdClassifierCV(
        estimator=LogisticRegression()
    ).fit(X, y)

Script to run:

# Just for completeness you need to have Python 3.12 to be able to use sysmon
pip install scikit-learn coverage
git clone https://github.com/scikit-learn/scikit-learn --depth 1

# Note: there are some scikit-learn specific warnings, so the grep is to keep only the timing info
(time coverage run --branch --source sklearn /tmp/test-coverage.py 2>&1) | grep total
# on my machine: 9.75s user 0.09s system 129% cpu 7.616 total

(time COVERAGE_CORE=sysmon coverage run --branch --source sklearn /tmp/test-coverage.py 2>&1) | grep total
# on my machine: 16.88s user 0.10s system 114% cpu 14.798 total

I read the other performance issues with coverage and Python 3.12 for example #1665 and python/cpython#107674 and this seems like a slightly different issue. This may be related to the fact that only statement coverage is using sysmon and not branch coverage according to #1746 (comment) but I still find the performance degradation somewhat surprising.

How can we reproduce the problem? Please be specific. Don't link to a failing CI job. Answer the questions below:

  1. What version of Python are you using? 3.12.4
  2. What version of coverage.py shows the problem? The output of coverage debug sys is helpful.
-- sys -------------------------------------------------------
               coverage_version: 7.5.4
                coverage_module: /home/lesteve/micromamba/envs/test-coverage/lib/python3.12/site-packages/coverage/__init__.py
                           core: -none-
                        CTracer: available
           plugins.file_tracers: -none-
            plugins.configurers: -none-
      plugins.context_switchers: -none-
              configs_attempted: /home/lesteve/dev/cpython/.coveragerc
                                 /home/lesteve/dev/cpython/setup.cfg
                                 /home/lesteve/dev/cpython/tox.ini
                                 /home/lesteve/dev/cpython/pyproject.toml
                   configs_read: -none-
                    config_file: None
                config_contents: -none-
                      data_file: -none-
                         python: 3.12.4 | packaged by conda-forge | (main, Jun 17 2024, 10:23:07) [GCC 12.3.0]
                       platform: Linux-6.9.8-arch1-1-x86_64-with-glibc2.39
                 implementation: CPython
                    gil_enabled: True
                     executable: /home/lesteve/micromamba/envs/test-coverage/bin/python3.12
                   def_encoding: utf-8
                    fs_encoding: utf-8
                            pid: 510639
                            cwd: /home/lesteve/dev/cpython
                           path: /home/lesteve/micromamba/envs/test-coverage/bin
                                 /home/lesteve/micromamba/envs/test-coverage/lib/python312.zip
                                 /home/lesteve/micromamba/envs/test-coverage/lib/python3.12
                                 /home/lesteve/micromamba/envs/test-coverage/lib/python3.12/lib-dynload
                                 /home/lesteve/micromamba/envs/test-coverage/lib/python3.12/site-packages
                    environment: CONDA_PYTHON_EXE = /home/lesteve/micromamba/bin/python
                                 HOME = /home/lesteve
                                 PYTHONPATH = 
                   command_line: /home/lesteve/micromamba/envs/test-coverage/bin/coverage debug sys
         sqlite3_sqlite_version: 3.46.0
             sqlite3_temp_store: 0
        sqlite3_compile_options: ATOMIC_INTRINSICS=1, COMPILER=gcc-12.3.0, DEFAULT_AUTOVACUUM,
                                 DEFAULT_CACHE_SIZE=-2000, DEFAULT_FILE_FORMAT=4,
                                 DEFAULT_JOURNAL_SIZE_LIMIT=-1, DEFAULT_MMAP_SIZE=0, DEFAULT_PAGE_SIZE=4096,
                                 DEFAULT_PCACHE_INITSZ=20, DEFAULT_RECURSIVE_TRIGGERS,
                                 DEFAULT_SECTOR_SIZE=4096, DEFAULT_SYNCHRONOUS=2,
                                 DEFAULT_WAL_AUTOCHECKPOINT=1000, DEFAULT_WAL_SYNCHRONOUS=2,
                                 DEFAULT_WORKER_THREADS=0, DIRECT_OVERFLOW_READ, ENABLE_COLUMN_METADATA,
                                 ENABLE_DBSTAT_VTAB, ENABLE_FTS3, ENABLE_FTS3_TOKENIZER, ENABLE_FTS4,
                                 ENABLE_FTS5, ENABLE_GEOPOLY, ENABLE_MATH_FUNCTIONS, ENABLE_RTREE,
                                 ENABLE_UNLOCK_NOTIFY, MALLOC_SOFT_LIMIT=1024, MAX_ATTACHED=10,
                                 MAX_COLUMN=2000, MAX_COMPOUND_SELECT=500, MAX_DEFAULT_PAGE_SIZE=8192,
                                 MAX_EXPR_DEPTH=10000, MAX_FUNCTION_ARG=127, MAX_LENGTH=1000000000,
                                 MAX_LIKE_PATTERN_LENGTH=50000, MAX_MMAP_SIZE=0x7fff0000,
                                 MAX_PAGE_COUNT=0xfffffffe, MAX_PAGE_SIZE=65536, MAX_SQL_LENGTH=1000000000,
                                 MAX_TRIGGER_DEPTH=1000, MAX_VARIABLE_NUMBER=250000, MAX_VDBE_OP=250000000,
                                 MAX_WORKER_THREADS=8, MUTEX_PTHREADS, SECURE_DELETE, SYSTEM_MALLOC,
                                 TEMP_STORE=1, THREADSAFE=1
  1. What versions of what packages do you have installed? The output of pip freeze is helpful.
coverage==7.5.4
joblib==1.4.2
numpy==2.0.0
scikit-learn==1.5.1
scipy==1.14.0
setuptools==70.2.0
threadpoolctl==3.5.0
wheel==0.43.0

Expected behavior
Usins COVERAGE_CORE=sysmon improves performance and in the worst case may not improve it much but at least does not degrade it

Additional context

We originally saw this in the scikit-learn CI scikit-learn/scikit-learn#29444 (comment).

@lesteve lesteve added the bug Something isn't working label Jul 10, 2024
@lesteve lesteve changed the title COVERAGE_CORE=sysmon with branch coverage is 2x slower than default COVERAGE_CORE=sysmon with branch coverage can be 2x slower than default in some cases Jul 10, 2024
@nedbat
Copy link
Owner

nedbat commented Jul 11, 2024

Yes, the sysmon support is not yet in place to do a good job with branch coverage. I could add a warning to that effect if it would help.

@lesteve
Copy link
Author

lesteve commented Jul 19, 2024

Thanks for your answer, I guess this was more of a "is this expected" question than a call for action.

I am not sure whether a warning would be that useful, in particular because in our use case we use coverage via pytest-cov and I think pytest would capture the warning which would kind of hide it amongst a number of other warnings that happened when we run our tests.

@nedbat
Copy link
Owner

nedbat commented Jul 19, 2024

Another option: refuse to run with COVERAGE_CORE=sysmon and branch=True.

@lesteve
Copy link
Author

lesteve commented Jul 19, 2024

Why not error indeed. I guess it would make sense to do this if you think that in a vast majority of cases the performance will be worse using COVERAGE_CORE=sysmon with branch=True compared to branch=True with COVERAGE_CORE unset.

@stianjensen
Copy link

In my case, we did see a (moderate) improvement in performance when setting COVERAGE_CORE=sysmon while still using branch coverage. So I at least like that combining them does not result in an error (the performance improvement was much bigger if I also disabled branch coverage, but for now we prefer retaining that even with some perf cost).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants