Skip to content

Improve performance of query string parsing#1493

Merged
bdraco merged 7 commits intomasterfrom
query_string_parsing
Apr 5, 2025
Merged

Improve performance of query string parsing#1493
bdraco merged 7 commits intomasterfrom
query_string_parsing

Conversation

@bdraco
Copy link
Member

@bdraco bdraco commented Apr 3, 2025

What do these changes do?

When looking at the performance regression in #1492, I noticed that much of the time was spent in parse_qsl, however we do not need all the complexity of parse_sql since we use a very limited subset of the functionality.

We can write a faster version using the unquoter built-in to yarl with very little code.

Are there changes in behavior for the user?

no

Related issue number

n/a

Checklist

  • I think the code is well written
  • Unit tests for the changes exist
  • Documentation reflects the changes

When looking at the performance regression in #1492,
I noticed that much of the time was spent in parse_qsl,
however we do not need all the complexity of parse_sql
since we use a very limited subset of the functionality.

We can write a faster version using the unquoter built-in
to yarl with very little code.
@codspeed-hq
Copy link

codspeed-hq bot commented Apr 3, 2025

CodSpeed Performance Report

Merging #1493 will improve performances by 29.13%

Comparing query_string_parsing (563933c) with master (74f79ae)

Summary

⚡ 3 improvements
✅ 98 untouched benchmarks

Benchmarks breakdown

Benchmark BASE HEAD Change
test_parse_query_uncached[long] 29.8 ms 25.3 ms +17.81%
test_parse_query_uncached[short] 2.6 ms 2 ms +29.13%
test_update_query_string 752.4 µs 621.3 µs +21.1%

@codecov
Copy link

codecov bot commented Apr 3, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98.92%. Comparing base (74f79ae) to head (563933c).
Report is 44 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1493   +/-   ##
=======================================
  Coverage   98.92%   98.92%           
=======================================
  Files          32       32           
  Lines        6055     6070   +15     
  Branches      363      365    +2     
=======================================
+ Hits         5990     6005   +15     
  Misses         62       62           
  Partials        3        3           
Flag Coverage Δ
CI-GHA 98.92% <100.00%> (+<0.01%) ⬆️
MyPy 98.07% <100.00%> (+<0.01%) ⬆️
OS-Linux 98.78% <100.00%> (+<0.01%) ⬆️
OS-Windows 98.82% <100.00%> (+<0.01%) ⬆️
OS-macOS 98.56% <100.00%> (+<0.01%) ⬆️
Py-3.10.11 98.53% <100.00%> (+<0.01%) ⬆️
Py-3.10.16 98.74% <100.00%> (+<0.01%) ⬆️
Py-3.11.11 98.74% <100.00%> (+<0.01%) ⬆️
Py-3.11.9 98.53% <100.00%> (+<0.01%) ⬆️
Py-3.12.9 98.74% <100.00%> (+<0.01%) ⬆️
Py-3.13.2 98.74% <100.00%> (+<0.01%) ⬆️
Py-3.9.13 98.49% <100.00%> (+<0.01%) ⬆️
Py-3.9.21 98.70% <100.00%> (+<0.01%) ⬆️
Py-pypy7.3.16 98.69% <100.00%> (+<0.01%) ⬆️
Py-pypy7.3.19 98.71% <100.00%> (+<0.01%) ⬆️
VM-macos-latest 98.56% <100.00%> (+<0.01%) ⬆️
VM-ubuntu-latest 98.78% <100.00%> (+<0.01%) ⬆️
VM-windows-latest 98.82% <100.00%> (+<0.01%) ⬆️
pytest 98.78% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bdraco bdraco marked this pull request as ready for review April 3, 2025 18:36
@psf-chronographer psf-chronographer bot added the bot:chronographer:provided There is a change note present in this PR label Apr 3, 2025
@asvetlov
Copy link
Member

asvetlov commented Apr 4, 2025

Good idea!
I suspect all functions from urllib.parse could be replaced with faster ones.
The only big question is the compatibility; URL parsing is not a trivial thing.

@bdraco
Copy link
Member Author

bdraco commented Apr 4, 2025

Good idea!
I suspect all functions from urllib.parse could be replaced with faster ones.
The only big question is the compatibility; URL parsing is not a trivial thing.

We are down to very few urllib.parse calls left. The one in

return "".join(c if c.isprintable() else quote(c) for c in s)
could probably be replaced as well. That one is probably a bit more tricky since I'm not sure any of the quoters are directly compatible with urllib.parse.quote. However I don't expect human_quote is called frequently so I haven't tried to optimize it so much.

@bdraco bdraco merged commit 3bf614a into master Apr 5, 2025
48 of 50 checks passed
@bdraco bdraco deleted the query_string_parsing branch April 5, 2025 23:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bot:chronographer:provided There is a change note present in this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants