Repository: falconry/falcon
Pull request #1844 (2020-12-28 00:56:19)

perf(ASGI): cythonize falcon.asgi module tree

codecov[bot]: # [Codecov](https://codecov.io/gh/falconry/falcon/pull/1844?src=pr&el=h1) Report
> Merging [#1844](https://codecov.io/gh/falconry/falcon/pull/1844?src=pr&el=desc) (e59610d) into [master](https://codecov.io/gh/falconry/falcon/commit/29b05ed4a3635e917e324569596ba8682a8e19b8?el=desc) (29b05ed) will **decrease** coverage by `65.56%`.
> The diff coverage is `75.00%`.

[![Impacted file tree graph](https://codecov.io/gh/falconry/falcon/pull/1844/graphs/tree.svg?width=650&height=150&src=pr&token=zOtkfNQ3h6)](https://codecov.io/gh/falconry/falcon/pull/1844?src=pr&el=tree)

```diff
@@             Coverage Diff              @@
##            master    #1844       +/-   ##
============================================
- Coverage   100.00%   34.43%   -65.57%     
============================================
  Files           54       55        +1     
  Lines         5154     5164       +10     
  Branches       831      833        +2     
============================================
- Hits          5154     1778     -3376     
- Misses           0     3345     +3345     
- Partials         0       41       +41     
```


| [Impacted Files](https://codecov.io/gh/falconry/falcon/pull/1844?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [falcon/asgi/\_asgi\_helpers.py](https://codecov.io/gh/falconry/falcon/pull/1844/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvX2FzZ2lfaGVscGVycy5weQ==) | `57.14% <57.14%> (ø)` | |
| [falcon/asgi/app.py](https://codecov.io/gh/falconry/falcon/pull/1844/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvYXBwLnB5) | `20.36% <100.00%> (-79.64%)` | :arrow_down: |
| [falcon/util/reader.py](https://codecov.io/gh/falconry/falcon/pull/1844/diff?src=pr&el=tree#diff-ZmFsY29uL3V0aWwvcmVhZGVyLnB5) | `12.62% <0.00%> (-87.38%)` | :arrow_down: |
| [falcon/asgi/stream.py](https://codecov.io/gh/falconry/falcon/pull/1844/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvc3RyZWFtLnB5) | `13.58% <0.00%> (-86.42%)` | :arrow_down: |
| [falcon/asgi/structures.py](https://codecov.io/gh/falconry/falcon/pull/1844/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvc3RydWN0dXJlcy5weQ==) | `14.58% <0.00%> (-85.42%)` | :arrow_down: |
| [falcon/middleware.py](https://codecov.io/gh/falconry/falcon/pull/1844/diff?src=pr&el=tree#diff-ZmFsY29uL21pZGRsZXdhcmUucHk=) | `14.89% <0.00%> (-85.11%)` | :arrow_down: |
| [falcon/asgi/reader.py](https://codecov.io/gh/falconry/falcon/pull/1844/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvcmVhZGVyLnB5) | `16.00% <0.00%> (-84.00%)` | :arrow_down: |
| [falcon/testing/helpers.py](https://codecov.io/gh/falconry/falcon/pull/1844/diff?src=pr&el=tree#diff-ZmFsY29uL3Rlc3RpbmcvaGVscGVycy5weQ==) | `18.63% <0.00%> (-81.37%)` | :arrow_down: |
| [falcon/asgi/ws.py](https://codecov.io/gh/falconry/falcon/pull/1844/diff?src=pr&el=tree#diff-ZmFsY29uL2FzZ2kvd3MucHk=) | `22.66% <0.00%> (-77.34%)` | :arrow_down: |
| ... and [39 more](https://codecov.io/gh/falconry/falcon/pull/1844/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/falconry/falcon/pull/1844?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/falconry/falcon/pull/1844?src=pr&el=footer). Last update [29b05ed...e59610d](https://codecov.io/gh/falconry/falcon/pull/1844?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

vytas7: Merging, as it is overall a clear improvement (albeit a modest one), although we might need to remove that decorator, and skip cythonization of App instead... I realized what the main limitation of cythonizing coro funcs is. Other files in asgi/ are still good to keep cythonized.

Patch:
diff --git a/falcon/asgi/_asgi_helpers.py b/falcon/asgi/_asgi_helpers.py
new file mode 100644
index 0000000..44f26ab
--- /dev/null
+++ b/falcon/asgi/_asgi_helpers.py
@@ -0,0 +1,48 @@
+# Copyright 2020 by Vytautas Liuolia.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import inspect
+
+
+def _wrap_asgi_coroutine_func(asgi_impl):
+    """Wrap an ASGI application in another coroutine.
+
+    This utility is used to wrap the cythonized ``App.__call__`` in order to
+    masquerade it as a pure-Python coroutine function.
+
+    Conversely, if the ASGI callable is not detected as a coroutine function,
+    the application server might incorrectly assume an ASGI 2.0 application
+    (i.e., the double-callable style).
+
+    In case the app class is not cythonized, this function is a simple
+    passthrough of the original implementation.
+
+    Args:
+        asgi_impl(callable): An ASGI application class method.
+
+    Returns:
+        A pure-Python ``__call__`` implementation.
+    """
+
+    # NOTE(vytas): We are wrapping unbound class methods, hence the first
+    #   "self" parameter.
+    # NOTE(vytas): Intentionally not using functools.wraps as it erroneously
+    #   inherits the cythonized method's traits.
+    async def __call__(self, scope, receive, send):
+        await asgi_impl(self, scope, receive, send)
+
+    if inspect.iscoroutinefunction(asgi_impl):
+        return asgi_impl
+
+    return __call__
diff --git a/falcon/asgi/app.py b/falcon/asgi/app.py
index e0ff2dc..ab1a1d5 100644
--- a/falcon/asgi/app.py
+++ b/falcon/asgi/app.py
@@ -1,4 +1,4 @@
-# Copyright 2019 by Kurt Griffiths
+# Copyright 2019-2020 by Kurt Griffiths
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -40,6 +40,7 @@ from falcon.util.sync import (
     get_running_loop,
     wrap_sync_to_async,
 )
+from ._asgi_helpers import _wrap_asgi_coroutine_func
 from .multipart import MultipartForm
 from .request import Request
 from .response import Response
@@ -279,6 +280,7 @@ class App(falcon.app.App):
 
         self.add_error_handler(WebSocketDisconnected, self._ws_disconnected_error_handler)
 
+    @_wrap_asgi_coroutine_func
     async def __call__(self, scope, receive, send):  # noqa: C901
         try:
             asgi_info = scope['asgi']
@@ -809,7 +811,14 @@ class App(falcon.app.App):
                                      'method named "handle" that is a '
                                      'member of the given exception class.')
 
-        handler = _wrap_non_coroutine_unsafe(handler)
+        # NOTE(vytas): Do not shoot ourselves in the foot in case error
+        #   handlers are our own cythonized code.
+        if handler not in (
+            self._http_status_handler,
+            self._http_error_handler,
+            self._python_error_handler,
+        ):
+            handler = _wrap_non_coroutine_unsafe(handler)
 
         # NOTE(kgriffs): iscoroutinefunction() always returns False
         #   for cythonized functions.
diff --git a/setup.py b/setup.py
index 3c8e979..7e464ea 100644
--- a/setup.py
+++ b/setup.py
@@ -44,6 +44,7 @@ if CYTHON:
 
     package_names = [
         'falcon',
+        'falcon.asgi',
         'falcon.cyutil',
         'falcon.media',
         'falcon.routing',
@@ -65,6 +66,7 @@ if CYTHON:
         #
         # NOTE(vytas): It is pointless to cythonize reader.py, since cythonized
         #   Falcon is using reader.pyx instead.
+        'falcon.asgi._asgi_helpers',
         'falcon.hooks',
         'falcon.responders',
         'falcon.util.reader',


Test patch:
diff --git a/tests/asgi/test_asgi_helpers.py b/tests/asgi/test_asgi_helpers.py
new file mode 100644
index 0000000..83d8bf3
--- /dev/null
+++ b/tests/asgi/test_asgi_helpers.py
@@ -0,0 +1,35 @@
+import inspect
+
+import falcon
+from falcon.asgi import _asgi_helpers
+
+
+class IntricateApp:
+
+    def unorthodox_call(self, scope, receive, send):
+        return self._call_factory(scope, receive, send)
+
+    async def _call_factory(self, scope, receive, send):
+        await send('Hello!')
+        await send('Bye.')
+
+    __call__ = _asgi_helpers._wrap_asgi_coroutine_func(unorthodox_call)
+
+
+@falcon.runs_sync
+async def test_intricate_app():
+    async def receive():
+        pass
+
+    async def send(msg):
+        messages.append(msg)
+
+    app = IntricateApp()
+    messages = []
+
+    assert not inspect.iscoroutinefunction(app.unorthodox_call)
+    assert inspect.iscoroutinefunction(app.__call__)
+
+    await app({}, receive, send)
+
+    assert messages == ['Hello!', 'Bye.']