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

proveryai #2

Open
wants to merge 30 commits into
base: zepto
Choose a base branch
from
Open

proveryai #2

wants to merge 30 commits into from

Conversation

shmel1k
Copy link
Owner

@shmel1k shmel1k commented Dec 15, 2018

No description provided.

Copy link

@n-canter n-canter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

добавь circleci раз уж проект на похвастаться

worker.go Outdated
for {
select {
case t := <-w.tasks:
if t != nil {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

эти 4 строчки лучше обернуть в отдельнюу функцию типа runTask(t TaskFn) ...

return
case <-w.quit:
return
case t := <-w.tasks:

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

в функцию

worker.go Outdated

t()

select {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

это можно удалить, внутри цикла такая же проверка

worker.go Outdated
}
}

type additionalWorker struct {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

я бы сделал type additionalWorker worker

default:
}

ticker := time.NewTicker(w.conf.ttl)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

вообще если уж заморачиваться над производительностью, то таймеры лучше запулить, они много жрут, и похоже тут тебе нужен не ticker, а timer. NewTimer() или time.After()

}

func (w *worker) run() {
for {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

если в Shutdown закрывать канал tasks, то тут можно просто писать

for t := range w.tasks{
...
}

return nil
}

if p.ticker.C == nil {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

вроде бы, так не бывает

return ErrPoolFull
}

select {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а это зачем?
типа если таск не успел зашедулиться за какое-то время то дронуть его? если логика такая, то таймер нужно в предыдущий селект написать и нужно рассчитывать не на тикер, а начинать время отсчитывать перед входом в селект

pool.go Outdated
atomic.AddInt32(&p.realQueueSize, -1)

p.mu.Lock()
p.additionalWorkersAvailable--

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ты на один запуска extraworker делаешь два декремента счетчика и один инкремент

}

uAvail := atomic.LoadInt32(&p.unstoppableWorkersAvailable)
if uAvail == 0 && p.spawnExtraWorker(t) == nil {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

тут тоже есть рэйс логический, но на него наверно можно забить

@shmel1k shmel1k force-pushed the master branch 5 times, most recently from 1180318 to 34d4d26 Compare December 17, 2018 11:04
@shmel1k shmel1k force-pushed the master branch 2 times, most recently from 39c87f6 to 54ff7ae Compare December 17, 2018 12:58
@shmel1k shmel1k force-pushed the master branch 2 times, most recently from 4523cfd to 534d6c3 Compare December 18, 2018 09:42
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.

2 participants