Repository navigation
perf: Skip the reload of the loaded but empty relations without force - #70
Conversation
- Check if the relation is loaded with has_value instead of the truthiness of its value - Stop querying the empty lists and unset to-one relations on every call without force - Cover the loading of the relations with and without force in the tests
|
Caution Review failedThe pull request is closed.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7b58189f26
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if not force and self.has_value(name): | ||
| return value |
There was a problem hiding this comment.
After EntityClass.reset() (also called by detach(reset=True) and detach_l(reset=True)), relations are stored in the instance as colony.Lazy, not removed. In data/src/entity_manager/structures.py, has_value() only checks membership in __dict__, and get_value() returns that sentinel unchanged. Consequently, relation(name, force=False) now treats these unloaded relations as loaded and returns colony.Lazy without asking the entity manager to fetch them. Exclude lazy sentinel values from this early return while retaining the optimization for genuine empty lists and None.
Useful? React with 👍 / 👎.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Closes #69
Summary
relation(name, force=False)checks if the relation is loaded withhas_value(the name set in the entity) instead of the truthiness of its value, so a relation that is loaded but empty (an empty list of a to-many relation or an unset to-one relation,None) is not loaded again on every call, its value being read only when it's returnedforce=True(the default) always loadsMockRelationEntityandMockEntityManager.relationin the mocks)Performance
Measured in Omni (the benchmark of hivesolutions/omni#336, MariaDB 11.8, the version of omni-ldj, with the demo data plus 2000 users), where
SystemUser.get_from_sessionruns several times on every request:Regression checks
mvc_utilsof this branch (2074 tests, SQLite), andtest_get_from_session_emptyof hivesolutions/omni#343 passes with it while failing with themvc_utilsof masterget_descriptionof the new test case, as for every other test case