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

Port the outdated pipeline patch against master #57

Closed
wants to merge 1 commit into from
Closed

Port the outdated pipeline patch against master #57

wants to merge 1 commit into from

Conversation

trustin
Copy link

@trustin trustin commented Dec 3, 2013

This is a modification of 220df26 that works against master.

@trustin
Copy link
Author

trustin commented Dec 4, 2013

Dynamic pipelined requests are handled correctly now. All known memory leaks related with pipelining were fixed.

- Modification of 220df26
- script_request() has been renamed to script_request_single()
- The new script_request() accepts the number of requests so that it
  generates a series of pipelined requests.
@wg
Copy link
Owner

wg commented Dec 8, 2013

Thanks @trustin! I've actually been planning to deprecate the --pipeline option in favor of returning multiple concatenated requests from a script's request() function. This would allow for more realistic tests that request a number of different resources.

I've finally gotten around to pushing a new pipeline branch, and example script (https://github.com/wg/wrk/blob/572274fe09be96a7af9fd701fe320ed4870a2ea3/scripts/pipeline.lua).

@wg wg closed this Dec 8, 2013
@trustin
Copy link
Author

trustin commented Dec 8, 2013

My use case is to pipeline the same requests. Would it still be possible to do that without compromising throughput?

@wg
Copy link
Owner

wg commented Dec 8, 2013

Yes, if you precompute the requests in init() and just return the same global in request() throughput should be very high. I did some brief testing on my usual EC2 cc2.8xlarge instance with TSC clocksource and saw around 1.5M req/sec with pipeline.lua modified for a depth of 8.

@trustin
Copy link
Author

trustin commented Dec 9, 2013

Thanks, @wg :-)

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.

2 participants