Draft: support zerocopysend asgi extension #20

synodriver · 2022-06-17T12:02:29Z

第一个原型，还会添加更多测试。

实现细节：

发送文件时完全使用os.open读取。这样做是因为一旦发现zerocopy不可用，还能使用os.read来读取为bytes，但是如果一开始就读取为bytes那是无论如何都不能再变回文件描述符了

synodriver · 2022-06-17T12:15:47Z

迭代文件描述符的时候使用了run_in_executor，在zerocopy不可用的时候提高效率

synodriver · 2022-06-17T12:18:15Z

这个中间件可以作为打开zerocopysend的开关来使用。

class UseSendFileMiddlware:
    def __init__(self, app, open=True):
        self.app = app
        self.open = open

    async def __call__(self, scope,receive,send):
        if not self.open:
            if "extensions" in scope and "http.response.zerocopysend" in scope["extensions"]:
                scope["extensions"].pop("http.response.zerocopysend")
        await self.app(scope,receive,send)

或许可以增加一个配置项？

synodriver · 2022-06-17T12:20:20Z

既然要集成进别的asgi框架，那logger应该也可以考虑下。目前的logger使用的比较散乱，是否可以添加一个app级别的logger统一使用？

rexzhang · 2022-06-17T12:26:10Z

zero-copy 是否在 ASGI 接口部分应该是服务启动后即可知道的.
应该可以一开始只是一个

这个中间件可以作为打开zerocopysend的开关来使用。

class UseSendFileMiddlware:
    def __init__(self, app, open=True):
        self.app = app
        self.open = open

    async def __call__(self, scope,receive,send):
        if not self.open:
            if "extensions" in scope and "http.response.zerocopysend" in scope["extensions"]:
                scope["extensions"].pop("http.response.zerocopysend")
        await self.app(scope,receive,send)

或许可以增加一个配置项？

使用配置来控制,这样也能方便 CLI 等方式来控制.

class Config():
    # response
    compression: Compression = Compression()
    cors: CORS = CORS()
    enable_dir_browser: bool = True
    enable_asgi_zero_copy: bool = True

代码部分检查这个新的控制变量来执行不同的行为

rexzhang · 2022-06-17T12:28:15Z

既然要集成进别的asgi框架，那logger应该也可以考虑下。目前的logger使用的比较散乱，是否可以添加一个app级别的logger统一使用？

这个开一个新的 Issue 讨论比较好,如果你有比较完整的想法的话

rexzhang · 2022-06-17T12:29:42Z

迭代文件描述符的时候使用了run_in_executor，在zerocopy不可用的时候提高效率

当 zerocopy 不可用时需要完全回落到以前的代码;也就是说 zerocopy 部分需要以可挂载的方式添加进来

synodriver · 2022-06-17T12:37:08Z

迭代文件描述符的时候使用了run_in_executor，在zerocopy不可用的时候提高效率

当 zerocopy 不可用时需要完全回落到以前的代码;也就是说 zerocopy 部分需要以可挂载的方式添加进来

目前是provider统一读取为文件描述符了，要完全回到原来那种很困难。主要是要考虑多方面因素

ASGI服务器是否支持zerocopysend？
用户是否配置了zerocopysend？
用户启用了zerocopysend但是ASGI服务器不支持怎么办？
目前的构架，往里面更改整个实现逻辑实在是有点困难了，一旦zerocopy不可用，还能使用os.read来读，但是bytes就不可能变成文件描述符的，基于此可以认为让FileSystemProvider返回fd: int是比较合理的

lgtm-com · 2022-06-17T12:56:56Z

This pull request introduces 5 alerts when merging 5257dec into 1b0dc6b - view on LGTM.com

new alerts:

5 for Module-level cyclic import

rexzhang · 2022-06-17T13:03:36Z

迭代文件描述符的时候使用了run_in_executor，在zerocopy不可用的时候提高效率

当 zerocopy 不可用时需要完全回落到以前的代码;也就是说 zerocopy 部分需要以可挂载的方式添加进来

目前是provider统一读取为文件描述符了，要完全回到原来那种很困难。主要是要考虑多方面因素
* ASGI服务器是否支持zerocopysend？

* 用户是否配置了zerocopysend？

* 用户启用了zerocopysend但是ASGI服务器不支持怎么办？

* 目前的构架，往里面更改整个实现逻辑实在是有点困难了，一旦zerocopy不可用，还能使用os.read来读，但是bytes就不可能变成文件描述符的，基于此可以认为让`FileSystemProvider`返回`fd: int`是比较合理的

前三条都是静态信息,启动完成后即确定的.架构的话,可以先确定需要在什么地方确定什么状态来调整

synodriver · 2022-06-17T13:05:53Z

ASGI服务器是否支持zerocopysend没法在服务器启动的时候知道，只有等第一个request来了以后去scope里面看

rexzhang · 2022-06-17T13:10:57Z

ASGI服务器是否支持zerocopysend没法在服务器启动的时候知道，只有等第一个request来了以后去scope里面看

这就有点离谱了,难道 ASGI 服务器在启动后不能确定自己是否支持 zerocopysend? 这个应该可以修改 ASGI 服务器来实现吧?
而且可以认定,当用户设置了开启 enable_asgi_zero_copy 选项,即表示用户明确的知道 ASGI 服务器支持;在这个设计下,这个选项默认关闭即可

synodriver · 2022-06-17T13:23:22Z

ASGI服务器是否支持zerocopysend没法在服务器启动的时候知道，只有等第一个request来了以后去scope里面看

这就有点离谱了,难道 ASGI 服务器在启动后不能确定自己是否支持 zerocopysend? 这个应该可以修改 ASGI 服务器来实现吧? 而且可以认定,当用户设置了开启 enable_asgi_zero_copy 选项,即表示用户明确的知道 ASGI 服务器支持;在这个设计下,这个选项默认关闭即可

因为asgiapp可以部署在各种asgi兼容服务器上，uvicorn, hypercorn, daphne, nginx unit, prehttp, nonecorn都可以，甚至还可以是个云函数，环境多种多样，app甚至都不知道他在什么服务器上面跑。正规做法是去scope判定，不过如果用户明确设置了这个，则可以认为用户知道这样做会有什么后果，那可以不处理异常了？

然后是目前的实现相关。在FileSystemProvider._do_get，要判断是否使用zerocopy就得从这开始了。但是目前提供的函数是DAVResponse.can_be_compressed，它的参数是header的一个字段和rule，可是在FileSystemProvider._do_get里面header还没生成呢，要硬改的话改不好会cycle import

rexzhang · 2022-06-17T13:31:52Z

因为asgiapp可以部署在各种asgi兼容服务器上，uvicorn, hypercorn, daphne, nginx unit, prehttp, nonecorn都可以，甚至还可以是个云函数，环境多种多样，app甚至都不知道他在什么服务器上面跑。正规做法是去scope判定，不过如果用户明确设置了这个，则可以认为用户知道这样做会有什么后果，那可以不处理异常了？

考虑到:会做性能调整的用户应该是高级用户的定位.只要在遇到明确是配置问题的地方抛出“配置/初始化”异常,同时退出服务;应该就可以了

synodriver · 2022-06-17T13:37:01Z

因为asgiapp可以部署在各种asgi兼容服务器上，uvicorn, hypercorn, daphne, nginx unit, prehttp, nonecorn都可以，甚至还可以是个云函数，环境多种多样，app甚至都不知道他在什么服务器上面跑。正规做法是去scope判定，不过如果用户明确设置了这个，则可以认为用户知道这样做会有什么后果，那可以不处理异常了？

考虑到:会做性能调整的用户应该是高级用户的定位.只要在遇到明确是配置问题的地方抛出“配置/初始化”异常,同时退出服务;应该就可以了

这个就简单了，把这个event原样扔给asgi服务器，asgi服务器收到不支持的事件自己会报错的

rexzhang · 2022-06-17T13:51:41Z

然后是目前的实现相关。在FileSystemProvider._do_get，要判断是否使用zerocopy就得从这开始了。但是目前提供的函数是DAVResponse.can_be_compressed，它的参数是header的一个字段和rule，可是在FileSystemProvider._do_get里面header还没生成呢，要硬改的话改不好会cycle import

判断是否会执行压缩其实是基于 response 的 header 中的 Content-Type 确定的. 而 Content-Type 是有 request 中 GET 方法需要获取的这个文件的文件名和文件格式来确定的.
也就是说在 FileSystemProvider._do_get() 中, 如果配置中启用了 ZCS 就做一个判断,然后根据判断后的结果执行 ZCS 或者回落执行以前的代码. 挂载的方式加入是完全可行的

rexzhang · 2022-06-17T13:53:03Z

另外, PR 的代码请先执行 isort 再执行 black 格式化代码后再提交. 相关配置我都是做好了的,只需要在项目根目录执行即可

rexzhang · 2022-06-17T13:56:15Z

ASGI服务器是否支持zerocopysend没法在服务器启动的时候知道，只有等第一个request来了以后去scope里面看

其实可以去 asgiref 提一个需求. 添加一个标准特性:提供一个 API, 允许在获取 ASGI 实例的当前支持的特性和扩展清单

synodriver · 2022-06-17T14:44:45Z

django组织的成员挺固执的……估计他们不会听，原来我那个trailingheader的扩展就没通过，我就自己实现了

lgtm-com · 2022-06-17T16:25:13Z

This pull request introduces 7 alerts when merging 6d353c1 into 1b0dc6b - view on LGTM.com

new alerts:

6 for Module-level cyclic import
1 for Unused import

lgtm-com · 2022-06-17T20:09:59Z

This pull request introduces 1 alert when merging 41272e2 into 1b0dc6b - view on LGTM.com

new alerts:

1 for Unused import

lgtm-com · 2022-06-19T07:22:15Z

This pull request introduces 1 alert when merging 28a236b into 1b0dc6b - view on LGTM.com

new alerts:

1 for Module is imported with 'import' and 'import from'

synodriver · 2022-06-28T16:11:54Z

单元测试中的get_response_content需要修改一下，现在response._content可能是异步生成器也可能是zerocopy的fd，或者默认enable_asgi_zero_copy: bool是false？这里做了一个假设：用户会使用相应的asgi服务器来部署，不过看情况不能这样，还是应该保守一些

rexzhang · 2022-07-01T17:13:42Z

单元测试中的get_response_content需要修改一下，现在response._content可能是异步生成器也可能是zerocopy的fd，或者默认enable_asgi_zero_copy: bool是false？这里做了一个假设：用户会使用相应的asgi服务器来部署，不过看情况不能这样，还是应该保守一些

需要如何改?欢迎直接提方案

rexzhang · 2022-07-01T17:15:20Z

单元测试部分,可以先完善 PR 涉及的部分,有冲突部分后续解决也可.并行推进效率高很多

synodriver · 2022-07-11T16:10:28Z

相关的单元测试加了个patch，应该好了

codecov · 2022-07-12T07:33:56Z

Codecov Report

Attention: Patch coverage is 43.01075% with 53 lines in your changes are missing coverage. Please review.

Project coverage is 65.25%. Comparing base (ac3161d) to head (e4b9946).
Report is 5 commits behind head on main.

❗ Current head e4b9946 differs from pull request most recent head df4e798. Consider uploading reports for the commit df4e798 to get more accurate results

Files	Patch %	Lines
asgi_webdav/response.py	42.50%	23 Missing ⚠️
asgi_webdav/helpers.py	26.92%	19 Missing ⚠️
asgi_webdav/provider/file_system.py	56.00%	11 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #20      +/-   ##
==========================================
- Coverage   65.99%   65.25%   -0.75%     
==========================================
  Files          24       24              
  Lines        2770     2849      +79     
==========================================
+ Hits         1828     1859      +31     
- Misses        942      990      +48

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

rexzhang · 2022-07-14T04:40:58Z

可否添加些性能对比测试? uvicorn/nonecorn 打开 ZeroCopy/nonecorn 关闭 ZeroCopy

synodriver · 2022-07-14T06:30:55Z

我做过一般网站的性能测试，确实可以有数倍的提升

rexzhang · 2022-07-14T06:39:00Z

可否针对这个项目做一个如上三个环境的对比性能测试? 可以放在 performance/asgi_zero_copy 目录下

rexzhang · 2022-07-14T06:39:59Z

测试结果以文本文件的方式存储,可以简单的(在命令行)重定向输出到文件

synodriver · 2022-07-14T06:41:57Z

可以写一个对应的配置文件由用户自己测试吗？或者是由action测试？

rexzhang · 2022-07-14T06:44:47Z

不需要 action, 简单说就是在

performance/asgi_zero_copy 目录下创建一个环境,任何人 clone 了整个代码库后可以自行开始测试,并获得测试结果
同时也在这个目录下,将你测试的结果作为展示给不打算自己测试的人

rexzhang · 2022-07-14T06:46:12Z

可以有自己的 requirements 等等,可以理解为某种 demo 子项目

synodriver · 2022-07-14T06:47:59Z

可以，正好我魔改过一个异步的webdav库，异步对异步，这个应该可以作为测试的依赖吧

rexzhang · 2022-07-14T06:50:20Z

可以，正好我魔改过一个异步的webdav库，异步对异步，这个应该可以作为测试的依赖吧

这个随意.最关键的是需要添加了 asgi-zero-copy 支持的 asgi-webdav 在上面三个环境下的对比测试,这个最具有说服力

rexzhang · 2022-11-26T10:28:21Z

@synodriver 性测试部分进展得如何了?

synodriver · 2022-12-03T08:18:51Z

东西差不多有了，不过最近在肝论文，没push（：

rexzhang · 2022-12-05T04:43:29Z

东西差不多有了，不过最近在肝论文，没push（：

加油,我要做些web端辅助工具相关的功能,可能会动response部分

synodriver · 2022-12-05T05:03:58Z

没事，我这边跟着merge就行

for more information, see https://pre-commit.ci

synodriver added 2 commits June 17, 2022 18:14

draft: support zerocopysend asgi extension

5257dec

Merge branch 'main' into zerocopy

63e70d0

refactory

5e75434

failback to origin handler

6d353c1

fix cycle import

41272e2

fix default value for zerocopydata

28a236b

Merge branch 'rexzhang:main' into zerocopy

dddc278

rexzhang linked an issue Jun 28, 2022 that may be closed by this pull request

support sendfile extension in asgi spec #8

Open

synodriver and others added 3 commits July 11, 2022 23:18

Merge branch 'rexzhang:main' into zerocopy

37f20e0

fix unitest

eeff928

Merge upstream into zerocopy

d4646db

synodriver and others added 5 commits July 1, 2023 18:04

merge upstream

41e07b4

[pre-commit.ci] auto fixes from pre-commit.com hooks

e4b9946

for more information, see https://pre-commit.ci

Merge branch 'main' into zerocopy

b4539e6

merge from upstream

9886b96

Merge branch 'temp' into zerocopy

df4e798

Draft: support zerocopysend asgi extension #20

Are you sure you want to change the base?

Draft: support zerocopysend asgi extension #20

Conversation

synodriver commented Jun 17, 2022

synodriver commented Jun 17, 2022

synodriver commented Jun 17, 2022

synodriver commented Jun 17, 2022

rexzhang commented Jun 17, 2022

rexzhang commented Jun 17, 2022

rexzhang commented Jun 17, 2022

synodriver commented Jun 17, 2022

lgtm-com bot commented Jun 17, 2022

rexzhang commented Jun 17, 2022

synodriver commented Jun 17, 2022

rexzhang commented Jun 17, 2022

synodriver commented Jun 17, 2022

rexzhang commented Jun 17, 2022

synodriver commented Jun 17, 2022

rexzhang commented Jun 17, 2022

rexzhang commented Jun 17, 2022

rexzhang commented Jun 17, 2022

synodriver commented Jun 17, 2022

lgtm-com bot commented Jun 17, 2022

lgtm-com bot commented Jun 17, 2022

lgtm-com bot commented Jun 19, 2022

synodriver commented Jun 28, 2022 • edited Loading

rexzhang commented Jul 1, 2022

rexzhang commented Jul 1, 2022

synodriver commented Jul 11, 2022

codecov bot commented Jul 12, 2022 • edited Loading

Codecov Report

rexzhang commented Jul 14, 2022

synodriver commented Jul 14, 2022

rexzhang commented Jul 14, 2022

rexzhang commented Jul 14, 2022

synodriver commented Jul 14, 2022

rexzhang commented Jul 14, 2022 • edited Loading

rexzhang commented Jul 14, 2022

synodriver commented Jul 14, 2022

rexzhang commented Jul 14, 2022 • edited Loading

rexzhang commented Nov 26, 2022

synodriver commented Dec 3, 2022

rexzhang commented Dec 5, 2022

synodriver commented Dec 5, 2022

synodriver commented Jun 28, 2022 •

edited

Loading

codecov bot commented Jul 12, 2022 •

edited

Loading

rexzhang commented Jul 14, 2022 •

edited

Loading

rexzhang commented Jul 14, 2022 •

edited

Loading