Patchwork [4,of,4] worker: implement _blockingreader.readinto() (issue6444)

login
register
mail settings
Submitter Manuel Jacob
Date May 22, 2022, 12:37 a.m.
Message ID <643d03eb9cdf48513c0b.1653179834@tmp>
Download mbox | patch
Permalink /patch/50986/
State New
Headers show

Comments

Manuel Jacob - May 22, 2022, 12:37 a.m.
# HG changeset patch
# User Manuel Jacob <me@manueljacob.de>
# Date 1653171058 -7200
#      Sun May 22 00:10:58 2022 +0200
# Node ID 643d03eb9cdf48513c0bc639915f2c763d0635c5
# Parent  f8b3781ee6a2241fe40d5420f5d0293947f94022
# EXP-Topic worker-improvements
worker: implement _blockingreader.readinto() (issue6444)

The core logic for readinto() was already implemented in read(), so this is
mostly extracting that code into its own method.

Another fix for issue6444 was sent for the stable branch: https://foss.heptapod.net/mercurial/mercurial-devel/-/merge_requests/115
That is a minimal fix that implements readinto() only on Python versions that
require readinto() (3.8.0 and 3.8.1), which is the right approach for the
stable branch. However, I think that this patch has its value. It improves
performance in cases when pickle can use readinto(), it reduces code
duplication compared to the other patch, and by defining readinto() on all
Python versions, it makes behavior more consistent across all Python versions.
When merging stable into default, the other change can be dropped.

Patch

diff --git a/mercurial/worker.py b/mercurial/worker.py
--- a/mercurial/worker.py
+++ b/mercurial/worker.py
@@ -79,20 +79,12 @@ 
     def __init__(self, wrapped):
         self._wrapped = wrapped
 
-    # Do NOT implement readinto() by making it delegate to
-    # _wrapped.readinto(), since that is unbuffered. The unpickler is fine
-    # with just read() and readline(), so we don't need to implement it.
-
     def readline(self):
         return self._wrapped.readline()
 
-    # issue multiple reads until size is fulfilled (or EOF is encountered)
-    def read(self, size=-1):
-        if size < 0:
-            return self._wrapped.readall()
-
-        buf = bytearray(size)
+    def readinto(self, buf):
         pos = 0
+        size = len(buf)
 
         with memoryview(buf) as view:
             while pos < size:
@@ -102,7 +94,16 @@ 
                     break
                 pos += ret
 
-        del buf[pos:]
+        return pos
+
+    # issue multiple reads until size is fulfilled (or EOF is encountered)
+    def read(self, size=-1):
+        if size < 0:
+            return self._wrapped.readall()
+
+        buf = bytearray(size)
+        n_read = self.readinto(buf)
+        del buf[n_read:]
         return bytes(buf)