Speeding up the protocol parsing #2596
Conversation
redis/connection.py
Outdated
| ] | ||
| elif byte not in (b"-", b"+", b":", b"$", b"*"): |
There was a problem hiding this comment.
Maybe this line could be just elif byte != b"+":. All other comparisons are known to be true here because none of the previous ifs/elifs matched.
There was a problem hiding this comment.
@Ernest0x please take another look.. I've actually made it simpler.. and everything passes currently. But - I would love more eyes.
There was a problem hiding this comment.
@chayim looks good. A minor thing could be that int(response) which is done in several places. Perhaps a variable could be set for that.
There was a problem hiding this comment.
The problem is that doing that adds overhead and slows the parsing down (a bit, sure). This is one of those cases where the speed improvement dictates something that is less "PEP 1"... But then again, it follows the zen
Codecov ReportBase: 92.31% // Head: 92.26% // Decreases project coverage by
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## master #2596 +/- ##
==========================================
- Coverage 92.31% 92.26% -0.06%
==========================================
Files 115 115
Lines 29744 29738 -6
==========================================
- Hits 27459 27438 -21
- Misses 2285 2300 +15
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
@dvora-h any idea why this hiredis test is failing? Have you seen this before? It shouldn't in any way be connected to these changes. |
* speeding up the protocol parser * linting * changes to ease
* Fixed issue #2598 - make Document class subscriptable * Last time added older file, fixed it * retrigger checks * update json().arrindex() default values (#2611) * update json().arrindex() default values * add unit test * fix falsy checks * more unit tests * add asyncio tests * fix lint line length --------- Co-authored-by: Alex Schmitz <aschmitz@box.com> * Speeding up the protocol parsing (#2596) * speeding up the protocol parser * linting * changes to ease * Fixed CredentialsProvider examples (#2587) Co-authored-by: dvora-h <67596500+dvora-h@users.noreply.github.com> * ConnectionPool SSL example (#2605) * [types] update return type of smismember to list[int] (#2617) * update return type of smismember * use Literal instead of int * retrigger checks * Added test for document subscriptable in tests/test_search.py * Fixed linter issue * retrigger checks --------- Co-authored-by: Alex Schmitz <alex.schmitz@gmail.com> Co-authored-by: Alex Schmitz <aschmitz@box.com> Co-authored-by: Chayim <chayim@users.noreply.github.com> Co-authored-by: Bar Shaul <88437685+barshaul@users.noreply.github.com> Co-authored-by: dvora-h <67596500+dvora-h@users.noreply.github.com> Co-authored-by: CrimsonGlory <CrimsonGlory@users.noreply.github.com> Co-authored-by: Raymond Yin <raymond@tryevergreen.com>
This PR makes small changes to the protocol parsers. Specifically, this reduces the number of branches, and composite branches (if then if) that happen along the way.
As part of this change, fewer cast operations now happen, and the response comparison is (when necessary) only on the first byte of the response. This in turn makes the comparison cost lower (memory), and faster, due to the lack of repeated casting.
This is true of both async and sync.
There are no user facing changes.