refactor: refine the code in `retrieveWorker` to make it more readable

POABOB commented 1 year ago

name: Pull request about: Propose changes to the code title: 'make func of retrieveWorker() more readable.' labels: '' assignees: ''

1. Are you opening this pull request for bug-fixs, optimizations or new feature?

refactor

2. Please describe how these code changes achieve your intention.

I noticed that when I call retrieveWorker(), it processes two repetitive functions: detach() and capacity == -1 || capacity > p.Running() in the else block. Thus, I move the retry label to the first instance of the detach() function and made a little change of if-else block. It didn't affect the logic, but it is more readable.

before

// retrieveWorker returns an available worker to run the tasks.
func (p *Pool) retrieveWorker() (w worker) {
    spawnWorker := func() {
        w = p.workerCache.Get().(*goWorker)
        w.run()
    }

    p.lock.Lock()
    w = p.workers.detach()
    if w != nil { // first try to fetch the worker from the queue
        p.lock.Unlock()
    } else if capacity := p.Cap(); capacity == -1 || capacity > p.Running() {
        // if the worker queue is empty and we don't run out of the pool capacity,
        // then just spawn a new worker goroutine.
        p.lock.Unlock()
        spawnWorker()
    } else { // otherwise, we'll have to keep them blocked and wait for at least one worker to be put back into pool.
        if p.options.Nonblocking {
            p.lock.Unlock()
            return
        }
    retry:
        if p.options.MaxBlockingTasks != 0 && p.Waiting() >= p.options.MaxBlockingTasks {
            p.lock.Unlock()
            return
        }

        p.addWaiting(1)
        p.cond.Wait() // block and wait for an available worker
        p.addWaiting(-1)

        if p.IsClosed() {
            p.lock.Unlock()
            return
        }

        if w = p.workers.detach(); w == nil {
            if p.Free() > 0 {
                p.lock.Unlock()
                spawnWorker()
                return
            }
            goto retry
        }
        p.lock.Unlock()
    }
    return
}

after

// retrieveWorker returns an available worker to run the tasks.
func (p *Pool) retrieveWorker() (w worker) {
    spawnWorker := func() {
        w = p.workerCache.Get().(*goWorker)
        w.run()
    }

    p.lock.Lock()

retry:
    if w = p.workers.detach(); w != nil {
        // first try to fetch the worker from the queue
        p.lock.Unlock()
        return
    }

    if capacity := p.Cap(); capacity == -1 || capacity > p.Running() {
        // if the worker queue is empty and we don't run out of the pool capacity,
        // then just spawn a new worker goroutine.
        p.lock.Unlock()
        spawnWorker()
        return
    }

    // otherwise, we'll have to keep them blocked and wait for at least one worker to be put back into pool.
    if p.options.Nonblocking {
        p.lock.Unlock()
        return
    }

    if p.options.MaxBlockingTasks != 0 && p.Waiting() >= p.options.MaxBlockingTasks {
        p.lock.Unlock()
        return
    }

    p.addWaiting(1)
    p.cond.Wait() // block and wait for an available worker
    p.addWaiting(-1)

    if p.IsClosed() {
        p.lock.Unlock()
        return
    }

    goto retry
}

3. Please link to the relevant issues (if any).

No

4. Which documentation changes (if any) need to be made/updated because of this PR?

No

4. Checklist

[x] I have squashed all insignificant commits.
[x] I have commented my code for explaining package types, values, functions, and non-obvious lines.
[x] I have written unit tests and verified that all tests passes (if needed).
[x] I have documented feature info on the README (only when this PR is adding a new feature).
[x] (optional) I am willing to help maintain this change if there are issues with it later.

POABOB commented 1 year ago

All done. @panjf2000

POABOB commented 1 year ago

Above done. @panjf2000

POABOB commented 1 year ago

Done. @panjf2000

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 100.00% and project coverage change: +1.59% :tada:

Comparison is base (1ce8146) 92.48% compared to head (a92f6c8) 94.08%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## dev #295 +/- ## ========================================== + Coverage 92.48% 94.08% +1.59% ========================================== Files 9 9 Lines 772 744 -28 ========================================== - Hits 714 700 -14 + Misses 44 32 -12 + Partials 14 12 -2 ``` | [Flag](https://app.codecov.io/gh/panjf2000/ants/pull/295/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andy+Pan) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/panjf2000/ants/pull/295/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andy+Pan) | `94.08% <100.00%> (+1.59%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andy+Pan#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files Changed](https://app.codecov.io/gh/panjf2000/ants/pull/295?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andy+Pan) | Coverage Δ | | |---|---|---| | [pool.go](https://app.codecov.io/gh/panjf2000/ants/pull/295?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andy+Pan#diff-cG9vbC5nbw==) | `93.80% <100.00%> (+2.55%)` | :arrow_up: | | [pool\_func.go](https://app.codecov.io/gh/panjf2000/ants/pull/295?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Andy+Pan#diff-cG9vbF9mdW5jLmdv) | `92.57% <100.00%> (+2.45%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

POABOB commented 1 year ago

Done. @panjf2000

panjf2000 commented 1 year ago

Thank you for your contribution! @POABOB

panjf2000 / ants