Skip to content
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

net/http: build tag to disable HTTP/2 #35082

Closed
crawshaw opened this issue Oct 22, 2019 · 13 comments
Closed

net/http: build tag to disable HTTP/2 #35082

crawshaw opened this issue Oct 22, 2019 · 13 comments
Labels

Comments

@crawshaw
Copy link
Contributor

@crawshaw crawshaw commented Oct 22, 2019

Background:

I am writing an iOS App Extension in Go. This particular extension is hard limited by the OS to 16MB RAM, which seems to be based on RSS. This means binary size is counted in the RAM limit, and indeed appears to dominate the RAM use. It is easy to keep heap < 5mb and stacks < 2mb, but the simplest Go programs blow out into 10mb binaries (after removing DWARF).

Suggestion:

Add a build tag to disable HTTP/2. This lets users cut 600kb off their (DWARF-less) binaries.

Experimenting with my app, it includes the HTTP client from net/http and is currently 8.6mb when compiled with -ldflags=-w. By deleting h2_bundle.go (the HTTP/2 implementation) and the few references to it in the other files of net/http, my binary size drops to 7.99mb.

@dmitshur

This comment has been minimized.

Copy link
Member

@dmitshur dmitshur commented Oct 22, 2019

/cc @bradfitz

@dmitshur

This comment has been minimized.

Copy link
Member

@dmitshur dmitshur commented Oct 22, 2019

Is it viable to instead make just the HTTP/1 client available in an external package, and use that instead of net/http? Similarly to how HTTP/2 exists in golang.org/x/net/http2 in addition to net/http.

The challenge of adding a build constraint for this is that it adds a new configuration that’ll need to be tested, and it may scale poorly when there’s also HTTP/3.

@bradfitz

This comment has been minimized.

Copy link
Member

@bradfitz bradfitz commented Oct 22, 2019

The challenge of adding a build constraint for this is that it adds a new configuration that’ll need to be tested,

We already do similar with osusergo and netgo and have a testing story there. (test that they compile in a separate cmd/dist test). I don't think we'd need any builder coverage other than making sure it compiles.

@odeke-em

This comment has been minimized.

Copy link
Member

@odeke-em odeke-em commented Oct 23, 2019

We started some simple discourse on Twitter. I suggested

+build !no_http2
OR
+build !disable_http2
For the h2_bundle.go file

and @bradfitz replied with

Double negatives always suck but probably unavoidable. At least we can avoid "not no". Like "not omit". Maybe omithttp2. It's not disabled. It doesn't exist. We don't generally use underscores in build tags.

and I agreed with

Yeah:
+build !omithttp2
works. I like the “omit” prefix

So a current pitch is to use +build !omithttp2 if using building tags

@bradfitz

This comment has been minimized.

Copy link
Member

@bradfitz bradfitz commented Oct 23, 2019

The existing precedent seems to be to prefix the build tag with the package name (netgo for the net package, osusergo for the os/user package). If we kept that up it'd be nethttpomithttp2 which is kinda a mouthful but ... specific. I'm okay with it.

We'd probably want to make the clientServerTest helper just t.Skip when http2 isn't compiled in. Hopefuly it's not much more than that.

@bradfitz

This comment has been minimized.

Copy link
Member

@bradfitz bradfitz commented Oct 23, 2019

In any case, I'm fine with this, especially if it's as small of a patch as I'm imagining. I haven't tried.

@bronze1man

This comment has been minimized.

Copy link
Contributor

@bronze1man bronze1man commented Oct 23, 2019

@crawshaw
Knowing workaround to memory limit of iOS App Extension :

using go1.9.x

@dmitshur

This comment has been minimized.

Copy link
Member

@dmitshur dmitshur commented Oct 23, 2019

Changing to NeedsFix per #35082 (comment). Should go back to NeedsDecision if there are complications.

@dmitshur dmitshur added NeedsFix and removed NeedsDecision labels Oct 23, 2019
@agnivade

This comment has been minimized.

Copy link
Contributor

@agnivade agnivade commented Oct 23, 2019

On a related note, this will help reduce wasm binary sizes too.

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Oct 30, 2019

If you import golang.org/x/net/http2 to get the "latest" http2 then you still end up with two HTTP2 stacks in your binaries, one just dead code. I wonder if we can do anything to fix that more general problem of being able to sub in a new http2 and actually drop the old one.

If we did that, then @crawshaw's use case could import an "emptyhttp2" instead.

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Oct 30, 2019

@bradfitz says the "right" fix is a ways off if it's anywhere. The build tag is fine to solve this specific problem in the short term.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Nov 3, 2019

Change https://golang.org/cl/205017 mentions this issue: cmd/bundle: add -tags flag

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Nov 4, 2019

Change https://golang.org/cl/205139 mentions this issue: net/http: support disabling built-in HTTP/2 with a new build tag

gopherbot pushed a commit to golang/tools that referenced this issue Nov 4, 2019
Adds a "-tags" flag that'll allow build tags
to be passed in and added to the very top of the
generated and bundled file.

For example, when generating h2_bundle.go for
net/http, we'll now be able to do:

    bundle -tags '!nethttpomithttp2' -o h2_bundle.go
                -prefix http2 golang.org/x/net/http2

Updates golang/go#35082

Change-Id: I55edd7227aec8641b60ba560c79e0d50d0692d52
Reviewed-on: https://go-review.googlesource.com/c/tools/+/205017
Run-TryBot: Emmanuel Odeke <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@gopherbot gopherbot closed this in 2566e21 Nov 4, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.