-
Notifications
You must be signed in to change notification settings - Fork 213
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
Re-evaluate "http.response.zerocopysend" #423
Comments
The ones who know more about it are @synodriver and @abersheeran. |
The reason for facing buffers is because of the design of asyncio. It seems possible to solve this problem if we modify the zerocopysend standard so that it can only be sent at the end of the message. |
In terms of what this would like in practice, do you have a proposed interface? Trying to work backwards and see how/if to push through this issue. Lack of zero copy send is pretty painful for Whitenoise and ReactPy. |
Would an interface change allow h2 support without waiting on library support? |
I don't think it's possible. hypercorn relys on those sans-io projects to parse raw data. You can send data directly, but that will break h2 library's internal state machine. |
cc: @synodriver @pgjones @gi0baro As I have recently adopted the maintenance burden of WhiteNoise via my approved fork Before I start shooting PRs into the dark, I'd like the SW leads of the most popular webservers to help brainstorm on what the path forward is here. After revisiting this issue, my first thought was to effectively clone the WSGI async def application(scope, receive, send):
FileWrapper = scope["extensions"]["file_wrapper"]
# Returning a FileWrapper allows the webserver to handle the file with whatever it feels is the best solution.
return FileWrapper(open("example.css", "rb"), BLOCK_SIZE)
|
Not necessarily, the previous PR on uvicorn wasn't going to be acceptable due to the behaviour changes it introduced. Probably just needs someone to step up to it again. |
Not really, but I can only speak in regards of Granian here. Also I'd keep Granian out of the topic here, as:
Given these points, and also the fact Granian is quite young compared to other servers, I don't think In terms of the extension itself: I don't have any particular objection in how it was designed; I'd probably would only change the fact |
Second guessing, I'd assume that the intent there was for zero-copy-send to be usable with |
Isn't that the purpose of |
Yes the Worst case, I'd be okay with each webserver creating webserver-specific extensions outside the ASGI spec while the community decides what the best path forward is. |
Yes.
I can't speak for "here's how it should be supported", tho I would suggest dropping or otherwise sidelining the extension as currently designed. |
I'm realizing it's likely more computationally efficient to continue down the route of Path Send. I know the Path Send spec has already been formalized, but is it technologically feasible to add |
If there's some way to add it backwards-compatibly, then we can do that, but I can't immediately think of a perfect way to. |
If you're okay with cluttering the ASGI extension namespace, it could be implemented as a |
@Archmonger maybe it's just me, but I still miss the point here.. |
Pretty much all browsers use range requests when loading large media like audio and video. Safari/WebKit even goes further and fails to load videos if range requests are not supported. |
Those are not numbers from the project though. Browsers' behaviour doesn't really take a role in ASGI design. |
You were asking for numbers how many users use range requests (unless I misunderstood your question). I just wanted to note that for some scenarios 100% of users issue range requests (and many would see errors if the server would not offer range requests).
That seems like a good idea 👍 |
@gi0baro The relevant repo(s) I'm currently maintaining are production-grade static file servers built in Python. Currently, WSGI file transfers can be handled via WSGI's
Yup I agree that more ASGI webservers need to support the path send spec. To be fair, the spec is fairly new so it's not really surprising that all the webservers don't support it yet. All further replies related to Path Send should be made in the new issue. |
Now circling back around to My personal opinion is that the extension spec can/should be deprecated when an alternative exists that can be used for HTTP range requests, but of course the final call goes to the Django leads. I will keep this issue open in case any of the Django leads want to give any thoughts/remarks. |
To be fair, And this is also why it won't probably produce any performance gain in a pure Python ASGI server: when sending files back the main bottleneck is to read from the file descriptor, especially given there are no real unblocking I/O implementation out there except for uring, but it really depends on a specific Linux kernel version only. |
My approach to accepting extensions has been relatively open; I deliberately did not deconflict I'd like to see us move more towards |
Is it not possible for web servers to offload I have been under the impression that the "backend design" of |
To be clear: my point about performance was about the fact ASGI servers are async by nature, so even if you offload disk I/O to the OS, that call will be still blocking and thus potentially block the the event-loop (eg: Granian still reads from disk and writes to network in the user-space, it simply does that outside of the Python interpreter and thus the GIL). So @Archmonger you should probably take into account that, independently of the specific extension, this will probably need efforts in handling some complexity on the servers maintainer's side to be implemented, given your true aim here is increase the general I/O performance on files. |
Yep, on a related note - in my tests using a simple Ironically, this solution ended up being more performant than any of the async file IO libraries I found on PyPi so that solution is functionally indentical to what I would assume similar or greater performance improvements when using threading alongside There is a really nasty workaround for an |
Right now, none of the existing ASGI webservers support
zerocopysend
. This seems to be due to the additional code complexity needed to support this extension.This lack of support has prevented me from adding
zerocopysend
to whitenoise, which would have been a big win for the Python web development community.I'm opening this issue as a place to discuss what, if anything, can be done to encourage adoption for
zerocopysend
.Here's a few questions to get this discussion started:
zerocopysend2
interface needed to encourage adoption?asgiref
need some helper utilities to simplify adoption of the current extension?zerocopysend
would require facing the buffer. Is it worthwhile (or feasible) to engineer a way of avoiding this?The text was updated successfully, but these errors were encountered: