-
Notifications
You must be signed in to change notification settings - Fork 54
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Parallel parsing, fix logging and deprecation warnings (#1620)
#1537 introduced various pecularities in the logging regarding parallel parsing. In particular, for a single input stream, a deprecation warning is shown even when `--parse-parallel` or `-p` are explicity specified on the command line, which is the recommended behavior. This code is now refactored, with a more uniform and informative logging, and a deprecation warning is only shown when it should be.
- Loading branch information
1 parent
26a3d42
commit 39ca684
Showing
4 changed files
with
101 additions
and
54 deletions.
There are no files selected for viewing
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 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 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| // Copyright 2024, University of Freiburg, | ||
| // Chair of Algorithms and Data Structures. | ||
| // Author: Johannes Kalmbach(joka921) <[email protected]> | ||
| // | ||
| // Copyright 2024, University of Freiburg | ||
| // Chair of Algorithms and Data Structures | ||
| // Author: Johannes Kalmbach <[email protected]> | ||
|
|
||
| #pragma once | ||
|
|
||
| #include <optional> | ||
|
|
@@ -27,6 +27,10 @@ struct InputFileSpecification { | |
| // Turtle files with all prefixes at the beginning and no multiline literals. | ||
| bool parseInParallel_ = false; | ||
|
|
||
| // Remember if the value for parallel parsing was set explicitly (via the | ||
| // command line). | ||
| bool parseInParallelSetExplicitly_ = false; | ||
|
|
||
| bool operator==(const InputFileSpecification&) const = default; | ||
| }; | ||
| } // namespace qlever | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| // Copyright 2015, University of Freiburg, | ||
| // Copyright 2015 - 2024, University of Freiburg, | ||
| // Chair of Algorithms and Data Structures. | ||
| // Author: Björn Buchhold ([email protected]) | ||
| // Authors: Björn Buchhold <[email protected]> [2015 - 2017] | ||
| // Johannes Kalmbach <[email protected]> | ||
| // Hannah Bast <[email protected]> | ||
|
|
||
| #include <gmock/gmock.h> | ||
| #include <gtest/gtest.h> | ||
|
|
@@ -527,50 +529,82 @@ TEST(IndexTest, trivialGettersAndSetters) { | |
|
|
||
| TEST(IndexTest, updateInputFileSpecificationsAndLog) { | ||
| using enum qlever::Filetype; | ||
| std::vector<qlever::InputFileSpecification> files{ | ||
| {"singleFile.ttl", Turtle, std::nullopt, false}}; | ||
| std::vector<qlever::InputFileSpecification> singleFileSpec = { | ||
| {"singleFile.ttl", Turtle, std::nullopt}}; | ||
| std::vector<qlever::InputFileSpecification> twoFilesSpec = { | ||
| {"firstFile.ttl", Turtle, std::nullopt}, | ||
| {"secondFile.ttl", Turtle, std::nullopt}}; | ||
| using namespace ::testing; | ||
|
|
||
| // Parallel parsing not specified anywhere. For a single input stream, we then | ||
| // default to `true` for reasons of backwards compatibility, but this is | ||
| // deprecated. For multiple input streams, we default to `false` and this is | ||
| // normal behavior. | ||
| { | ||
| singleFileSpec.at(0).parseInParallelSetExplicitly_ = false; | ||
| testing::internal::CaptureStdout(); | ||
| IndexImpl::updateInputFileSpecificationsAndLog(files, false); | ||
| IndexImpl::updateInputFileSpecificationsAndLog(singleFileSpec, | ||
| std::nullopt); | ||
| EXPECT_THAT(testing::internal::GetCapturedStdout(), | ||
| AllOf(HasSubstr("singleFile.ttl"), HasSubstr("deprecated"))); | ||
| EXPECT_TRUE(singleFileSpec.at(0).parseInParallel_); | ||
| } | ||
| { | ||
| twoFilesSpec.at(0).parseInParallelSetExplicitly_ = false; | ||
| twoFilesSpec.at(1).parseInParallelSetExplicitly_ = false; | ||
| testing::internal::CaptureStdout(); | ||
| IndexImpl::updateInputFileSpecificationsAndLog(twoFilesSpec, std::nullopt); | ||
| EXPECT_THAT( | ||
| testing::internal::GetCapturedStdout(), | ||
| AllOf(HasSubstr("from singleFile.ttl"), Not(HasSubstr("parallel")))); | ||
| EXPECT_FALSE(files.at(0).parseInParallel_); | ||
| AllOf(HasSubstr("from 2 input streams"), Not(HasSubstr("deprecated")))); | ||
| EXPECT_FALSE(twoFilesSpec.at(0).parseInParallel_); | ||
| EXPECT_FALSE(twoFilesSpec.at(1).parseInParallel_); | ||
| } | ||
|
|
||
| { | ||
| // Parallel parsing specified on the command line and not in the | ||
| // `settings.json`. This is normal behavior (no deprecation warning). | ||
| for (auto parallelParsing : {true, false}) { | ||
| singleFileSpec.at(0).parseInParallel_ = parallelParsing; | ||
| singleFileSpec.at(0).parseInParallelSetExplicitly_ = true; | ||
| testing::internal::CaptureStdout(); | ||
| IndexImpl::updateInputFileSpecificationsAndLog(files, true); | ||
| EXPECT_THAT(testing::internal::GetCapturedStdout(), | ||
| AllOf(HasSubstr("from singleFile.ttl"), | ||
| HasSubstr("settings.json"), HasSubstr("deprecated"))); | ||
| EXPECT_TRUE(files.at(0).parseInParallel_); | ||
| IndexImpl::updateInputFileSpecificationsAndLog(singleFileSpec, | ||
| std::nullopt); | ||
| EXPECT_THAT( | ||
| testing::internal::GetCapturedStdout(), | ||
| AllOf(HasSubstr("singleFile.ttl"), Not(HasSubstr("deprecated")))); | ||
| EXPECT_EQ(singleFileSpec.at(0).parseInParallel_, parallelParsing); | ||
| } | ||
| { | ||
| twoFilesSpec.at(0).parseInParallel_ = true; | ||
| twoFilesSpec.at(1).parseInParallel_ = false; | ||
| twoFilesSpec.at(0).parseInParallelSetExplicitly_ = true; | ||
| twoFilesSpec.at(1).parseInParallelSetExplicitly_ = true; | ||
| testing::internal::CaptureStdout(); | ||
| IndexImpl::updateInputFileSpecificationsAndLog(files, std::nullopt); | ||
| EXPECT_THAT(testing::internal::GetCapturedStdout(), | ||
| AllOf(HasSubstr("from singleFile.ttl"), | ||
| HasSubstr("single input"), HasSubstr("deprecated"))); | ||
| EXPECT_TRUE(files.at(0).parseInParallel_); | ||
| IndexImpl::updateInputFileSpecificationsAndLog(twoFilesSpec, std::nullopt); | ||
| EXPECT_THAT( | ||
| testing::internal::GetCapturedStdout(), | ||
| AllOf(HasSubstr("from 2 input streams"), Not(HasSubstr("deprecated")))); | ||
| EXPECT_TRUE(twoFilesSpec.at(0).parseInParallel_); | ||
| EXPECT_FALSE(twoFilesSpec.at(1).parseInParallel_); | ||
| } | ||
|
|
||
| // Parallel parsing not specified on the command line, but explicitly set in | ||
| // the `settings.json` file. This is deprecated for a single input | ||
| // stream and forbidden for multiple input streams. | ||
| { | ||
| files.emplace_back("secondFile.ttl", Turtle, std::nullopt, false); | ||
| auto filesCopy = files; | ||
| singleFileSpec.at(0).parseInParallelSetExplicitly_ = false; | ||
| testing::internal::CaptureStdout(); | ||
| IndexImpl::updateInputFileSpecificationsAndLog(files, false); | ||
| IndexImpl::updateInputFileSpecificationsAndLog(singleFileSpec, true); | ||
| EXPECT_THAT(testing::internal::GetCapturedStdout(), | ||
| AllOf(HasSubstr("from 2 input streams"), | ||
| Not(HasSubstr("is deprecated")))); | ||
| EXPECT_EQ(files, filesCopy); | ||
| AllOf(HasSubstr("singleFile.ttl"), HasSubstr("deprecated"))); | ||
| EXPECT_TRUE(singleFileSpec.at(0).parseInParallel_); | ||
| } | ||
|
|
||
| { | ||
| twoFilesSpec.at(0).parseInParallelSetExplicitly_ = false; | ||
| twoFilesSpec.at(1).parseInParallelSetExplicitly_ = false; | ||
| AD_EXPECT_THROW_WITH_MESSAGE( | ||
| IndexImpl::updateInputFileSpecificationsAndLog(files, true), | ||
| HasSubstr("but has to be specified")); | ||
| IndexImpl::updateInputFileSpecificationsAndLog(twoFilesSpec, true), | ||
| AllOf(Not(HasSubstr("from 2 input streams")), HasSubstr("forbidden"))); | ||
| } | ||
| } | ||
|
|
||
|
|
||