Skip to content

Commit faf9d09

Browse files
authored
Addons: improve serializer performance (#12335)
- Instead of passing the version slug to the project serializer, just pass the actual version, so we can use resolve_version, this reduces one less query. - We were passing the resolver around some places, but doing this weird thing where removing it from kwargs, and then adding it again. This introduced a bug that was creating a new resolver for each serializer instance instead of re-using the one passed to the initializer. This took ~10ms off locally <img width="1550" height="237" alt="Screenshot 2025-07-17 at 16-04-42 Silky - Profiling -" src="https://github.com/user-attachments/assets/5667e02c-fe74-4583-b2db-43a086c994b5" /> Note: I'm running these benchmarks in power saver mode, otherwise my computer is too fast and can't get meaningful comparisons :D
1 parent 2691126 commit faf9d09

File tree

3 files changed

+20
-39
lines changed

3 files changed

+20
-39
lines changed

readthedocs/api/v3/serializers.py

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -355,16 +355,11 @@ class Meta:
355355
"privacy_level",
356356
]
357357

358-
def __init__(self, *args, resolver=None, version_serializer=None, **kwargs):
359-
super().__init__(*args, **kwargs)
360-
358+
def __init__(self, *args, resolver=None, **kwargs):
361359
# Use a shared resolver to reduce the amount of DB queries while
362360
# resolving version URLs.
363-
self.resolver = kwargs.pop("resolver", Resolver())
364-
365-
# Allow passing a specific serializer when initializing it.
366-
# This is required to pass ``VersionSerializerNoLinks`` from the addons API.
367-
self.version_serializer = version_serializer or VersionSerializer
361+
self.resolver = resolver or Resolver()
362+
super().__init__(*args, **kwargs)
368363

369364
def get_downloads(self, obj):
370365
downloads = obj.get_downloads()
@@ -387,7 +382,8 @@ def get_aliases(self, obj):
387382
if obj.slug == LATEST:
388383
alias_version = obj.project.get_original_latest_version()
389384
if alias_version and alias_version.active:
390-
return [self.version_serializer(alias_version).data]
385+
# NOTE: we use __class__, as this serializer can be subclassed.
386+
return [self.__class__(alias_version).data]
391387
return []
392388

393389

@@ -472,9 +468,9 @@ def get_downloads(self, obj):
472468
return None
473469

474470
def get_documentation(self, obj):
475-
version_slug = getattr(self.parent, "version_slug", None)
471+
version = getattr(self.parent, "version", None)
476472
resolver = getattr(self.parent, "resolver", Resolver())
477-
return obj.get_docs_url(version_slug=version_slug, resolver=resolver)
473+
return resolver.resolve_version(project=obj, version=version)
478474

479475

480476
class RepositorySerializer(serializers.Serializer):
@@ -837,13 +833,13 @@ class Meta:
837833
),
838834
}
839835

840-
def __init__(self, *args, **kwargs):
841-
# Receive a `Version.slug` here to build URLs properly
842-
self.version_slug = kwargs.pop("version_slug", None)
836+
def __init__(self, *args, resolver=None, **kwargs):
837+
# Receive a `Version` here to build URLs properly
838+
self.version = kwargs.pop("version", None)
843839

844840
# Use a shared resolver to reduce the amount of DB queries while
845841
# resolving version URLs.
846-
self.resolver = kwargs.pop("resolver", Resolver())
842+
self.resolver = resolver or Resolver()
847843

848844
super().__init__(*args, **kwargs)
849845
# When using organizations, projects don't have the concept of users.

readthedocs/proxito/tests/test_hosting.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -785,7 +785,7 @@ def test_number_of_queries_project_version_slug(self):
785785
active=True,
786786
)
787787

788-
with self.assertNumQueries(20):
788+
with self.assertNumQueries(17):
789789
r = self.client.get(
790790
reverse("proxito_readthedocs_docs_addons"),
791791
{
@@ -814,7 +814,7 @@ def test_number_of_queries_url(self):
814814
active=True,
815815
)
816816

817-
with self.assertNumQueries(22):
817+
with self.assertNumQueries(18):
818818
r = self.client.get(
819819
reverse("proxito_readthedocs_docs_addons"),
820820
{
@@ -850,7 +850,7 @@ def test_number_of_queries_url_subproject(self):
850850
active=True,
851851
)
852852

853-
with self.assertNumQueries(26):
853+
with self.assertNumQueries(22):
854854
r = self.client.get(
855855
reverse("proxito_readthedocs_docs_addons"),
856856
{
@@ -866,7 +866,7 @@ def test_number_of_queries_url_subproject(self):
866866
assert r.status_code == 200
867867

868868
# Test parent project has fewer queries
869-
with self.assertNumQueries(21):
869+
with self.assertNumQueries(17):
870870
r = self.client.get(
871871
reverse("proxito_readthedocs_docs_addons"),
872872
{
@@ -892,7 +892,7 @@ def test_number_of_queries_url_translations(self):
892892
language=language,
893893
)
894894

895-
with self.assertNumQueries(39):
895+
with self.assertNumQueries(35):
896896
r = self.client.get(
897897
reverse("proxito_readthedocs_docs_addons"),
898898
{
@@ -1042,7 +1042,7 @@ def test_file_tree_diff(self, get_manifest):
10421042
],
10431043
),
10441044
]
1045-
with self.assertNumQueries(27):
1045+
with self.assertNumQueries(25):
10461046
r = self.client.get(
10471047
reverse("proxito_readthedocs_docs_addons"),
10481048
{

readthedocs/proxito/views/hosting.py

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -252,24 +252,9 @@ class RelatedProjectSerializerNoLinks(NoLinksMixin, RelatedProjectSerializer):
252252
class ProjectSerializerNoLinks(NoLinksMixin, ProjectSerializer):
253253
related_project_serializer = RelatedProjectSerializerNoLinks
254254

255-
def __init__(self, *args, **kwargs):
256-
resolver = kwargs.pop("resolver", Resolver())
257-
super().__init__(
258-
*args,
259-
resolver=resolver,
260-
**kwargs,
261-
)
262-
263255

264256
class VersionSerializerNoLinks(NoLinksMixin, VersionSerializer):
265-
def __init__(self, *args, **kwargs):
266-
resolver = kwargs.pop("resolver", Resolver())
267-
super().__init__(
268-
*args,
269-
resolver=resolver,
270-
version_serializer=VersionSerializerNoLinks,
271-
**kwargs,
272-
)
257+
pass
273258

274259

275260
class BuildSerializerNoLinks(NoLinksMixin, BuildSerializer):
@@ -616,7 +601,7 @@ def _get_projects_response(self, *, request, project, version, resolver):
616601
translations = ProjectSerializerNoLinks(
617602
translations_qs,
618603
resolver=resolver,
619-
version_slug=version.slug if version else None,
604+
version=version,
620605
many=True,
621606
).data
622607
else:
@@ -626,7 +611,7 @@ def _get_projects_response(self, *, request, project, version, resolver):
626611
"current": ProjectSerializerNoLinks(
627612
project,
628613
resolver=resolver,
629-
version_slug=version.slug if version else None,
614+
version=version,
630615
).data,
631616
"translations": translations,
632617
}

0 commit comments

Comments
 (0)