Implement materialized index scans by materializing lazy scans #1323
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
… scan. This makes the code much simpler, and also shouldn't be to expensive, as the expensive parts (reading from disk and decompressing) are still done in parallel.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1323 +/- ##
==========================================
- Coverage 88.89% 88.89% -0.01%
==========================================
Files 326 326
Lines 28992 28931 -61
Branches 3214 3207 -7
==========================================
- Hits 25773 25718 -55
+ Misses 2066 2061 -5
+ Partials 1153 1152 -1 ☔ View full report in Codecov by Sentry. |
|
RobinTF
reviewed
Apr 25, 2024
| // block in parallel | ||
| #pragma omp task | ||
| { | ||
| if (!cancellationHandle->isCancelled()) { |
There was a problem hiding this comment.
This is the only usage of isCancelled() outside of tests I think. I'd suggest removing the function or replacing it with something like ASSERT_THROW(throwIfCancelled()) where it makes sense to keep assertions around.
|
hannahbast
approved these changes
Jun 13, 2024
realHannes
pushed a commit
to realHannes/qlever
that referenced
this pull request
Jun 15, 2024
…eiburg#1323) This makes the code much simpler, and makes no difference for almost all queries. The expensive part (reading from disk and decompressing) is still done in parallel, only the writing to the `IdTable` is now serialized + there is an additional copy compared to before. An example query that is slower now because of this change is: materialize a large index scan (for example, for the predicate `rdf:type`) and group by subject (there is a shortcut for grouping by object when there are few objects). But such queries will become lazy soon anyway (see ad-freiburg#1350) and then this will be irrelevant.
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 makes the code much simpler, and makes no difference for almost all queries. The expensive part (reading from disk and decompressing) is still done in parallel, only the writing to the
IdTableis now serialized + there is an additional copy compared to before. An example query that is slower now because of this change is: materialize a large index scan (for example, for the predicaterdf:type) and group by subject (there is a shortcut for grouping by object when there are few objects). But such queries will become lazy soon anyway (see #1350) and then this will be irrelevant.