Reduce per-row overhead in session recv and putValue dispatch#195
Reduce per-row overhead in session recv and putValue dispatch#195TheDistributor wants to merge 1 commit into
Conversation
Streamline the data-ingestion pipeline and outbound parameter encoding loop by eliminating high-overhead memory copies, optimizing primitive type comparisons, and increasing TCP window throughput. Detailed changes: - session: Request an opportunistic 1 MiB socket receive buffer (SO_RCVBUF) to minimize fragmented kernel reads across heavy wire payload streams. - session: Rewrite `__readFully` to pre-allocate a single message-sized bytearray. Populate it directly using zero-copy `sock.recv_into()` on a slicing memoryview, replacing an iterative chunk concatenation loop. - encodedsession: Update `recv()` and input stream targets to return and use raw mutable bytearray buffers directly, cutting down an extra copy. - encodedsession: Refactor `putValue` to use rapid pointer-level exact type comparisons (`type(v) is X`) for common database primitives (int, str, float, bool) before falling back to heavier hierarchy-aware `isinstance()` checks for the long tail.
madscientist
left a comment
There was a problem hiding this comment.
This looks good but please remember that, until our test environment is ported to Python3, we must preserve python2 compatibility.
The handling of bytes/bytearray is different between P2 and P3 and this difference is the cause of some of the complexity in this area. The Circle CI/CD tests only test Python 3: to check Python 2 you need to run the tests on our local systems, using nuopython.
If this is passing then these changes look good to me.
| # Preserve historic wire behaviour: bools encode as integers | ||
| # because the original isinstance(value, int) chain matched True | ||
| # and False before reaching the (dead) bool branch below. |
There was a problem hiding this comment.
Ugh. How does this work? I mean I understand that we want to preserve compatibility but doesn't this mean we can't send bool types?
Does the server interpret the integer as a bool based on the type?
| recv_into(), avoiding the repeated bytearray concatenations and the | ||
| final bytes() copy that the previous implementation performed. |
There was a problem hiding this comment.
We shouldn't justify this implementation comparing it to some previous implementaiton, in the docstring of the method. This information is appropriate for the git commit message, but not here.
recv_into, and drop the redundant bytes->bytearray copy in _exchangeMessages
fewer reads
direct type check before falling through to the slower isinstance chain
Nominal performance change on its own, but lays the groundwork for future changes.