From 15a8b5c8946de0231ef20831e8e5ffb025f55c54 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Sun, 21 Mar 2021 09:47:21 +0200 Subject: [PATCH] Perf improv (#2074) * handler improvements for performance * Resovle tests * Linting * Add tests --- sanic/app.py | 25 +++++------- sanic/request.py | 24 +++++++++--- sanic/router.py | 23 +++-------- .../test_route_resolution_benchmark.py | 38 +++++++++++-------- tests/test_request.py | 21 ++++++++++ tests/test_routes.py | 4 +- 6 files changed, 79 insertions(+), 56 deletions(-) diff --git a/sanic/app.py b/sanic/app.py index 21df65c0..9304175e 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -676,27 +676,23 @@ class Sanic(BaseSanic): response = None try: # Fetch handler from router - ( - route, - handler, - kwargs, - ) = self.router.get(request) + route, handler, kwargs = self.router.get( + request.path, request.method, request.headers.get("host") + ) request._match_info = kwargs request.route = route - request.name = route.name - request.uri_template = f"/{route.path}" - request.endpoint = request.name if ( - request.stream - and request.stream.request_body + request.stream.request_body # type: ignore and not route.ctx.ignore_body ): if hasattr(handler, "is_stream"): # Streaming handler: lift the size limit - request.stream.request_max_size = float("inf") + request.stream.request_max_size = float( # type: ignore + "inf" + ) else: # Non-streaming handler: preload body await request.receive_body() @@ -730,8 +726,7 @@ class Sanic(BaseSanic): if response: response = await request.respond(response) else: - if request.stream: - response = request.stream.response + response = request.stream.response # type: ignore # Make sure that response is finished / run StreamingHTTP callback if isinstance(response, BaseHTTPResponse): @@ -757,9 +752,9 @@ class Sanic(BaseSanic): ): request.app = self if not getattr(handler, "__blueprintname__", False): - request.endpoint = handler.__name__ + request._name = handler.__name__ else: - request.endpoint = ( + request._name = ( getattr(handler, "__blueprintname__", "") + handler.__name__ ) diff --git a/sanic/request.py b/sanic/request.py index 372c8608..b518e66b 100644 --- a/sanic/request.py +++ b/sanic/request.py @@ -86,15 +86,14 @@ class Request: "_remote_addr", "_socket", "_match_info", + "_name", "app", "body", "conn_info", "ctx", - "endpoint", "head", "headers", "method", - "name", "parsed_args", "parsed_not_grouped_args", "parsed_files", @@ -106,7 +105,6 @@ class Request: "route", "stream", "transport", - "uri_template", "version", ) @@ -124,6 +122,7 @@ class Request: # TODO: Content-Encoding detection self._parsed_url = parse_url(url_bytes) self._id: Optional[Union[uuid.UUID, str, int]] = None + self._name: Optional[str] = None self.app = app self.headers = headers @@ -136,7 +135,6 @@ class Request: self.body = b"" self.conn_info: Optional[ConnInfo] = None self.ctx = SimpleNamespace() - self.name: Optional[str] = None self.parsed_forwarded: Optional[Options] = None self.parsed_json = None self.parsed_form = None @@ -147,12 +145,10 @@ class Request: self.parsed_not_grouped_args: DefaultDict[ Tuple[bool, bool, str, str], List[Tuple[str, str]] ] = defaultdict(list) - self.uri_template: Optional[str] = None self.request_middleware_started = False self._cookies: Optional[Dict[str, str]] = None self._match_info: Dict[str, Any] = {} self.stream: Optional[Http] = None - self.endpoint: Optional[str] = None self.route: Optional[Route] = None self._protocol = None @@ -207,6 +203,22 @@ class Request: if not self.body: self.body = b"".join([data async for data in self.stream]) + @property + def name(self): + if self._name: + return self._name + elif self.route: + return self.route.name + return None + + @property + def endpoint(self): + return self.name + + @property + def uri_template(self): + return f"/{self.route.path}" + @property def protocol(self): if not self._protocol: diff --git a/sanic/router.py b/sanic/router.py index d0bad197..83067c27 100644 --- a/sanic/router.py +++ b/sanic/router.py @@ -11,7 +11,6 @@ from sanic_routing.route import Route # type: ignore from sanic.constants import HTTP_METHODS from sanic.exceptions import MethodNotSupported, NotFound, SanicException from sanic.models.handler_types import RouteHandler -from sanic.request import Request ROUTER_CACHE_SIZE = 1024 @@ -27,16 +26,11 @@ class Router(BaseRouter): DEFAULT_METHOD = "GET" ALLOWED_METHODS = HTTP_METHODS - # Putting the lru_cache on Router.get() performs better for the benchmarsk - # at tests/benchmark/test_route_resolution_benchmark.py - # However, overall application performance is significantly improved - # with the lru_cache on this method. - @lru_cache(maxsize=ROUTER_CACHE_SIZE) def _get( - self, path, method, host + self, path: str, method: str, host: Optional[str] ) -> Tuple[Route, RouteHandler, Dict[str, Any]]: try: - route, handler, params = self.resolve( + return self.resolve( path=path, method=method, extra={"host": host}, @@ -50,14 +44,9 @@ class Router(BaseRouter): allowed_methods=e.allowed_methods, ) - return ( - route, - handler, - params, - ) - + @lru_cache(maxsize=ROUTER_CACHE_SIZE) def get( # type: ignore - self, request: Request + self, path: str, method: str, host: Optional[str] ) -> Tuple[Route, RouteHandler, Dict[str, Any]]: """ Retrieve a `Route` object containg the details about how to handle @@ -69,9 +58,7 @@ class Router(BaseRouter): correct response :rtype: Tuple[ Route, RouteHandler, Dict[str, Any]] """ - return self._get( - request.path, request.method, request.headers.get("host") - ) + return self._get(path, method, host) def add( # type: ignore self, diff --git a/tests/benchmark/test_route_resolution_benchmark.py b/tests/benchmark/test_route_resolution_benchmark.py index 7d857bb3..a921d063 100644 --- a/tests/benchmark/test_route_resolution_benchmark.py +++ b/tests/benchmark/test_route_resolution_benchmark.py @@ -23,18 +23,21 @@ class TestSanicRouteResolution: ) router, simple_routes = sanic_router(route_details=simple_routes) route_to_call = choice(simple_routes) + request = Request( + "/{}".format(route_to_call[-1]).encode(), + {"host": "localhost"}, + "v1", + route_to_call[0], + None, + None, + ) result = benchmark.pedantic( router.get, ( - Request( - "/{}".format(route_to_call[-1]).encode(), - {"host": "localhost"}, - "v1", - route_to_call[0], - None, - None, - ), + request.path, + request.method, + request.headers.get("host"), ), iterations=1000, rounds=1000, @@ -56,18 +59,21 @@ class TestSanicRouteResolution: ) print("{} -> {}".format(route_to_call[-1], url)) + request = Request( + "/{}".format(url).encode(), + {"host": "localhost"}, + "v1", + route_to_call[0], + None, + None, + ) result = benchmark.pedantic( router.get, ( - Request( - "/{}".format(url).encode(), - {"host": "localhost"}, - "v1", - route_to_call[0], - None, - None, - ), + request.path, + request.method, + request.headers.get("host"), ), iterations=1000, rounds=1000, diff --git a/tests/test_request.py b/tests/test_request.py index 74598f91..5b79b1e3 100644 --- a/tests/test_request.py +++ b/tests/test_request.py @@ -35,6 +35,27 @@ def test_request_id_defaults_uuid(): assert request.id == request.id == request._id +def test_name_none(): + request = Request(b"/", {}, None, "GET", None, None) + + assert request.name is None + + +def test_name_from_route(): + request = Request(b"/", {}, None, "GET", None, None) + route = Mock() + request.route = route + + assert request.name == route.name + + +def test_name_from_set(): + request = Request(b"/", {}, None, "GET", None, None) + request._name = "foo" + + assert request.name == "foo" + + @pytest.mark.parametrize( "request_id,expected_type", ( diff --git a/tests/test_routes.py b/tests/test_routes.py index 08a5e92c..60c0e760 100644 --- a/tests/test_routes.py +++ b/tests/test_routes.py @@ -166,7 +166,9 @@ def test_matching(path, headers, expected): request = Request(path, headers, None, "GET", None, app) try: - app.router.get(request=request) + app.router.get( + request.path, request.method, request.headers.get("host") + ) except NotFound: response = 404 except Exception: