From 15a8b5c8946de0231ef20831e8e5ffb025f55c54 Mon Sep 17 00:00:00 2001 From: Adam Hopkins Date: Sun, 21 Mar 2021 09:47:21 +0200 Subject: [PATCH 1/2] 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 21df65c0ef..9304175e95 100644 --- a/sanic/app.py +++ b/sanic/app.py @@ -676,27 +676,23 @@ async def handle_request(self, request: Request): 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 @@ async def handle_request(self, request: Request): 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 @@ async def _websocket_handler( ): 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 372c860851..b518e66ba1 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 @@ def __init__( # 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 @@ def __init__( 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 @@ def __init__( 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 @@ async def receive_body(self): 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 d0bad197af..83067c27c1 100644 --- a/sanic/router.py +++ b/sanic/router.py @@ -11,7 +11,6 @@ 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 @@ def _get( 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 @@ def get( # type: ignore 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 7d857bb3e0..a921d06306 100644 --- a/tests/benchmark/test_route_resolution_benchmark.py +++ b/tests/benchmark/test_route_resolution_benchmark.py @@ -23,18 +23,21 @@ async def test_resolve_route_no_arg_string_path( ) 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 @@ async def test_resolve_route_with_typed_args( ) 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 74598f914f..5b79b1e3f9 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 08a5e92cdf..60c0e76054 100644 --- a/tests/test_routes.py +++ b/tests/test_routes.py @@ -166,7 +166,9 @@ def handler(request): 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: From 6763e2bb0a390825e463abde6669631b9549ed01 Mon Sep 17 00:00:00 2001 From: Arthur Goldberg Date: Sun, 21 Mar 2021 09:09:31 +0100 Subject: [PATCH 2/2] fix?: recursion error on Sanic subclass initialisation (#2072) * fix?: recursion error on Sanic subclass init * tests: add test case for sanic subclass initialisation * Remove BaseSanic metaclass Co-authored-by: Adam Hopkins --- sanic/base.py | 27 ++++----------------------- sanic/blueprints.py | 1 + tests/test_app.py | 7 +++++++ 3 files changed, 12 insertions(+), 23 deletions(-) diff --git a/sanic/base.py b/sanic/base.py index 5170614950..368288c408 100644 --- a/sanic/base.py +++ b/sanic/base.py @@ -8,38 +8,19 @@ from sanic.mixins.signals import SignalMixin -class Base(type): - def __new__(cls, name, bases, attrs): - init = attrs.get("__init__") - - def __init__(self, *args, **kwargs): - nonlocal init - nonlocal name - - bases = [ - b for base in type(self).__bases__ for b in base.__bases__ - ] - - for base in bases: - base.__init__(self, *args, **kwargs) - - if init: - init(self, *args, **kwargs) - - attrs["__init__"] = __init__ - return type.__new__(cls, name, bases, attrs) - - class BaseSanic( RouteMixin, MiddlewareMixin, ListenerMixin, ExceptionMixin, SignalMixin, - metaclass=Base, ): __fake_slots__: Tuple[str, ...] + def __init__(self, *args, **kwargs) -> None: + for base in BaseSanic.__bases__: + base.__init__(self, *args, **kwargs) # type: ignore + def __str__(self) -> str: return f"<{self.__class__.__name__} {self.name}>" diff --git a/sanic/blueprints.py b/sanic/blueprints.py index f8b0323c00..85b60ddc18 100644 --- a/sanic/blueprints.py +++ b/sanic/blueprints.py @@ -73,6 +73,7 @@ def __init__( version: Optional[int] = None, strict_slashes: Optional[bool] = None, ): + super().__init__() self._apps: Set[Sanic] = set() self.ctx = SimpleNamespace() diff --git a/tests/test_app.py b/tests/test_app.py index bfe7900c23..f096496a47 100644 --- a/tests/test_app.py +++ b/tests/test_app.py @@ -405,3 +405,10 @@ def test_app_set_context(app): retrieved = Sanic.get_app(app.name) assert retrieved.ctx.foo == 1 + + +def test_subclass_initialisation(): + class CustomSanic(Sanic): + pass + + CustomSanic("test_subclass_initialisation")