Skip to content

Commit a7bc43d

Browse files
blothmannblothmannpre-commit-ci[bot]patrick91
authored
Improve performance of convert_argument for list of primitives (#3773)
* Improve performance of `convert_argument` for list of primitives (scalars, optional scalars, enums) * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * refactor bad naming * refactor to _is_leaf_type, which is compatible to graphql-core wording * refactor _is_optional_leaf_type to prevent nested type_.of_type.of_type * Enhance test for optional elements in lists. * Remove list * backwards compat * Lint --------- Co-authored-by: blothmann <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Patrick Arminio <[email protected]>
1 parent 69fa221 commit a7bc43d

File tree

4 files changed

+89
-6
lines changed

4 files changed

+89
-6
lines changed

RELEASE.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Release type: patch
2+
3+
This release improves performance of argument conversion for lists of primitives.

strawberry/types/arguments.py

Lines changed: 51 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -144,12 +144,52 @@ def is_maybe(self) -> bool:
144144
return isinstance(self.type, StrawberryMaybe)
145145

146146

147+
def _is_leaf_type(
148+
type_: Union[StrawberryType, type],
149+
scalar_registry: Mapping[object, Union[ScalarWrapper, ScalarDefinition]],
150+
skip_classes: tuple[type, ...] = (),
151+
) -> bool:
152+
if type_ in skip_classes:
153+
return False
154+
155+
if is_scalar(type_, scalar_registry):
156+
return True
157+
158+
if isinstance(type_, EnumDefinition):
159+
return True
160+
161+
if isinstance(type_, LazyType):
162+
return _is_leaf_type(type_.resolve_type(), scalar_registry)
163+
164+
if hasattr(type_, "_enum_definition"):
165+
enum_definition: EnumDefinition = type_._enum_definition
166+
return _is_leaf_type(enum_definition, scalar_registry)
167+
168+
return False
169+
170+
171+
def _is_optional_leaf_type(
172+
type_: Union[StrawberryType, type],
173+
scalar_registry: Mapping[object, Union[ScalarWrapper, ScalarDefinition]],
174+
skip_classes: tuple[type, ...] = (),
175+
) -> bool:
176+
if type_ in skip_classes:
177+
return False
178+
179+
if isinstance(type_, StrawberryOptional):
180+
return _is_leaf_type(type_.of_type, scalar_registry, skip_classes)
181+
182+
return False
183+
184+
147185
def convert_argument(
148186
value: object,
149187
type_: Union[StrawberryType, type],
150188
scalar_registry: Mapping[object, Union[ScalarWrapper, ScalarDefinition]],
151189
config: StrawberryConfig,
152190
) -> object:
191+
from strawberry.relay.types import GlobalID
192+
153193
# TODO: move this somewhere else and make it first class
154194
if isinstance(type_, StrawberryOptional):
155195
res = convert_argument(value, type_.of_type, scalar_registry, config)
@@ -164,22 +204,27 @@ def convert_argument(
164204

165205
if isinstance(type_, StrawberryList):
166206
value_list = cast("Iterable", value)
207+
208+
if _is_leaf_type(
209+
type_.of_type, scalar_registry, skip_classes=(GlobalID,)
210+
) or _is_optional_leaf_type(
211+
type_.of_type, scalar_registry, skip_classes=(GlobalID,)
212+
):
213+
return value_list
214+
215+
value_list = cast("Iterable", value)
216+
167217
return [
168218
convert_argument(x, type_.of_type, scalar_registry, config)
169219
for x in value_list
170220
]
171221

172-
if is_scalar(type_, scalar_registry):
173-
from strawberry.relay.types import GlobalID
174-
222+
if _is_leaf_type(type_, scalar_registry):
175223
if type_ is GlobalID:
176224
return GlobalID.from_id(value) # type: ignore
177225

178226
return value
179227

180-
if isinstance(type_, EnumDefinition):
181-
return value
182-
183228
if isinstance(type_, LazyType):
184229
return convert_argument(value, type_.resolve_type(), scalar_registry, config)
185230

tests/benchmarks/test_arguments.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
import pytest
2+
from pytest_codspeed import BenchmarkFixture
3+
4+
from strawberry.schema.config import StrawberryConfig
5+
from strawberry.schema.types.scalar import DEFAULT_SCALAR_REGISTRY
6+
from strawberry.types.arguments import convert_argument
7+
from strawberry.types.base import StrawberryList
8+
9+
10+
@pytest.mark.parametrize("ntypes", [2**k for k in range(14, 23, 2)])
11+
def test_convert_argument_large_list(benchmark: BenchmarkFixture, ntypes):
12+
test_value = list(range(ntypes))
13+
type_ = StrawberryList(int)
14+
15+
def run():
16+
result = convert_argument(
17+
test_value, type_, DEFAULT_SCALAR_REGISTRY, StrawberryConfig()
18+
)
19+
assert test_value == result
20+
21+
benchmark(run)

tests/utils/test_arguments_converter.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ def test_list():
5656
args = {
5757
"integerList": [1, 2],
5858
"stringList": ["abc", "cde"],
59+
"optionalIntegerList": [1, 2],
60+
"optionalStringList": ["abc", "cde", None],
5961
}
6062

6163
arguments = [
@@ -69,6 +71,16 @@ def test_list():
6971
python_name="string_list",
7072
type_annotation=StrawberryAnnotation(list[str]),
7173
),
74+
StrawberryArgument(
75+
graphql_name="optionalIntegerList",
76+
python_name="optional_integer_list",
77+
type_annotation=StrawberryAnnotation(list[Optional[int]]),
78+
),
79+
StrawberryArgument(
80+
graphql_name="optionalStringList",
81+
python_name="optional_string_list",
82+
type_annotation=StrawberryAnnotation(list[Optional[str]]),
83+
),
7284
]
7385

7486
assert convert_arguments(
@@ -79,6 +91,8 @@ def test_list():
7991
) == {
8092
"integer_list": [1, 2],
8193
"string_list": ["abc", "cde"],
94+
"optional_integer_list": [1, 2],
95+
"optional_string_list": ["abc", "cde", None],
8296
}
8397

8498

0 commit comments

Comments
 (0)