Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make (Ordered)ThreadSafeQueue safer und easier to use #1023

Merged

Conversation

joka921
Copy link
Member

@joka921 joka921 commented Jul 7, 2023

This allows for easier lifetime management of the queue.

@codecov
Copy link

codecov bot commented Jul 7, 2023

Codecov Report

Patch coverage: 88.23% and project coverage change: +0.64 🎉

Comparison is base (7e83a61) 75.17% compared to head (96495ed) 75.81%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1023      +/-   ##
==========================================
+ Coverage   75.17%   75.81%   +0.64%     
==========================================
  Files         255      262       +7     
  Lines       23829    24470     +641     
  Branches     2943     3007      +64     
==========================================
+ Hits        17913    18552     +639     
+ Misses       4735     4730       -5     
- Partials     1181     1188       +7     
Impacted Files Coverage Δ
src/util/ThreadSafeQueue.h 93.51% <88.23%> (-1.15%) ⬇️

... and 21 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@hannahbast hannahbast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor typo + Johannes has an idea for further improvement

Copy link
Member

@hannahbast hannahbast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some renaming + maybe test case where both producer and consumer throw an exception

Copy link
Member

@hannahbast hannahbast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor renames left

Copy link
Member

@hannahbast hannahbast left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great now + very nice!

@hannahbast
Copy link
Member

Make ThreadSafeQueue and OrderedThreadSafeQueue easier and safer to use. In particular, the finish method is now noexcept. To use the queue, use the function queueManager, which takes a queue size, the number of threads, and a producer function, and returns a value generator.

@hannahbast hannahbast changed the title Make the finish method of ThreadSafeQueue noexcept Make (Ordered)ThreadSafeQueue safer und easier to use Jul 11, 2023
@hannahbast hannahbast merged commit b57869e into ad-freiburg:master Jul 11, 2023
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
0.0% 0.0% Duplication

hannahbast pushed a commit that referenced this pull request Jul 12, 2023
Under certain conditions, index scans are now lazily materialized (and there is a special status for this in the runtime information, see ad-freiburg/qlever-ui#54).

The conditions are: (1) the index scan is the operand of a single-column join operation, (2) the other operand of the join operation contains no UNDEF values in the join column and the query planner knows this, and (3) the number of rows of the index scan, if it were fully materialized, is above the runtime parameter `lazy-index-scan-max-size-materialization`.

Lazily materialized then means two things: (1) a subset of the blocks is identified (using the metadata of the blocks) that is sufficient to compute the correct result, and (2) these blocks are produced one after the other and buffered only to a limited extent. There are two reasons for the buffering: (1) it is sometimes necessary to have several blocks in memory at the same time to compute cross-products efficiently, and (2) there is an `OrderedThreadSafeQueue` for the blocks (see #1011 and #1023) with runtime parameters `lazy-index-scan-queue-size` and `lazy-index-scan-num-threads` in order to allow parallel decompression of the blocks and minimize waiting times for the join operation consuming the blocks.

For all cases, where the conditions for lazy materialization are not met, the operands of the join are fully materialized, and one of the three existing join implementations is used: the standard zipper join, the galloping join (when the size of one operand is much larger than the other), or the general-purpose join (which can handle multiple columns and UNDEF values).
@joka921 joka921 deleted the noexcept_finish_threadsafe_queue branch July 12, 2023 15:06
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