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

Integrate with fasthttp #500

Draft
wants to merge 16 commits into
base: master
Choose a base branch
from
Draft

Integrate with fasthttp #500

wants to merge 16 commits into from

Conversation

tsenart
Copy link
Owner

@tsenart tsenart commented Mar 7, 2020

Screenshot from 2020-03-15 16-29-41

Fixes #486

@AndreiPashkin
Copy link

How can we solve this without depending on two http libraries?

What about HTTP/2? FastHTTP currently doesn't support it.

@tsenart
Copy link
Owner Author

tsenart commented Mar 7, 2020

Depending on two HTTP libraries means we’d still support HTTP2, since one of them, net/http, provides it.

@AndreiPashkin
Copy link

So we have to depend to two libraries if we want to support HTTP/2.

@AndreiPashkin
Copy link

AndreiPashkin commented Mar 8, 2020

@tsenart, I don't understand - are we pursuing not using 2 libraries and backwards compatibility of modules APIs or not?

@tsenart
Copy link
Owner Author

tsenart commented Mar 8, 2020

I’m experimenting with using two libraries and breaking backwards compatibility, maybe it’s worth it.

@blakewatters
Copy link

Honestly the loss of HTTP/2 support issue is all that held me back from going full throttle to push my original experiment forward to something that could be merged (and having no idea if @tsenart would merge it).

It's a bigger lift, but an interface that lets you switch engines feels like the ideal setup -- plus if you get the abstractions right you in theory support arbitrary underlying transports with HTTP semantics (where's that Golang QUIC library at?).

FastHTTP really needs to get HTTP/2 support landed.

But the performance gains are just astronomical compared to net/http. It's easily the difference between running a single node versus going distributed

attack.go Outdated
}

// TODO(tsenart): Add fasthttp support for:
// - opts.redirects

Choose a reason for hiding this comment

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

The Do function lets you manually handle redirects https://godoc.org/github.com/valyala/fasthttp#Do

Copy link
Owner Author

Choose a reason for hiding this comment

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

Ideally, I'd like the same behaviour as net/http. I guess I need to do it manually.

attack.go Outdated Show resolved Hide resolved
attack.go Outdated
// - opts.keepalive
// - opts.proxyHeaders
// - HTTP_PROXY
// - opts.localAddr

Choose a reason for hiding this comment

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

Proxy and local addr through Dial function? Hitting the edge of my Golang-fu but trying to help: valyala/fasthttp#363

@blakewatters
Copy link

I was seeing 4x speedups on my hack-job implementation (60k rps -> 240k). Your changes are tighter but it should be faster. FastHTTP's main trick seems to be avoiding memory copies at all costs, maybe there's implicit copies going on?

@tsenart
Copy link
Owner Author

tsenart commented Mar 14, 2020

@blakewatters

It's a bigger lift, but an interface that lets you switch engines feels like the ideal setup

What do you think of the Hitter interface in this PR?

I was seeing 4x speedups on my hack-job implementation (60k rps -> 240k)

Could you check-out this branch and run the benchmarks I wrote today?

go test -run '^$' -bench=Hit ./...

FastHTTP's main trick seems to be avoiding memory copies at all costs, maybe there's implicit copies going on?

Main allocations come from allocating Result to return and copying headers to http.Header.
Did you do anything substantially different there?

@tsenart
Copy link
Owner Author

tsenart commented Mar 15, 2020

@blakewatters: I'm getting better results :-)

$ go test -run='^$' -bench Hit
goos: linux
goarch: amd64
pkg: github.com/tsenart/vegeta/lib
BenchmarkFastHTTPHitter_Hit-16            500396              2391 ns/op            418246 req/s            1300 B/op         21 allocs/op
BenchmarkNetHTTPHitter_Hit-16             116127             18996 ns/op             52643 req/s           10326 B/op         81 allocs/op
PASS
ok      github.com/tsenart/vegeta/lib   3.893s

@tsenart
Copy link
Owner Author

tsenart commented Mar 15, 2020

Check this out!

name           old time/op    new time/op    delta
Hitter_Hit-16   12.5µs ±111%     2.2µs ± 1%   -82.27%  (p=0.000 n=26+27)

name           old req/s      new req/s      delta
Hitter_Hit-16     79.0k ±97%    450.5k ± 1%  +470.33%  (p=0.000 n=30+27)

name           old alloc/op   new alloc/op   delta
Hitter_Hit-16    10.3kB ±13%     1.3kB ± 0%   -87.62%  (p=0.000 n=30+24)

name           old allocs/op  new allocs/op  delta
Hitter_Hit-16      82.4 ± 2%      21.0 ± 0%   -74.53%  (p=0.000 n=27+30)

// TODO: Write CLI integration tests

type Hitter interface {
Hit(*Target) *Result
Copy link
Contributor

Choose a reason for hiding this comment

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

Why pointers and not values? I bet you might see some even substantial perf improvements on long test runs if this were

type Hitter interface {
    Hit(Target) Result
}

Copy link
Owner Author

Choose a reason for hiding this comment

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

Benchmarks seems inconclusive, but slightly worse with values. These aren't very small structs. It follows the same pattern as the Doer interface, which takes in an *http.Request and returns an *http.Response.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Old = pointers
New = values

name                   old time/op    new time/op    delta
FastHTTPHitter_Hit-12    10.3µs ± 6%    10.7µs ±11%   ~     (p=0.133 n=9+10)
NetHTTPHitter_Hit-12     1.79ms ± 4%    1.79ms ±15%   ~     (p=0.167 n=8+9)

name                   old req/s      new req/s      delta
FastHTTPHitter_Hit-12     95.8k ±12%     93.8k ±10%   ~     (p=0.315 n=10+10)
NetHTTPHitter_Hit-12      3.46 ±700%   117.33 ±372%   ~     (p=0.529 n=8+9)

name                   old alloc/op   new alloc/op   delta
FastHTTPHitter_Hit-12    1.28kB ± 1%    1.28kB ± 1%   ~     (p=0.955 n=10+10)
NetHTTPHitter_Hit-12     5.45kB ±28%    6.21kB ±50%   ~     (p=0.459 n=9+10)

name                   old allocs/op  new allocs/op  delta
FastHTTPHitter_Hit-12      21.0 ± 0%      21.0 ± 0%   ~     (all equal)
NetHTTPHitter_Hit-12       79.0 ± 0%      80.4 ±11%   ~     (p=0.776 n=8+9)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you'd have to run the test for long enough to trigger a bunch of GC in order to see meaningful differences. But no big deal in any case.

@AndreiPashkin
Copy link

AndreiPashkin commented Mar 15, 2020

@tsenart, If I want to add a commit (with docstrings) to this branch - how can I do that (I'm new to Golang but at least I want to contribute with whatever I can)? 🤔

// TODO: Fix attacker tests
// TODO: Write CLI integration tests

type Hitter interface {

Choose a reason for hiding this comment

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

Unrelated to the (awesome) fasthttp addition, this abstraction would make it a lot easier to "re-purpose" Vegeta for SQL/general TCP load testing. A Postgres Hitter could (somewhat) easily be implemented on top of this.

I realise I'd still have to create an interface for the Target struct, but it's at least one step closer.

Some very old prior art that is basically what I'd need: https://github.com/masahide/vegeta-mysql

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

Successfully merging this pull request may close these issues.

Integrate with FastHTTP (massive performance gains)
5 participants