- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 33.3k
 
gh-129813, PEP 782: Use PyBytesWriter in bufferedio.c #138954
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Replace PyBytes_FromStringAndSize(NULL, size) and _PyBytes_Resize() with the new public PyBytesWriter API.
| 
           Benchmark: import pyperf
import os
file = open(__file__, "rb")
def func():
    file.seek(0)
    for _ in range(100):
        file.read1(1)
runner = pyperf.Runner()
runner.bench_func('bench', func)
file.close()Result:   | 
    
| 
           cc @cmaloney  | 
    
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍  looks good to me; could make the PyBytesWriter a little later and save some cleanup code (but would mean making it during the buffered lock)
| if (n == 0) | ||
| return PyBytes_FromStringAndSize(NULL, 0); | ||
| if (n == 0) { | ||
| return Py_GetConstant(Py_CONSTANT_EMPTY_BYTES); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comes up fairly frequently in the I/O code, thinking of making a _Py_EMPTY_BYTES internal macro for it (separate from this PR)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean a macro specific to the _io module?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was thinking pycore_bytesobject.h; currently shows up 24 times across the CPython source code. BufferedIO and a couple other places also use a pattern: (PyObject *)&_Py_SINGLETON(bytes_empty)
Move PyBytesWriter_Create() call
| 
           Merged, thanks for the review and suggestions @cmaloney.  | 
    
Replace PyBytes_FromStringAndSize(NULL, size) and _PyBytes_Resize() with the new public PyBytesWriter API.