Skip to content

logging: reuse LoggingCaptureHandler instance since it's expensive to create #7227

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

Merged
merged 1 commit into from
May 18, 2020

Conversation

bluetech
Copy link
Member

Previously, a LoggingCaptureHandler was instantiated for each test's setup/call/teardown which turns out to be expensive.

Instead, only keep one instance and reset it between runs.

(Previously in #7214. Now I'm pretty sure it will interact well with the other PRs).

Benchmark
import pytest
@pytest.mark.parametrize("x", range(5000))
def test_foo(x): pass

Before:

============================ 5000 passed in 11.87s =============================
         10866808 function calls (10313462 primitive calls) in 12.163 seconds

After:

============================ 5000 passed in 11.25s =============================
         10431870 function calls (9878523 primitive calls) in 11.541 seconds

… create

Previously, a LoggingCaptureHandler was instantiated for each test's
setup/call/teardown which turns out to be expensive.

Instead, only keep one instance and reset it between runs.
Copy link
Contributor

@twmr twmr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thx for this PR. It would have been helpful for the review, if you mentioned why you changed the catch_log related code in the commit msg.

@bluetech
Copy link
Member Author

Thanks for the review @Thisch.

It would have been helpful for the review, if you mentioned why you changed the catch_log related code in the commit msg.

The reason is that there is no longer a different handler for each _runtest_for, so the previous solution of keeping references to the handlers of each phase no longer works. So instead, now it only keeps a reference to the records of each phase, which get replaces each phase and stay in tact.

@bluetech bluetech merged commit 85a06cf into pytest-dev:master May 18, 2020
@bluetech bluetech deleted the logging-reuse-handler branch June 17, 2020 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants