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

Add LimitedStream benchmark #57

Open
carltongibson opened this issue Jul 26, 2022 · 3 comments
Open

Add LimitedStream benchmark #57

carltongibson opened this issue Jul 26, 2022 · 3 comments

Comments

@carltongibson
Copy link
Member

Hi @deepakdinesh1123 @smithdc1.

Have a look at the ticket here: https://code.djangoproject.com/ticket/33865

There's a proposal to optimise LimitedStream (which wraps the WSGI request body input stream to ensure we can't read into other requests...).

It might be nice to quickly add a benchmark for it, and then be able to say, "Look here's main, and here's the PR".
Are we there yet?

What do you think?

@deepakdinesh1123
Copy link
Contributor

#58, in this PR I have added the benchmark based on the benchmarks (here)[https://code.djangoproject.com/attachment/ticket/33865/bench_limitedstream.py], and the results, were as follows

     before           after      ratio
 [e20e5d15]       [021916db]
 <main>           <pull/15872/head~3>
  115±3ms          125±2ms     1.09 
 other_benchmarks.limited_stream.benchmark.LimitedStreamBench.time_limited_stream_read
  116±1ms         132±20ms     1.14  
other_benchmarks.limited_stream.benchmark.LimitedStreamBench.time_limited_stream_read_8192
 82.1±3ms         95.0±9ms    ~1.16  
other_benchmarks.limited_stream.benchmark.LimitedStreamBench.time_limited_stream_readline
 89.1±5ms         89.2±7ms     1.00  
other_benchmarks.limited_stream.benchmark.LimitedStreamBench.time_limited_stream_readline_8192

At first, I had benchmarked all the methods together but I split them into separate methods, but still, the results mentioned in the (ticket)[https://code.djangoproject.com/ticket/33865#no1] differed from what I got, Are there any changes that I need to make to the benchmark?

@carltongibson
Copy link
Member Author

Hey @ngnpope — Hoping this is of interest to you! 🙂

@deepakdinesh1123 is working to push forward the work @smithdc1 began to use ASV to regularly run benchmarks on Django.

Seeing your PR on LimitedStream, it looked like a good test case.

I don't know if there's enough info (README etc.) for you to have a proper investigation here yet... 😬 but I thought I'd give you a ping.

(@deepakdinesh1123 I have a bit of admin for next week to do this morning, then I'll sit down with this again. Thanks! 👍)

@ngnpope
Copy link
Member

ngnpope commented Jul 27, 2022

Thanks @carltongibson. I'm actually trying to start writing these benchmarks for optimisations with this effort in mind. Looking forward to seeing this realised 👏🏻

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

No branches or pull requests

3 participants