Title: gh-130167: Improve speed of `ftplib.parse150` by replacing `re` by donbarbos · Pull Request #130243 · python/cpython · GitHub
Open Graph Title: gh-130167: Improve speed of `ftplib.parse150` by replacing `re` by donbarbos · Pull Request #130243 · python/cpython
X Title: gh-130167: Improve speed of `ftplib.parse150` by replacing `re` by donbarbos · Pull Request #130243 · python/cpython
Description: I also suggest adding tests for this function because they are missing in the Lib/test/test_ftplib.py file. I wrote tests (would be in next comment) and benchmarks (results here) parse150_bench.py: from Lib.ftplib import error_reply import timeit # Regex version _150_re = None def parse150_re(resp): if resp[:3] != "150": raise error_reply(resp) global _150_re if _150_re is None: import re _150_re = re.compile(r"150 .* \((\d+) bytes\)", re.IGNORECASE | re.ASCII) m = _150_re.match(resp) if not m: return None return int(m.group(1)) # No regex version def parse150(resp): if not resp.startswith("150"): raise error_reply(resp) start = resp.find("(") end = resp.find(" bytes)") if start == -1 or end == -1 or start >= end: return None try: return int(resp[start + 1 : end]) except ValueError: return None test_cases = [ "150 Opening BINARY mode data connection (4096 bytes)", "150 Transfer starting (32768 bytes)", "150 Data connection accepted (131072 bytes)", "150 Some random message without bytes", "150 Large file transfer (987654321 bytes)", "150 Small file (1 bytes)", "150 Transfer starting (99999999 bytes)", "150 Weird case (42 bytes) with extra text", ] for case in test_cases: regex_time = timeit.timeit(lambda: parse150_re(case), number=1_000_000) no_regex_time = timeit.timeit(lambda: parse150(case), number=1_000_000) print(f"Test case: {case}") print(f" Regex version: {regex_time:.6f} sec") print(f" No regex version: {no_regex_time:.6f} sec") Results for different cases (from x1.82 to x1.92 as fast): $ ./python -B parse150_bench.py Test case: 150 Opening BINARY mode data connection (4096 bytes) Regex version: 1.147499 sec No regex version: 0.630863 sec Test case: 150 Transfer starting (32768 bytes) Regex version: 1.113087 sec No regex version: 0.610494 sec Test case: 150 Data connection accepted (131072 bytes) Regex version: 1.140198 sec No regex version: 0.616784 sec Test case: 150 Some random message without bytes Regex version: 0.643958 sec No regex version: 0.302508 sec Test case: 150 Large file transfer (987654321 bytes) Regex version: 1.160474 sec No regex version: 0.605776 sec Test case: 150 Small file (1 bytes) Regex version: 1.005945 sec No regex version: 0.522877 sec Test case: 150 Transfer starting (99999999 bytes) Regex version: 1.147927 sec No regex version: 0.616777 sec Test case: 150 Weird case (42 bytes) with extra text Regex version: 1.140925 sec No regex version: 0.592486 sec Issue: gh-130167
Open Graph Description: I also suggest adding tests for this function because they are missing in the Lib/test/test_ftplib.py file. I wrote tests (would be in next comment) and benchmarks (results here) parse150_bench.py:...
X Description: I also suggest adding tests for this function because they are missing in the Lib/test/test_ftplib.py file. I wrote tests (would be in next comment) and benchmarks (results here) parse150_bench.py:...
Opengraph URL: https://github.com/python/cpython/pull/130243
X: @github
Domain: github.com
| route-pattern | /:user_id/:repository/pull/:id/files(.:format) |
| route-controller | pull_requests |
| route-action | files |
| fetch-nonce | v2:bb514d24-9339-8afd-a7eb-e0921bc1735f |
| current-catalog-service-hash | ae870bc5e265a340912cde392f23dad3671a0a881730ffdadd82f2f57d81641b |
| request-id | D464:3A35C6:1C7C405:2580B30:696AF9F4 |
| html-safe-nonce | f1baa64b75f27a3f1d43616ed0ec677c83e5005422272356e54176b05eb4e125 |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiJENDY0OjNBMzVDNjoxQzdDNDA1OjI1ODBCMzA6Njk2QUY5RjQiLCJ2aXNpdG9yX2lkIjoiNjUxMzY1OTcxNjgyOTkwMzM0OCIsInJlZ2lvbl9lZGdlIjoiaWFkIiwicmVnaW9uX3JlbmRlciI6ImlhZCJ9 |
| visitor-hmac | 2b013fa6e8a0899e3841b99b1bbdc8e857c27dda23d53fdfff1e71cdcd3c7d23 |
| hovercard-subject-tag | pull_request:2340839963 |
| github-keyboard-shortcuts | repository,pull-request-list,pull-request-conversation,pull-request-files-changed,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/python/cpython/pull/130243/files |
| twitter:image | https://avatars.githubusercontent.com/u/47272787?s=400&v=4 |
| twitter:card | summary_large_image |
| og:image | https://avatars.githubusercontent.com/u/47272787?s=400&v=4 |
| og:image:alt | I also suggest adding tests for this function because they are missing in the Lib/test/test_ftplib.py file. I wrote tests (would be in next comment) and benchmarks (results here) parse150_bench.py:... |
| og:site_name | GitHub |
| og:type | object |
| hostname | github.com |
| expected-hostname | github.com |
| None | 5f99f7c1d70f01da5b93e5ca90303359738944d8ab470e396496262c66e60b8d |
| turbo-cache-control | no-preview |
| diff-view | unified |
| go-import | github.com/python/cpython git https://github.com/python/cpython.git |
| octolytics-dimension-user_id | 1525981 |
| octolytics-dimension-user_login | python |
| octolytics-dimension-repository_id | 81598961 |
| octolytics-dimension-repository_nwo | python/cpython |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 81598961 |
| octolytics-dimension-repository_network_root_nwo | python/cpython |
| turbo-body-classes | logged-out env-production page-responsive full-width |
| disable-turbo | true |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | 82560a55c6b2054555076f46e683151ee28a19bc |
| ui-target | canary-2 |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width