database/sql: Please don't spawn canceler goroutine when context passed to Query cannot be canceled

[navytux]

Currently database/sql spawns goroutine for every Query made through it. In case an SQL driver works with local disk (thus non-network system calls) and in particalur is CGo based this can be harming performance. Let me qoute myself from #21827 (comment):

---- 8< ----
I've hit the case with SQLite (only Cgo, no LockOSThread) where presense of other goroutines combined with Cgo calls on "main" one show big overhead: github.com/mattn/go-sqlite3 uses CGo and performs several CGo calls inside one Query or Exec. There was also an interrupt goroutine spawned for every Query or Exec to call sqlite3_interrup if context is canceled.

With Go1.10 avoiding to spawn that interrupt goroutine, if we know the context cannot be canceled, brings ~ 1.5x speedup to Exec (mattn/go-sqlite3#530).

However Query was made faster only by 5% because after 2b283ced (/cc @kardianos, @bradfitz) database/sql always spawns a goroutine to close Rows on context or transaction cancel.

( @kardianos it would be nice if somehow we could avoid spawning Rows.awaitDone if original query context cannot be cancelled. Because with the following test patch:

diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go
index 8f5588ed26..4345aa736a 100644
--- a/src/database/sql/sql.go
+++ b/src/database/sql/sql.go
@@ -2568,6 +2568,7 @@ type Rows struct {
 }
 
 func (rs *Rows) initContextClose(ctx, txctx context.Context) {
+       return
        ctx, rs.cancel = context.WithCancel(ctx)
        go rs.awaitDone(ctx, txctx)
 }

SQLite Query and rest becomes speed up too:

    name               old req/s    new req/s    delta
    Exec                 379k ± 1%    375k ± 3%     ~     (p=0.218 n=10+10)
    Query               96.4k ± 1%  132.2k ± 3%  +37.14%  (p=0.000 n=10+10)
    Params              87.0k ± 1%  117.2k ± 3%  +34.66%  (p=0.000 n=10+10)
    Stmt                 129k ± 1%    178k ± 2%  +37.45%  (p=0.000 n=9+9)
    Rows                3.06k ± 1%   3.45k ± 1%  +12.49%  (p=0.000 n=10+9)
    StmtRows            3.13k ± 1%   3.54k ± 1%  +12.85%  (p=0.000 n=10+9)

    name               old time/op  new time/op  delta
    CustomFunctions-4  10.1µs ± 1%   7.2µs ± 2%  -28.68%  (p=0.000 n=10+10)

/cc @mattn)

---- 8< ----

I've reverified the situation is the same on latest tip (go version devel +c941e27e70 Fri Feb 16 19:28:41 2018 +0000 linux/amd64) so indeed if database/sql could be changed not to spawn 1 additional goroutine per every query if context cannot be canceled it would help.

Thanks beforehand,
Kirill

[gopherbot]

Change https://golang.org/cl/102478 mentions this issue: database/sql: a context.Background will never need to await

[navytux]

@kardianos, thanks for fixing this issue.