Skip to content

Commit 99e76c1

Browse files
fix memory issues by moving router instantiation out of view __init__ (#77)
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
1 parent af7fd99 commit 99e76c1

File tree

6 files changed

+305
-5
lines changed

6 files changed

+305
-5
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ and this project attempts to adhere to [Semantic Versioning](https://semver.org/
1818

1919
## [Unreleased]
2020

21+
### Fixed
22+
23+
- Fixed excessive memory usage in `AsyncWebhookView` and `SyncWebhookView` caused by creating a new `GitHubRouter` instance on each request.
24+
2125
## [0.6.0]
2226

2327
### Added

pyproject.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,10 @@ dev = [
1616
"pytest-cov>=6.0.0",
1717
"pytest-django>=4.9.0",
1818
"pytest-httpx>=0.33.0",
19+
"pytest-memray>=1.7.0",
1920
"pytest-randomly>=3.16.0",
2021
"pytest-xdist>=3.6.1",
21-
"ruff>=0.7.3"
22+
"ruff>=0.7.3",
2223
]
2324
types = [
2425
"django-stubs>=5.1.1",

src/django_github_app/views.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727

2828
GitHubAPIType = TypeVar("GitHubAPIType", AsyncGitHubAPI, SyncGitHubAPI)
2929

30+
_router = GitHubRouter(*GitHubRouter.routers)
31+
3032

3133
class BaseWebhookView(View, ABC, Generic[GitHubAPIType]):
3234
github_api_class: type[GitHubAPIType]
@@ -59,7 +61,7 @@ def get_response(self, event_log: EventLog) -> JsonResponse:
5961

6062
@property
6163
def router(self) -> GitHubRouter:
62-
return GitHubRouter(*GitHubRouter.routers)
64+
return _router
6365

6466
@abstractmethod
6567
def post(

tests/test_routing.py

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
from __future__ import annotations
2+
3+
import pytest
4+
from django.http import HttpRequest
5+
from django.http import JsonResponse
6+
7+
from django_github_app.github import SyncGitHubAPI
8+
from django_github_app.routing import GitHubRouter
9+
from django_github_app.views import BaseWebhookView
10+
11+
12+
@pytest.fixture(autouse=True)
13+
def test_router():
14+
import django_github_app.views
15+
from django_github_app.routing import GitHubRouter
16+
17+
old_routers = GitHubRouter._routers.copy()
18+
GitHubRouter._routers = []
19+
20+
old_router = django_github_app.views._router
21+
22+
test_router = GitHubRouter()
23+
django_github_app.views._router = test_router
24+
25+
yield test_router
26+
27+
GitHubRouter._routers = old_routers
28+
django_github_app.views._router = old_router
29+
30+
31+
class View(BaseWebhookView[SyncGitHubAPI]):
32+
github_api_class = SyncGitHubAPI
33+
34+
def post(self, request: HttpRequest) -> JsonResponse:
35+
return JsonResponse({})
36+
37+
38+
class LegacyView(BaseWebhookView[SyncGitHubAPI]):
39+
github_api_class = SyncGitHubAPI
40+
41+
@property
42+
def router(self) -> GitHubRouter:
43+
# Always create a new router (simulating issue #73)
44+
return GitHubRouter(*GitHubRouter.routers)
45+
46+
def post(self, request: HttpRequest) -> JsonResponse:
47+
return JsonResponse({})
48+
49+
50+
class TestGitHubRouter:
51+
def test_router_single_instance(self):
52+
view1 = View()
53+
view2 = View()
54+
55+
router1 = view1.router
56+
router2 = view2.router
57+
58+
assert router1 is router2
59+
assert view1.router is router1
60+
assert view2.router is router2
61+
62+
def test_no_duplicate_routers(self):
63+
router_ids = set()
64+
65+
for _ in range(1000):
66+
view = View()
67+
router_ids.add(id(view.router))
68+
69+
assert len(router_ids) == 1
70+
71+
def test_duplicate_routers_without_module_level_router(self):
72+
router_ids = set()
73+
74+
for _ in range(5):
75+
view = LegacyView()
76+
router_ids.add(id(view.router))
77+
78+
assert len(router_ids) == 5
79+
80+
@pytest.mark.limit_memory("2.5MB")
81+
def test_router_memory_stress_test(self):
82+
view_count = 50000
83+
views = []
84+
85+
for _ in range(view_count):
86+
view = View()
87+
views.append(view)
88+
89+
assert len(views) == view_count
90+
assert all(view.router is views[0].router for view in views)
91+
92+
@pytest.mark.limit_memory("4MB")
93+
def test_router_memory_stress_test_legacy(self):
94+
view_count = 50000
95+
views = []
96+
97+
for _ in range(view_count):
98+
view = LegacyView()
99+
views.append(view)
100+
101+
assert len(views) == view_count
102+
assert not all(view.router is views[0].router for view in views)

tests/test_views.py

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@
1717
from django_github_app.github import AsyncGitHubAPI
1818
from django_github_app.github import SyncGitHubAPI
1919
from django_github_app.models import EventLog
20-
from django_github_app.routing import GitHubRouter
2120
from django_github_app.views import AsyncWebhookView
2221
from django_github_app.views import BaseWebhookView
2322
from django_github_app.views import SyncWebhookView
@@ -71,9 +70,21 @@ def _make_request(
7170

7271
@pytest.fixture
7372
def test_router():
73+
import django_github_app.views
74+
from django_github_app.routing import GitHubRouter
75+
76+
old_routers = GitHubRouter._routers.copy()
7477
GitHubRouter._routers = []
75-
yield GitHubRouter()
76-
GitHubRouter._routers = []
78+
79+
old_router = django_github_app.views._router
80+
81+
test_router = GitHubRouter()
82+
django_github_app.views._router = test_router
83+
84+
yield test_router
85+
86+
GitHubRouter._routers = old_routers
87+
django_github_app.views._router = old_router
7788

7889

7990
@pytest.fixture

0 commit comments

Comments
 (0)