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 atomic operations to avoid locking as much as possible #18

Closed
wants to merge 1 commit into from

Conversation

waponxie
Copy link

@waponxie waponxie commented Jan 9, 2022

For this issue:#17

I use atomic operations to avoid locking as much as possible.This can make performance a little better.

I used the case in the issue above, compare with master. I got the result as below:

  1. when i loop 10,000 times,they are almost the same.
  2. but when i loop 100,000 , 91568 rows of data are output. on this branch, 86648 rows of data are output.

So I think it would be slightly better in this implementation.

@jackc
Copy link
Owner

jackc commented Jan 14, 2022

This change is not safe. If you use atomics to write you must also use atomics to read.

But even if that's fixed I'm not eager to add another locking primitive to puddle. It has to use channels to support context cancel. But channels are significantly slower than a mutex. So it uses sync.Cond with a mutex and channels to be fast when a resource is immediately available and to support context cancel when it is not.

To reiterate what I said in #17, if you have 100,000 clients contending for a 1 resource, the solution isn't to optimize the wait state of the resource pool. The solution is back pressure to drop most of those clients at the start.

@jackc jackc closed this Jan 14, 2022
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.

3 participants