Skip to content

bpo-32410: Implement loop.sock_sendfile() #4976

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.

Sign up for GitHub

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

Merged
asvetlov merged 54 commits into from
Jan 16, 2018
Merged

bpo-32410: Implement loop.sock_sendfile() #4976

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
ee13d5b
Draft for sock_sendfile
asvetlov Dec 22, 2017
8b610ad
Add test for blocked socket
asvetlov Dec 22, 2017
8d658be
Polish tests
asvetlov Dec 22, 2017
24c5a28
Test partial file content
asvetlov Dec 22, 2017
10b0b61
Add NEWS entry
asvetlov Dec 22, 2017
5f782a5
Add test for abstract sock_sendfile
asvetlov Dec 22, 2017
3068aa9
Polishment
asvetlov Dec 22, 2017
898766e
Don't call loop.set_debug(True)
asvetlov Dec 22, 2017
6e70a62
Revert set_debug() back
asvetlov Dec 22, 2017
20b3778
Work on tests
asvetlov Dec 22, 2017
7ed0e67
Improve test cleanup
asvetlov Dec 22, 2017
f4b61b1
Make tests stable
asvetlov Dec 23, 2017
6f6b94b
Merge remote-tracking branch 'upstream/master' into sock_sendfile
asvetlov Dec 24, 2017
1c05795
Refactor _check_sendfile_params helper
asvetlov Dec 24, 2017
f670fad
Use NotImplementedError in private socket.sendfile implementation.
asvetlov Dec 24, 2017
26f6d4a
Refactoring
asvetlov Dec 24, 2017
87d4804
Polish error text
asvetlov Dec 24, 2017
76eeef5
Update docs
asvetlov Dec 24, 2017
35071ea
Fix check for SOCK_STREAM
asvetlov Dec 24, 2017
92ae10b
Accept int fd along with socket instance
asvetlov Dec 24, 2017
8c451d2
Drop support for int FD for socket
asvetlov Dec 24, 2017
921fe69
NotImplementedError -> RuntimeError
asvetlov Dec 24, 2017
0f2a48f
Switch to RuntimeError back
asvetlov Dec 24, 2017
1cc0e8f
Merge branch 'master' into sock_sendfile
asvetlov Dec 30, 2017
272029e
Add sendfile fallback
asvetlov Dec 30, 2017
fa0954e
Fix private names
asvetlov Dec 30, 2017
0ddb410
Another renaming
asvetlov Dec 30, 2017
c18c3c8
Work on
asvetlov Dec 30, 2017
a4a174b
Fix NEWS.d
asvetlov Dec 30, 2017
448e949
More tests
asvetlov Dec 30, 2017
46c92ed
Polish docs
asvetlov Dec 31, 2017
8dd45dc
Revert changes in socket.py
asvetlov Dec 31, 2017
c9112b9
Merge branch 'master' into sock_sendfile
asvetlov Dec 31, 2017
b6273e4
More tests
asvetlov Dec 31, 2017
4d88063
Tests
asvetlov Dec 31, 2017
71b9f93
Test partial file with fallback
asvetlov Dec 31, 2017
db2445e
Improve test coverage
asvetlov Dec 31, 2017
46a6b46
Switch to custom exception type
asvetlov Dec 31, 2017
2ec48f8
read -> readinto
asvetlov Dec 31, 2017
8a6ed3f
Make tests more stable
asvetlov Dec 31, 2017
44da800
Merge remote-tracking branch 'upstream/master' into sock_sendfile
asvetlov Jan 1, 2018
967408e
Change base class for _SendfileNotAvailable to RuntimeError
asvetlov Jan 2, 2018
099dc56
Better exception type when sendfile is not available
asvetlov Jan 2, 2018
f9701cb
Add a test for mixed sock_send and sock_sendfile
asvetlov Jan 2, 2018
a30acc9
Add cancellation callback
asvetlov Jan 2, 2018
f7d9bab
More tests
asvetlov Jan 2, 2018
4d25927
Support tribool for fallback
asvetlov Jan 2, 2018
e303db9
Fix signature of abstract sock_sendfile
asvetlov Jan 2, 2018
84e1057
Merge branch 'master' into sock_sendfile
asvetlov Jan 2, 2018
657aa67
Revert back tribool for fallback
asvetlov Jan 16, 2018
dd4143a
Fix tests
asvetlov Jan 16, 2018
96d0032
Merge branch 'master' into sock_sendfile
asvetlov Jan 16, 2018
5deb0e2
Add a space
asvetlov Jan 16, 2018
3c9abaf
Fix sock_sendfile callback
asvetlov Jan 16, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Another renaming
  • Loading branch information
@asvetlov
asvetlov committed Dec 30, 2017
commit 0ddb410a51ab9e2897c1a8d922e71d09fabc56e7
6 changes: 3 additions & 3 deletions Lib/asyncio/base_events.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,17 +653,17 @@ async def sock_sendfile(self, sock, file, offset=0, count=None,
raise ValueError("the socket must be non-blocking")
socket._check_sendfile_params(sock, file, offset, count)
try:
await self._sock_sf_fast(sock, file, offset, count)
await self._sock_sendfile_native(sock, file, offset, count)
except RuntimeError:

@1st1 1st1 Dec 31, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

many things in sock_sendfile_native can cause a RuntimeError that this code will mask. I recommend to just add hasattr(os, 'sendfile')

@1st1 1st1 Dec 31, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or add a private exception class (not exported and with a leading underscore)

@asvetlov asvetlov Dec 31, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting migration.
Private exception class -> NotImplementedError -> RuntimeError -> private exception again.
I'm ok with custom exception, it is the best choice.

@1st1 1st1 Dec 31, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, Andrew ;) Sometimes it's hard to see the big picture when we just start working on a new big PR

Happy New Year, by the way!!

if fallback:
await self._sock_sf_fallback(sock, file, offset, count)
else:
raise

@1st1 1st1 Jan 2, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

raise RuntimeError('os.sendfile is not available') from None

@asvetlov asvetlov Jan 2, 2018

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposed message text is not always correct: sometimes os.sendfile is available but passed file object cannot be used by syscall.
_SendfileNotAvailable is derived from RuntimeError now and it always contains proper message text.
The best what I can do is reraising:
raise RuntimeError(exc.args[0]) from None

@1st1 1st1 Jan 2, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case let's make the exception public. I'd rename it to SendfileNotAvailableError and export it to asyncio module. Don't wrap it then.


async def _sock_sf_fast(self, sock, file, offset=0, count=None):
async def _sock_sendfile_native(self, sock, file, offset=0, count=None):
raise RuntimeError("Fast sendfile is not available")

async def _sock_sf_fallback(self, sock, file, offset=0, count=None):
async def _sock_send_fallback(self, sock, file, offset=0, count=None):
if offset:
file.seek(offset)
blocksize = min(count, 16384) if count else 16384
Expand Down
17 changes: 9 additions & 8 deletions Lib/asyncio/unix_events.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ async def create_unix_server(
ssl_handshake_timeout=ssl_handshake_timeout)
return server

async def _sock_sf_fast(self, sock, file, offset=0, count=None):
async def _sock_sendfile_native(self, sock, file, offset=0, count=None):
try:
os.sendfile
except AttributeError as exc:
Expand All @@ -320,12 +320,12 @@ async def _sock_sf_fast(self, sock, file, offset=0, count=None):

fut = self.create_future()
fd = sock.fileno()
self._sock_sf_fast_impl(fut, None, fd, fileno,
offset, count, blocksize, 0)
self._sock_sendfile_native_impl(fut, None, fd, fileno,
offset, count, blocksize, 0)
return await fut

def _sock_sf_fast_impl(self, fut, registered_fd, fd, fileno, offset,
count, blocksize, total_sent):
def _sock_sendfile_native_impl(self, fut, registered_fd, fd, fileno,
offset, count, blocksize, total_sent):
if registered_fd is not None:
# Remove the callback early. It should be rare that the
# selector says the fd is ready but the call still returns
Expand All @@ -345,7 +345,7 @@ def _sock_sf_fast_impl(self, fut, registered_fd, fd, fileno, offset,
try:
sent = os.sendfile(fd, fileno, offset, blocksize)
except (BlockingIOError, InterruptedError):
self.add_writer(fd, self._sock_sf_fast_impl, fut, fd, fd,
self.add_writer(fd, self._sock_sendfile_native_impl, fut, fd, fd,
fileno, offset, count, blocksize. total_sent)
except OSError as exc:
if total_sent == 0:
Expand All @@ -370,8 +370,9 @@ def _sock_sf_fast_impl(self, fut, registered_fd, fd, fileno, offset,
else:
offset += sent
total_sent += sent
self.add_writer(fd, self._sock_sf_fast_impl, fut, fd, fd,
fileno, offset, count, blocksize, total_sent)
self.add_writer(fd, self._sock_sendfile_native_impl, fut,
fd, fd, fileno,
offset, count, blocksize, total_sent)

def _update_filepos(self, fileno, offset, total_sent):

@1st1 1st1 Dec 30, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename to _sock_sendfile_update_filepos

if total_sent > 0:
Expand Down