Skip to content

perf: share pk-only related models across rows of a query - #1853

Open
collerek wants to merge 1 commit into
masterfrom
perf/share-pk-only-related
Open

collerek wants to merge 1 commit into
masterfrom
perf/share-pk-only-related

Conversation

@collerek

Copy link
Copy Markdown
Collaborator

Foreign keys that are not selected built a separate pk-only related model for every row (1000 Author placeholders for 1000 books by 50 authors), about half of the time of a plain all(). Rows pointing to the same pk from the same relation now reuse one instance through the per-query row registry from #1851; models below a many-to-many relation stay scoped to their parent.

Foreign keys that are not selected used to build a separate pk-only related
model for every row, e.g. 1000 Author placeholders for 1000 books by 50
authors. Rows pointing to the same pk from the same position now reuse one
instance through the per-query row registry, the same way selected related
models are shared since #1851. Models below a many-to-many relation stay
scoped to their parent.
@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed

codspeed Bot commented Oct 16, 2026

Copy link
Copy Markdown

Merging this PR will regress 2 benchmarks

⚠️ 112 benchmarks spent significant time in system calls

System calls cannot be consistently instrumented, so they are not included in the measure, which understates the real cost. Please switch to the Walltime instrument to accurately measure system calls.

Measurement and system calls

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 6 improved benchmarks
❌ 2 regressed benchmarks
✅ 118 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
❌ ⚠️ test_min[500] 5.3 ms 7.2 ms -26.77%
❌ ⚠️ test_avg[250] 6.4 ms 7.3 ms -12.66%
⚡ ⚠️ test_get_all_with_related_models[40] 30.6 ms 22 ms +38.84%
⚡ ⚠️ test_get_all_with_related_models[20] 18.4 ms 14.4 ms +28.06%
⚡ ⚠️ test_duplicate_relation_paths[200] 259.9 ms 217.4 ms +19.54%
⚡ ⚠️ test_duplicate_relation_paths[100] 135.3 ms 114.1 ms +18.61%
⚡ ⚠️ test_duplicate_relation_paths[50] 78.2 ms 67.7 ms +15.58%
⚡ ⚠️ test_get_all_with_related_models[10] 16.9 ms 14.8 ms +13.68%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing perf/share-pk-only-related (274c351) with master (67d8106)

Open in CodSpeed

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants