Skip to content

Commit

Permalink
Many more improvements for the tests and for the tools.
Browse files Browse the repository at this point in the history
Signed-off-by: Johannes Kalmbach <[email protected]>
  • Loading branch information
joka921 committed Feb 6, 2025
1 parent 5f2ec6c commit d8080b3
Show file tree
Hide file tree
Showing 4 changed files with 32 additions and 6 deletions.
17 changes: 16 additions & 1 deletion src/global/Pattern.h
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ struct CompactStringVectorWriter {
off_t _startOfFile;
using offset_type = typename CompactVectorOfStrings<data_type>::offset_type;
std::vector<offset_type> _offsets;

// A `CompactStringVectorWriter` that has been moved from may not call
// `finish()` any more in its destructor.
ad_utility::ResetWhenMoved<bool, true> _finished = false;
Expand Down Expand Up @@ -230,19 +231,33 @@ struct CompactStringVectorWriter {
}
}

// The copy operations would be deleted implicitly (because `File` is not
// copyable.
CompactStringVectorWriter(const CompactStringVectorWriter&) = delete;
CompactStringVectorWriter& operator=(const CompactStringVectorWriter&) =
delete;

// The move operations have to be explicitly defaulted, because we have a
// manually defined destructor.
// Note: The defaulted move operations behave correctly because of the usage
// of `ResetWhenMoved` with the `_finished` member.
CompactStringVectorWriter(CompactStringVectorWriter&&) = default;
CompactStringVectorWriter& operator=(CompactStringVectorWriter&&) = default;

private:
// Has to be run by all the constructors
void commonInitialization() {
AD_CONTRACT_CHECK(_file.isOpen());
// We don't known the data size yet.
// We don't know the data size yet.
_startOfFile = _file.tell();
size_t dataSizeDummy = 0;
_file.write(&dataSizeDummy, sizeof(dataSizeDummy));
}
};
static_assert(
std::is_nothrow_move_assignable_v<CompactStringVectorWriter<char>>);
static_assert(
std::is_nothrow_move_constructible_v<CompactStringVectorWriter<char>>);
} // namespace detail

// Forward iterator for a `CompactVectorOfStrings` that reads directly from
Expand Down
6 changes: 3 additions & 3 deletions src/index/vocabulary/VocabularyOnDisk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ OffsetAndSize VocabularyOnDisk::getOffsetAndSize(uint64_t i) const {
std::string VocabularyOnDisk::operator[](uint64_t idx) const {
AD_CONTRACT_CHECK(idx < size());
auto offsetAndSize = getOffsetAndSize(idx);
string result(offsetAndSize._size, '\0');
file_.read(result.data(), offsetAndSize._size, offsetAndSize._offset);
string result(offsetAndSize.size_, '\0');
file_.read(result.data(), offsetAndSize.size_, offsetAndSize.offset_);
return result;
}

Expand Down Expand Up @@ -88,7 +88,7 @@ VocabularyOnDisk::WordWriter::~WordWriter() {
void VocabularyOnDisk::buildFromStringsAndIds(
const std::vector<std::pair<std::string, uint64_t>>& wordsAndIds,
const std::string& fileName) {
return buildFromIterable(wordsAndIds, fileName);
buildFromIterable(wordsAndIds, fileName);
}

// _____________________________________________________________________________
Expand Down
4 changes: 2 additions & 2 deletions src/index/vocabulary/VocabularyOnDisk.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,8 @@ class VocabularyOnDisk : public VocabularyBinarySearchMixin<VocabularyOnDisk> {

// The offset of a word in `file_` and its size in number of bytes.
struct OffsetAndSize {
uint64_t _offset;
uint64_t _size;
uint64_t offset_;
uint64_t size_;
};

// Helper function for implementing a random access iterator.
Expand Down
11 changes: 11 additions & 0 deletions test/index/vocabulary/PolymorphicVocabularyTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
using ad_utility::VocabularyType;

namespace {

// Test a `PolymorphicVocabulary` with a given `vocabType`.
void testForVocabType(VocabularyType::Enum vocabType) {
VocabularyType type{vocabType};
std::string filename =
Expand Down Expand Up @@ -37,6 +39,15 @@ void testForVocabType(VocabularyType::Enum vocabType) {
}
} // namespace

// Test the general functionality of the `PolymorphicVocabulary` for all the
// possible `VocabularyType`s.
TEST(PolymorphicVocabulary, basicTests) {
ql::ranges::for_each(VocabularyType::all(), &testForVocabType);
}

// Test a corner case in a `switch` statement.
TEST(PolymorphicVocabulary, invalidVocabularyType) {
PolymorphicVocabulary vocab;
auto invalidType = VocabularyType{static_cast<VocabularyType::Enum>(23401)};
EXPECT_ANY_THROW(vocab.resetToType(invalidType));
}

0 comments on commit d8080b3

Please sign in to comment.