[POABOB]

---

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

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

[panjf2000]

Please also apply the same changes in pool_func.go

[POABOB]

All done. @panjf2000

[POABOB]

Above done. @panjf2000

[POABOB]

Done. @panjf2000

[codecov]

Codecov Report

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

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

Additional details and impacted files

@@            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	Coverage Δ
unittests	94.08% <100.00%> (+1.59%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed	Coverage Δ
pool.go	93.80% <100.00%> (+2.55%)	⬆️
pool_func.go	92.57% <100.00%> (+2.45%)	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[POABOB]

Done. @panjf2000

[panjf2000]

Thank you for your contribution!
@POABOB