You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2020/06/24 00:15:26 UTC

[GitHub] [pulsar-client-go] merlimat opened a new pull request #298: Added semaphore implementation with lower contention

merlimat opened a new pull request #298:
URL: https://github.com/apache/pulsar-client-go/pull/298


   ### Motivation
   
   When there are multiple go-routines sharing a single producer instance, there can be significant contention on the channel that is used as a semaphore. 
   
   That is because the channel is internally implemented with a mutex.
   
   Instead, use a counter to optimize for the normal case scenario (when we're not blocked by the semaphore), leaving the channel as a way to synchronize when the semaphore has exhausted all the permits.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] aahmed-se commented on pull request #298: Added semaphore implementation with lower contention

Posted by GitBox <gi...@apache.org>.
aahmed-se commented on pull request #298:
URL: https://github.com/apache/pulsar-client-go/pull/298#issuecomment-648546006


   @merlimat have you looked into this library it's benchmarked against the other semaphore implementations.
   https://github.com/marusama/semaphore


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] aahmed-se commented on pull request #298: Added semaphore implementation with lower contention

Posted by GitBox <gi...@apache.org>.
aahmed-se commented on pull request #298:
URL: https://github.com/apache/pulsar-client-go/pull/298#issuecomment-648545621


   @addisonj golang semaphore are channel based which are not as fast as this implementation.
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] addisonj commented on pull request #298: Added semaphore implementation with lower contention

Posted by GitBox <gi...@apache.org>.
addisonj commented on pull request #298:
URL: https://github.com/apache/pulsar-client-go/pull/298#issuecomment-648521291


   This looks like a nice improvement, one question I have though: is this now perhaps sane to have it be backed by https://godoc.org/golang.org/x/sync/semaphore? It seems like you could perhaps have most of the details covered with that package and add the details for blocked threads waiting for permits as part of a small wrapper? I am not 100% sure about the semaphore packages semantics to know if that would work but just what came to mind as I looked at the difference between the packages.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] merlimat merged pull request #298: Added semaphore implementation with lower contention

Posted by GitBox <gi...@apache.org>.
merlimat merged pull request #298:
URL: https://github.com/apache/pulsar-client-go/pull/298


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [pulsar-client-go] merlimat commented on pull request #298: Added semaphore implementation with lower contention

Posted by GitBox <gi...@apache.org>.
merlimat commented on pull request #298:
URL: https://github.com/apache/pulsar-client-go/pull/298#issuecomment-648936231


   @addisonj The x/sync/semaphore has more features (tryAcquire, weighted) but it's still using mutex, even in best case so it has the same contention profile.
   
   @aahmed-se I've checked that lib and it has the same approach used here though it has more features which we don't need (change limit, weighted). For these additional features it needs to have the RW-mutex and change the channel each time. 
   Since we don't use that, we should avoid paying for it.
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org