Skip to content

Commit

Permalink
Performance: remove methodtools/wirerope, use direct lru_cache()
Browse files Browse the repository at this point in the history
  • Loading branch information
vdboor committed Jul 8, 2024
1 parent a91e22f commit 142d754
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 15 deletions.
1 change: 0 additions & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ install_requires =
requests
jinja2
mappyfile
methodtools
jsonpath-rw
orjson
more-ds
Expand Down
31 changes: 31 additions & 0 deletions src/schematools/_utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
"""Internal utils, not meant to be used outside schematools."""

import functools
import weakref


def cached_method(*lru_args, **lru_kwargs):
"""A simple lru-cache per object.
This removed the need for methodtools.lru_cache(), which uses wirerope for purity.
The usage of wirerope started showing up as 5% of the request time, hence it's significant to remove.
"""

def decorator(func):
@functools.wraps(func)
def initial_wrapped_func(self, *args, **kwargs):
# Not storing the wrapped method inside the instance. If we had
# a strong reference to self the instance would never die.
self_weak = weakref.ref(self)

@functools.wraps(func)
@functools.lru_cache(*lru_args, **lru_kwargs)
def cached_method(*args, **kwargs):
return func(self_weak(), *args, **kwargs)

# Assigns to the self reference (preserving the cache), and optimizes the next access.
setattr(self, func.__name__, cached_method)
return cached_method(*args, **kwargs)

return initial_wrapped_func

return decorator
16 changes: 8 additions & 8 deletions src/schematools/permissions/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,12 @@
The :class:`UserScopes` class handles whether a dataset, table or field can be accessed.
The other classes in this module ease to retrieval of permission objects.
"""

from __future__ import annotations

from typing import Iterable, Iterator

import methodtools

from schematools._utils import cached_method
from schematools.types import (
DatasetFieldSchema,
DatasetSchema,
Expand Down Expand Up @@ -74,15 +74,15 @@ def add_query_params(self, params: list[str]):
"""
self._query_param_names.extend(params)

@methodtools.lru_cache() # type: ignore[misc]
@cached_method() # type: ignore[misc]
def has_all_scopes(self, needed_scopes: frozenset[str]) -> bool:
"""Check whether the request has all scopes.
This performs an AND check: all scopes should be present.
"""
return self._scopes.issuperset(needed_scopes)

@methodtools.lru_cache() # type: ignore[misc]
@cached_method() # type: ignore[misc]
def has_any_scope(self, needed_scopes: frozenset[str]) -> bool:
"""Check whether the request grants one of the given scopes.
Expand Down Expand Up @@ -151,7 +151,7 @@ def _has_field_auth_access(self, field: DatasetFieldSchema) -> Permission:
else:
return Permission.none

@methodtools.lru_cache()
@cached_method()
def _has_dataset_profile_access(self, dataset_id: str) -> Permission:
"""Give the permission access level for a dataset, as defined by the profile."""
return max(
Expand All @@ -162,7 +162,7 @@ def _has_dataset_profile_access(self, dataset_id: str) -> Permission:
default=Permission.none,
)

@methodtools.lru_cache()
@cached_method()
def _has_table_profile_access(self, table: DatasetTableSchema) -> Permission:
"""Give the permission level for a table.
Expand Down Expand Up @@ -239,7 +239,7 @@ def _has_field_profile_access(self, field: DatasetFieldSchema) -> Permission:

return max_permission

@methodtools.lru_cache()
@cached_method()
def get_active_profile_datasets(self, dataset_id: str) -> list[ProfileDatasetSchema]:
"""Find all profiles that mention a dataset and match the scopes.
Expand All @@ -260,7 +260,7 @@ def get_active_profile_datasets(self, dataset_id: str) -> list[ProfileDatasetSch
and (profile_dataset := profile.datasets.get(dataset_id)) is not None
]

@methodtools.lru_cache()
@cached_method()
def get_active_profile_tables(
self, dataset_id: str, table_id: str
) -> list[ProfileTableSchema]:
Expand Down
11 changes: 5 additions & 6 deletions src/schematools/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,8 @@
cast,
)

from methodtools import lru_cache

from schematools import MAX_TABLE_NAME_LENGTH
from schematools._utils import cached_method
from schematools.exceptions import (
DatasetFieldNotFound,
DatasetTableNotFound,
Expand Down Expand Up @@ -492,7 +491,7 @@ def _get_tables(self, include_nested: bool = False, include_through: bool = Fals
if include_through:
yield from self.through_tables

@lru_cache() # type: ignore[misc]
@cached_method() # type: ignore[misc]
def get_table_by_id(
self, table_id: str, include_nested: bool = True, include_through: bool = True
) -> DatasetTableSchema:
Expand Down Expand Up @@ -931,7 +930,7 @@ def get_db_fields(self) -> Iterator[DatasetFieldSchema]:
continue
yield field

@lru_cache() # type: ignore[misc]
@cached_method() # type: ignore[misc]
def get_field_by_id(self, field_id: str) -> DatasetFieldSchema:
"""Get a fields based on the ids of the field."""
for field_schema in self.fields:
Expand All @@ -940,7 +939,7 @@ def get_field_by_id(self, field_id: str) -> DatasetFieldSchema:

raise DatasetFieldNotFound(f"Field '{field_id}' does not exist in table '{self.id}'.")

@lru_cache() # type: ignore[misc]
@cached_method() # type: ignore[misc]
def get_additional_relation_by_id(self, relation_id: str) -> AdditionalRelationSchema:
"""Get the reverse relation based on the ids of the relation."""
for additional_relation in self.additional_relations:
Expand Down Expand Up @@ -1631,7 +1630,7 @@ def field_items(self) -> Json | None:
"""Return the item definition for an array type."""
return self.get("items", {}) if self.is_array else None

@lru_cache() # type: ignore[misc]
@cached_method() # type: ignore[misc]
def get_field_by_id(self, field_id: str) -> DatasetFieldSchema:
"""Finds and returns the subfield with the given id.
Expand Down

0 comments on commit 142d754

Please sign in to comment.