CODEWITHSUNDEEP
gocpu
Charles Bryant
Charles Bryant

Reputation: 1025

Golang for select loop consumes 100% of cpu

I have a resource that needs to be loaded before any access is allowed to it. It also needs to be updated every minute.

The channels are of length 1 struct{}, so the loop will be blocked if the resource has not been loaded.

This code below started to use 100% of my cpu, I tried adding

time.Sleep(10 * time.Millisecond)

Which made the application cpu consumption drop to 1%

I am thinking a ticker would be a better option for the timed collection.

Any ideas why it would consume 100% cpu or any better ideas for implemention?

func (al *AsyncLoop) Run() {
    go func() {
        for {
            select {
            case <-al.chanFetchRequest:
                if al.fetched == false {
                    al.fetchData()
                } else if al.lastUpdated.Add(1*time.Minute).Unix() < time.Now().Unix() && al.fetching == false {
                    go al.fetchData()
                }
                al.chanFetchResponse <- struct{}{}
                continue
            default:
                continue
            }
        }
    }()
}

Upvotes: 1

Views: 4795

Answers (2)

JanPo
JanPo

Reputation: 355

default statement creates a nonblocking infinite loop that leads 100% cpu usage. When the condition for case is not satisfied, the loop goes to the default nonblocking infinite loop.

Removing default will resolve the issue. But there will be a linter warning S1000: should use for range instead of for { select {} } to inform that select is meaningless with single channel.

The final code will be

for {
    _ := <-al.chanFetchRequest
    if al.fetched == false {
        al.fetchData()
        nextFetch = time.Now().Add(1 * time.Minute)
    } else if time.Now().After(nextFetch) {
        al.fetchData()
        nextFetch = time.Now().Add(1 * time.Minute)
    }
}

Upvotes: 4

AJR
AJR

Reputation: 1681

I think you just post to al.chanFetchRequest when there is new data so I think you have to keep reading from this channel all the time. Adding a ticker to the select might cause you to fetch the data even if it has not changed or (worse) before it has even loaded. Why not, in the normal case, take the time whenever you fetch the data then the next time make sure you have waited enough time before fetching again. Something like this:

        var nextFetch time.Time
        for {
            select {
            case <-al.chanFetchRequest:
                if al.fetched == false {
                    al.fetchData()
                    nextFetch = time.Now().Add(1 * time.Minute)
                } else if time.Now().After(nextFetch) {
                    al.fetchData()
                    nextFetch = time.Now().Add(1 * time.Minute)
                }
                al.chanFetchResponse <- struct{}{}
            }
        }

Upvotes: 1

Related Questions