Making the allocation part of read_tiles single threaded. #2753
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
|
This pull request has been linked to Shortcut Story #12965: read_tiles/unfiler_tiles: start unfiltering as soon as tiles are ready.. |
Shelnutt2
reviewed
Dec 20, 2021
KiterLuc
force-pushed
the
lr/read-unfilter-parallel/ch12965
branch
from
47bc9f7
to
92533bc
Compare
There was a problem hiding this comment.
The modifications to the Tile class have the same problem that the Stats class already has: mixing different lifespan concerns within the same class.
mutex/cv use: *****************------------------------
tile data use: ---------------**************************
One of the effects is simply memory consumption. After tiles are loaded, the mutex/cv remains in memory, not doing anything.
What's really here is a factory class with synchronized construction of Tile objects. It likely ought to appear as a separate class. Instances of a separate tile factory could also be reused through a resource pool, yield a modest fixed memory overhead and avoid most of the construction costs of factories.
KiterLuc
force-pushed
the
lr/read-unfilter-parallel/ch12965
branch
4 times, most recently
from
1bf33f0
to
c972f96
Compare
KiterLuc
changed the title
Shelnutt2
approved these changes
Dec 20, 2021
KiterLuc
force-pushed
the
lr/read-unfilter-parallel/ch12965
branch
4 times, most recently
from
0ff1051
to
0b77fba
Compare
This change allows unfiltering to start as soon as tiles are loaded by VFS. By including a condition variable for each tiles, VFS can signal once it loads an initial batch of tiles. This also adds the vfs.max_batch_read_size setting to specify the maximum size for a batch. --- TYPE: IMPROVEMENT DESC: Read/unfilter tiles: start unfiltering as soon as tiles are ready.
KiterLuc
force-pushed
the
lr/read-unfilter-parallel/ch12965
branch
7 times, most recently
from
ae9d7de
to
40224a6
Compare
KiterLuc
force-pushed
the
lr/read-unfilter-parallel/ch12965
branch
from
40224a6
to
5433574
Compare
github-actions bot
pushed a commit
that referenced
this pull request
Dec 23, 2021
* Read/unfilter tiles: start unfiltering as soon as tiles are ready. This change allows unfiltering to start as soon as tiles are loaded by VFS. By including a condition variable for each tiles, VFS can signal once it loads an initial batch of tiles. This also adds the vfs.max_batch_read_size setting to specify the maximum size for a batch. --- TYPE: IMPROVEMENT DESC: Read/unfilter tiles: start unfiltering as soon as tiles are ready. * Scoping back the change to only a few fixes and making tile generation single threaded.
Shelnutt2
pushed a commit
that referenced
this pull request
Dec 27, 2021
) * Read/unfilter tiles: start unfiltering as soon as tiles are ready. This change allows unfiltering to start as soon as tiles are loaded by VFS. By including a condition variable for each tiles, VFS can signal once it loads an initial batch of tiles. This also adds the vfs.max_batch_read_size setting to specify the maximum size for a batch. --- TYPE: IMPROVEMENT DESC: Read/unfilter tiles: start unfiltering as soon as tiles are ready. * Scoping back the change to only a few fixes and making tile generation single threaded. Co-authored-by: KiterLuc <67824247+KiterLuc@users.noreply.github.com>
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 change reverts the first part of read_tiles to be single threaded. As it is mostly doing memory allocations, it's better to keep it single threaded.
Also including a bug fix when checking the tile allocation size and another to prevent reloading query condition tiles in the sparse unordered with duplicates reader.
TYPE: IMPROVEMENT
DESC: Making the allocation part of read_tiles single threaded.