net/http: do not query HTTP_PROXY on every request

[dvyukov]

Do we need to query HTTP_PROXY env var before every request? The docs are quite vague.
Do we want to support a use case when an application itself changes HTTP_PROXY at
runtime? Then I think the app just need to specify the proxy programmatically rather
than changing HTTP_PROXY.

I am asking because querying HTTP_PROXY accounts for 15% of memory allocations on
end-to-end client/server http benchmark (10% of garbage generated).

Windows syscall package reallocates the string several times and we query it twice.

[davecheney]

Comment 1:

No, I do not think HTTP_PROXY needs to be checked on every request. That said, the
testing infrastructure relies on this property.

[dvyukov]

Comment 2:

It's a pity. For testing we can use a private resetProxyCache function.

[alexbrainman]

Comment 3:

I do not know why we do that. I cannot decide if it is importance or not. Up to Brad to
decide.
Alex

[bradfitz]

Comment 4:

I've never seen HTTP_PROXY stuff show up in benchmarks, but I've also never used
HTTP_PROXY in my life.
Is the problem that your machine actually uses HTTP_PROXY and then the
BenchmarkClientServer ends up load testing itself through a real proxy, non-hermetically?
Please explain what's happening.

[dvyukov]

Comment 5:

No, this happens even with empty env vars. On windows just querying env var creates at
least 2 copies of the env var name string (one to append 0, and another to convert to
[]uint16). And we query both HTTP_PROXY and http_proxy. This accounts for 15% of
allocations and 10% of garbage on end-to-end http client/server benchmark.
Look at the difference between linux and windows:
http://goperfd.appspot.com/perfgraph?builder=linux-amd64&builder=windows-amd64&benchmark=http&procs=1&metric=allocs&absolute=1&startcommit=1422&commitnum=2000&refresh=Refresh
What do you think if we query HTTP_PROXY once lazily (so that user has a chance to set
it in main) and provide updateProxyEnvVars() for tests?
I think the advice for other cases is to use programmatic interface. Env vars are not
"composable" anyway, if 2 subsystems alter HTTP_PROXY one of them won't work.

[bradfitz]

Comment 6:

ah!  I see.
Yes, let's fix it and only check the environment once.  I don't even care if it's lazy
(changing environment variables at runtime is not a good API), but whatever... lazy is
fine, so we don't break existing users.

[mattn]

Comment 7:

How about this?
diff -r 9d0f388bbe40 src/pkg/net/http/transport.go
--- a/src/pkg/net/http/transport.go Mon Jan 06 01:34:56 2014 +1100
+++ b/src/pkg/net/http/transport.go Tue Jan 07 20:28:49 2014 +0900
@@ -261,6 +261,9 @@
        if err != nil {
            return nil, err
        }
+       if cm.proxyURL == nil {
+           t.Proxy = nil
+       }
    }
    return cm, nil
 }

[mattn]

Comment 8:

Ah, useProxy require the request URL...

[bradfitz]

Comment 9:

This issue was closed by revision 4deead7.

Status changed to Fixed.