Go: net/url: optimization for calling URL.Query multiple times

[alpancs]

What version of Go are you using (go version)?

$ go version
go version go1.14 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output

$ go env

GO111MODULE=""

GOARCH="amd64"

GOBIN=""

GOCACHE="/home/alfan/.cache/go-build"

GOENV="/home/alfan/.config/go/env"

GOEXE=""

GOFLAGS=""

GOHOSTARCH="amd64"

GOHOSTOS="linux"

GOINSECURE=""

GONOPROXY=""

GONOSUMDB=""

GOOS="linux"

GOPATH="/home/alfan/go"

GOPRIVATE=""

GOPROXY="https://proxy.golang.org,direct"

GOROOT="/usr/local/go"

GOSUMDB="sum.golang.org"

GOTMPDIR=""

GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"

GCCGO="gccgo"

AR="ar"

CC="gcc"

CXX="g++"

CGO_ENABLED="1"

GOMOD="/home/alfan/Documents/bukalapak/wagyu/go.mod"

CGO_CFLAGS="-g -O2"

CGO_CPPFLAGS=""

CGO_CXXFLAGS="-g -O2"

CGO_FFLAGS="-g -O2"

CGO_LDFLAGS="-g -O2"

PKG_CONFIG="pkg-config"

GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build085565713=/tmp/go-build -gno-record-gcc-switches"

What did you do?

package main

import (
    "fmt"
    "net/url"
    "time"
)

func main() {
    someURL, _ := url.Parse("https://example.com?a=a&b=b&c=c&d=d&e=e&f=f&g=g&h=h&i=i&j=j") // error omitted

    startTime := time.Now()
    for i := 0; i < 500111; i++ {
        someURL.Query()
    }

    fmt.Println("elapsed time:", time.Since(startTime))
}

_*I write the source code here instead of writing it in play.golang.org because on that environment the output was always elapsed time: 0s (no idea 🤷‍♂️️)_

What did you expect to see?

elapsed time: 325.914µs

(or something under 1ms) ### What did you see instead?

elapsed time: 856.723768ms

[alpancs]

I'm planning to add a cache for the return value of method URL.Query.

1. Add two private fields of type URL

type URL struct {
    Scheme     string
    Opaque     string    // encoded opaque data
    User       *Userinfo // username and password information
    Host       string    // host or host:port
    Path       string    // path (relative paths may omit leading slash)
    RawPath    string    // encoded path hint (see EscapedPath method)
    ForceQuery bool      // append a query ('?') even if RawQuery is empty
    RawQuery   string    // encoded query values, without '?'
    Fragment   string    // fragment for references, without '#'

    lastRawQueryParsed string
    lastQueryResult Values
}

1. Use lastQueryResult as return value of method URL.Query when lastRawQueryParsed = RawQuery; otherwise get the return value first, then update the value of lastRawQueryParsed and lastQueryResult

[dmitshur]

Thanks for filing an issue and discussing a potential fix first.

The url.URL type is very low level and used by many Go programs. The benefit of this optimization will need to be evaluated against the cost very carefully.

It may not be viable to add unexported fields to the url.URL type, because it'll have significant effects on the usability of this type. For example, it won't be possible to compare two url.URL values with == operator reliably.

To have good performance, users can implement caching in their handlers if they need to use the result of Query method often:

q := req.URL.Query()
// ... use q many times

Alternatively, http.Request.FormValue already implements caching and can be used instead:

// use req.FormValue() many times

/cc @ianlancetaylor @dsnet

[dmitshur]

_*I write the source code here instead of writing it in play.golang.org because on that environment the output was always elapsed time: 0s (no idea 🤷‍♂️️)_

This is expected behavior. Computations are instantaneous on the Go playground and don't use up time. Read the Faking time section of the playground blog post.

[alpancs]

Thanks for the advice @dmitshur.
I think the problem has been solved by http.Request.FormValue.

Is it OK to close this issue?

_*TIL: http.Request.FormValue works for URL query. 😅_

[dmitshur]

Glad to hear it!

Let's wait a few days to see if anyone else has more thoughts on this, but after that it should be fine to close.