[
https://issues.apache.org/jira/browse/THRIFT-4552?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16437451#comment-16437451
]
ASF GitHub Bot commented on THRIFT-4552:
----------------------------------------
jiajunhuang opened a new pull request #1542: THRIFT-4552: remove unneccessary
lock
URL: https://github.com/apache/thrift/pull/1542
1. use CAS operation instead of lock
2. remove defer to reduce unneccessary performance consume
3. go fmt those code
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
> why acquire a lock in TSimpleServer implementation for Go?
> ----------------------------------------------------------
>
> Key: THRIFT-4552
> URL: https://issues.apache.org/jira/browse/THRIFT-4552
> Project: Thrift
> Issue Type: Improvement
> Affects Versions: 0.11.0
> Reporter: gansteed
> Priority: Minor
>
> I've sent a email to groups, but I think maybe here will be better?
>
> I'm using Thrift and I'm reading thrift implementation for Go, I found code
> in `TSimpleServer.AcceptLoop` like this:
>
> {code:go}
> func (p *TSimpleServer) AcceptLoop() error {
> for {
> client, err := p.serverTransport.Accept()
> p.mu.Lock()
> if atomic.LoadInt32(&p.closed) != 0 {
> return nil
> }
> if err != nil {
> return err
> }
> if client != nil {
> p.wg.Add(1)
> go func() {
> defer p.wg.Done()
> if err := p.processRequests(client); err !=
> nil {
> log.Println("error processing
> request:", err)
> }
> }()
> }
> p.mu.Unlock()
> }
> }
> {code}
>
> every time it accept a request,it:
>
> 1. read if protocol had been closed, this step is atomic, it does not need a
> lock.
> 2. p.wg.Add(1) to accumulate a goroutine? this step is atomic, too, it does
> not need a lock
> 3. after processor processed the request, it do p.wg.Done(), it's atomic,
> too, and it does not need a lock.
>
> by the way, it seems that `p.wg.Done()` do not need to put in defer? just put
> it after p.processRequests(client)?
>
> so is there any particular to do it in this way?if not, I would like to
> submit a PR to reduce unneccessary performance overhead in TSimpleServer
> implementation.
>
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
