Skip to content

Fewer memory allocations #68

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

Closed
wants to merge 1 commit into from
Closed

Fewer memory allocations #68

wants to merge 1 commit into from

Conversation

bradfitz
Copy link

@bradfitz bradfitz commented May 2, 2013

Feel free to take all or part of this. I'll be away until Monday, at which time I can resume working on memory/CPU/contention in the MySQL driver and database/sql code.

Before: 603 B/op 24 allocs/op
After: 344 B/op 15 allocs/op

Fewer garbage collection events, since we're allocating less.

From:

package main

import (
        "database/sql"
        "sync"
        "sync/atomic"
        "testing"

        _ "github.com/go-sql-driver/mysql"
)

const concurrencyLevel = 10

type TB interface {
        Fatal(...interface{})
}

func check(tb TB, err error) {
        if err != nil {
                tb.Fatal(err)
        }
}

func initDB(tb TB) *sql.DB {
        checkRes := func(res sql.Result, err error) {
                check(tb, err)
        }
        db, err := sql.Open("mysql", "root:r@/test?charset=utf8")
        check(tb, err)
        db.SetMaxIdleConns(concurrencyLevel)
        checkRes(db.Exec("DROP TABLE IF EXISTS foo"))
        checkRes(db.Exec("CREATE TABLE foo (id INT PRIMARY KEY, val CHAR(50))"))
        checkRes(db.Exec(`INSERT INTO foo VALUES (1, "one")`))
        checkRes(db.Exec(`INSERT INTO foo VALUES (2, "two")`))
        return db
}

func BenchmarkQuery(b *testing.B) {
        b.StopTimer()
        b.ReportAllocs()
        db := initDB(b)
        defer db.Close()

        stmt, err := db.Prepare("SELECT val FROM foo WHERE id=?")
        check(b, err)
        defer stmt.Close()
        b.StartTimer()

        remain := int64(b.N)
        var wg sync.WaitGroup
        wg.Add(concurrencyLevel)
        defer wg.Wait()
        for i := 0; i < concurrencyLevel; i ++ {
                go func() {
                        defer wg.Done()
                        for {
                                if atomic.AddInt64(&remain, -1) < 0 {
                                        return
                                }
                                var got string
                                check(b, stmt.QueryRow(1).Scan(&got))
                                if got != "one" {
                                        b.Errorf("query = %q; want one", got)
                                        return
                                }
                        }
                }()
        }
}

@bradfitz
Copy link
Author

bradfitz commented May 2, 2013

Btw, you can test with:

$ go test -c
$ GOGC=off ./dbperf.test -test.bench=. -test.v -test.benchtime=5s -test.memprofile=prof.mem
$ go tool pprof dbperf.test prof.mem
pprof> web

Then look at the allocation graph.

Or to see where in a function:

pprof> disasm MethodName

@ghost ghost assigned julienschmidt May 2, 2013
@julienschmidt
Copy link
Member

Thanks! I'll try this out at this weekend.

}

func putFields(f []mysqlField) {
select {
Copy link
Member

Choose a reason for hiding this comment

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

this should be fieldCache <- f, I see no need for a select. Same for putBytes and putMysqlRows.

Copy link
Member

Choose a reason for hiding this comment

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

If the cache is full (pool would be a better name, eh?) this would block until a slot is freed.
It gets worse with every new allocated Slice (L100)

Copy link
Member

Choose a reason for hiding this comment

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

You're right, I was too sloppy when reading the disassembly. I didn't catch the difference for bool *pres in chansend.

@julienschmidt
Copy link
Member

Continued at #71

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants