[dataquery] Optimize filtering of valid candidates for large datasets #9344
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
driusan
force-pushed
the
MoreEfficientCandidateFilter
branch
2 times, most recently
from
3450f73 to
e9e2bcf
Compare
regisoc
reviewed
Sep 20, 2024
| * | ||
| * @return \Database | ||
| */ | ||
| public function getNewDatabaseConnection() : \Database |
There was a problem hiding this comment.
Is this function useful in all places that use lorisinstance object?
Shouldn't it be placed only in the dataquery module query.class.inc because that is the only one using it?
There was a problem hiding this comment.
It's useful any place that calls setBuffering(false) and then iterates over a query result because the connection can't be re-used until the iteration is finished.
It's the only one now, but there will be others in other PRs.
There was a problem hiding this comment.
(ie. #9347 is another example that will use it.)
There was a problem hiding this comment.
Ok yes, thanks. I will make a quick test today ideally.
added 3 commits
September 23, 2024 08:59
This optimizes the filtering of inaccessible candidates, building on PR#9334 Previously, the entire query was loaded into memory, and then manipulated in a way that made it easier to see which candidates are valid, then returned as an array by putting the valid ones in a temporary table and selecting it. With this change, instead of an array, a generator is used so that only one candidate (and its sessions) need to be loaded into memory at a time. This is done on an unbuffered connection, which necessitates a different database connection since only one query can be run at a time on a connection and the caller may make other queries before the generator finished.
driusan
force-pushed
the
MoreEfficientCandidateFilter
branch
from
a3562a7 to
4d54282
Compare
This was referenced Sep 23, 2024
regisoc
approved these changes
Sep 24, 2024
|
@driusan tried several queries but I do not have something big enough to make a thorough testing. I think it is good noneless. |
|
I think the important thing is that you tested the queries. I've tested the performance on large datasets (albiet in combination with other optimizations..) |
ZhichGaming
pushed a commit
to ZhichGaming/Loris
that referenced
this pull request
Nov 25, 2024
…aces#9344) This optimizes the filtering of inaccessible candidates, building on PR#9334 Previously, the entire query was loaded into memory, and then manipulated in a way that made it easier to see which candidates are valid, then returned as an array by putting the valid ones in a temporary table and selecting from it. With this change, instead of an array, a generator is used so that only one candidate (and its sessions) need to be loaded into memory at a time. This is done on an unbuffered connection, which necessitates a different database connection since only one query can be run at a time on a connection and the caller may make other queries before the generator finished. This result should be significantly less memory required for filtering candidates.
driusan
added a commit
that referenced
this pull request
Feb 3, 2025
This updates DBRowProvisioner to remove the strval (and fixes #9335). After doing that, it became clear that the inner class could be replaced by a generator while still fulfilling the contract of returning a `\Traversable`. Doing this made it more obvious that since #9334, pselect could be used directly while maintaining the lazy evaluation instead of bypassing the Database object to use the PDO directly. However this required injecting a `LorisInstance` object to the constructor to get the database connection. With access to the `LorisInstance` object, the `DBRowProvisioner` can also get a new database connection and disable query buffering to lazily retrieve rows since PR #9344. This may theoretically improve performance and memory usage on large provisioners but has not been tested. (Performance did not decrease in raisinbread but it is not a large dataset where it would be noticeable.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This optimizes the filtering of inaccessible candidates, building on PR#9334
Previously, the entire query was loaded into memory, and then manipulated in a way that made it easier to see which candidates are valid, then returned as an array by putting the valid ones in a temporary table and selecting from it.
With this change, instead of an array, a generator is used so that only one candidate (and its sessions) need to be loaded into memory at a time. This is done on an unbuffered connection, which necessitates a different database connection since only one query can be run at a time on a connection and the caller may make other queries before the generator finished.
This result should be significantly less memory required for filtering candidates.