couchbase / go-couchbase

Couchbase client in Go
https://godoc.org/github.com/couchbase/go-couchbase
MIT License
321 stars 92 forks source link

Why GetBulk() use 4 goroutines to fetch data? #49

Open mnhkahn opened 9 years ago

mnhkahn commented 9 years ago

I want to use GetBulk to fetch data, which fetches multiple keys concurrently. But the source code of this function use only 4 worker goroutines to fetch concurrently, I think the count is len(kdm) is better. And the channel of key, wch, I think it should be allocated with enough buffer, like this, wch := make(chan uint16, len(kdm).

func (b *Bucket) processBulkGet(kdm map[uint16][]string,
    ch chan map[string]*gomemcached.MCResponse, ech chan error) {
    wch := make(chan uint16)
    defer close(ch)
    defer close(ech)

    wg := &sync.WaitGroup{}
    worker := func() {
        defer wg.Done()
        for k := range wch {
            b.doBulkGet(k, kdm[k], ch, ech)
        }
    }

    for i := 0; i < 4; i++ {
        wg.Add(1)
        go worker()
    }

    for k := range kdm {
        wch <- k
    }
    close(wch)
    wg.Wait()
}
dustin commented 9 years ago

Under what conditions did you test the performance gains you got from these changes?

How much of a gain did you get with each one independently?

How much of a gain did you get with them combined?

I'm particularly curious about the buffered wch -- buffering it will definitely make it use (a little) more memory, but it seems unlikely it would actually make it faster in any way that's worth the its weight in buffer overhead.