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

use threading.Lock.acquire's timeout for backpressure rather than sleep #650

Merged
merged 1 commit into from
Feb 11, 2023

Conversation

ypsah
Copy link
Contributor

@ypsah ypsah commented Feb 11, 2023

fixes: #649

@amoffatgmi amoffatgmi merged commit 10ee3fa into amoffat:develop Feb 11, 2023
# Using a small timeout provides backpressure against code that spams
# calls to .is_alive() which may block the main thread from acquiring
# the lock otherwise.
acquired = self._wait_lock.acquire(timeout=0.1)
Copy link
Contributor

Choose a reason for hiding this comment

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

I still had to set this back down to:

Suggested change
acquired = self._wait_lock.acquire(timeout=0.1)
acquired = self._wait_lock.acquire(timeout= 0.00001)

as it was before e9511de.

Given we're using the acquire provided timeout, is that fine now?

@JacobHayes
Copy link
Contributor

Ah, a minute too late 😁

@amoffatgmi
Copy link
Collaborator

There's still time @JacobHayes, it's only merged into the develop branch. It won't ship until it lands in the main branch. If you have a proposed change, make it and tag @ypsah

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.

Performance regression introduced in 2.0.0
4 participants