Integrate configManager in src/index/IndexImpl.cpp
#1041
+382
−208
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
…the fileToJson helper function.
… function public. Otherwise I can't check the set state of ConfigOptions.
…uilderSettingsFromFile.
…t value existence with multiple function overloads.
…ions now return a pointer to the created ConfigOption.
…ce instead of a pointer.
…ementation by usage of std::views.
# Conflicts: # src/index/IndexImpl.cpp
…' into IntegrateConfigManagerInIndexImpl # Conflicts: # src/index/IndexImpl.cpp
…ns for printConfigurationDoc, so that user have more control over the representation.
…tion of the config manager into its own file.
…e configuration options for the index builder.
joka921
requested changes
Dec 11, 2023
src/index/IndexImpl.h
Outdated
| @@ -631,6 +632,42 @@ class IndexImpl { | |||
| void writeConfiguration() const; | |||
| void readConfiguration(); | |||
|
|
|||
| // Assigns the entries of the enum `TurtleParserIntegerOverflowBehavior` to | |||
| // their string representation. | |||
| inline static const ad_utility::HashMap<std::string, | |||
There was a problem hiding this comment.
Either inline or static :) (with const the difference is not really important).
There was a problem hiding this comment.
Ah, but only with inline can I use a default value and without static every instance of the class has its own copy, even though they are never different.
| /* | ||
| @brief Print the detailed documentation of the options for the index builder. | ||
| */ | ||
| std::string getConfigurationDocForIndexBuilder(); |
There was a problem hiding this comment.
I think this should return also two strings: the JSON and the detailed configuration separately, then we can configure the printing in IndexBuilderMain (I will think of something).
There was a problem hiding this comment.
Shouldn't the configuration of the printing happen inside IndexImpl::getConfigurationDocForIndexBuilder? Separation of concerns and all that.
1. For a while now, we used `apt-spy` because `apt` was not working reliably within GitHub's workflows. Recently, it was `apt-spy` that was unreliable and so we are reverting back to `apt` 2. Use the occasion to factor out separate `action.yml` "sub"-workflow for installing a compiler on Ubuntu and for installing dependencies on Ubuntu, which are used at various places in our workflows
…ions (ad-freiburg#1174) Add some helper functions for adding statistics columns (that is columns with results that were not measured but computed from other columns) to the result table of a benchmark run.
…ata. (ad-freiburg#1181) In the hash map-based GROUP BY optimization, the hash map now only stores a mapping from ID->size_t where the size_t is an index into a vector that stores the actual data. This optimization will make the support for multiple different aggregates much more efficient.
So far, the Docker images were pushed to Docker Hub as `<private username>/qlever`, to make it easier to check whether everything works as it should. Everything seems to work as it should and so now we push as the official `adfreiburg/qlever`.
…d-freiburg#1187) So far, the parallel Turtle parser had the problem that certain exceptions disappeared. After ad-freiburg#1124, two exceptions happened more often, namely that the `FILE_BUFFER_SIZE` was not large enough (it was reduced from 100M to 10M), and an overflow of the sort key buffer (its preallocation was increased significantly to save time). So far, both exceptions had the effect that the parsing hung forever, either because batches were still being produced but not processed, or batches were still being processed but not produced. All exceptions are now caught and appear in the log. The potential overflow of the sort key buffer is now fixed. The `FILE_BUFFER_SIZE` can still be too small (and it happens for `UniProt` and `OSM Planet`), but now the exception shows in the log and ends the index build.
In the future we should manage more (or even all) of our git dependencies via FetchContent, as it is much simpler than working with git submodules.
…icated by the generated documentation of ConfigManager.
…ue of turtleParserIntegerOverflowBehaviorMap_.
|
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.
I replaced as much of the old parsing of configuration option with the new
ConfigManageras possible, in order to make this whole thing easier and easier to read.