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

Performance issue with Requests.php decompress() / compatible_gzinflate() functions #153

Open
adam-pro opened this issue Apr 24, 2015 · 3 comments

Comments

@adam-pro
Copy link

Currently the function: decompress() speculatively attempts the following in turn:

  1. gzdecode(...)
  2. gzinflate(...)
  3. self::compatible_gzinflate(...)
  4. gzuncompress(...)

However, #3 compatible_gzinflate(...) includes within it (at Lines 834 to 838) a "fallback" call to gzinflate(...) which is identical to that attempted at #2.

Hence if #2 fails. it may be uselessly repeated within #3.

One solution may be to do BOTH the following:

  • MOVE the "fallback" in compatible_gzinflate(...) to the beginning of that function (i.e. make it the first inflation attempted), and
  • REMOVE from decompress() function the gzinflate(...) attempt at Add PATCH method support #2.
@ozh
Copy link
Collaborator

ozh commented Apr 24, 2015

You should submit one pull request with all your proposed changes, and explain them, instead of filing a dozen separated issues... Just sayin' :)

@adam-pro
Copy link
Author

I'm new to github, but in my experience bugs/issues should be logged at an atomic level so they can be easily assessed / prioritised / managed.

In any event these aren't pull requests as I haven't actually made any code changes, I'm just highlighting them so the software author who knows his code can assess whether they have merit.

@ozh
Copy link
Collaborator

ozh commented Apr 24, 2015

Yep, you're right about keeping things on an atomic level. The beauty of pull requests, if done properly, is that you can "cherry pick" individual changes ("commits"). It also saves the author lot of time and increase vastly the chances that he'll actually test your proposed changes :)

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

2 participants