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
F.serializer #422
F.serializer #422
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code review in 1-1 with Johannes, thanks for the great mini library!
Johannes said: add some tests also for serializing hash maps and strings.
src/util/Serializer/Serializer.h
Outdated
Storage&& data() && { return std::move(_data); } | ||
|
||
private: | ||
std::vector<char> _data; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Storage _data;
src/util/Serializer/FileSerializer.h
Outdated
|
||
SerializationPosition getCurrentPosition() const { return _file.tell(); } | ||
|
||
File&& moveFileOut() && { return std::move(_file); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
moveFileOut -> file
src/util/Serializer/FileSerializer.h
Outdated
_file.seek(position, SEEK_SET); | ||
} | ||
|
||
File&& moveFileOut() && { return std::move(_file); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dito
@@ -131,7 +131,7 @@ void Index::passContextFileIntoVector(const string& contextFile, | |||
// this has to be repeated completely here because we have the possibility to | |||
// only add a text index. In that case the Vocabulary has never been | |||
// initialized before | |||
_vocab = Vocabulary<CompressedString, TripleComponentComparator>(); | |||
_vocab = std::move(Vocabulary<CompressedString, TripleComponentComparator>()); | |||
readConfiguration(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? Was there a problem before?
test/IndexTest.cpp
Outdated
ASSERT_EQ(Id(0), *reinterpret_cast<Id*>(buf + bytesDone)); | ||
EXPECT_EQ(Id(0), *reinterpret_cast<Id*>(buf + bytesDone)); | ||
bytesDone += sizeof(Id); | ||
ASSERT_EQ(4u, *reinterpret_cast<Id*>(buf + bytesDone)); | ||
EXPECT_EQ(4u, *reinterpret_cast<Id*>(buf + bytesDone)); | ||
bytesDone += sizeof(Id); | ||
ASSERT_EQ(0u, *reinterpret_cast<Id*>(buf + bytesDone)); | ||
EXPECT_EQ(0u, *reinterpret_cast<Id*>(buf + bytesDone)); | ||
bytesDone += sizeof(Id); | ||
ASSERT_EQ(5u, *reinterpret_cast<Id*>(buf + bytesDone)); | ||
EXPECT_EQ(5u, *reinterpret_cast<Id*>(buf + bytesDone)); | ||
bytesDone += sizeof(Id); | ||
|
||
// Relation b2 | ||
ASSERT_EQ(Id(0), *reinterpret_cast<Id*>(buf + bytesDone)); | ||
EXPECT_EQ(Id(0), *reinterpret_cast<Id*>(buf + bytesDone)); | ||
bytesDone += sizeof(Id); | ||
ASSERT_EQ(4u, *reinterpret_cast<Id*>(buf + bytesDone)); | ||
EXPECT_EQ(4u, *reinterpret_cast<Id*>(buf + bytesDone)); | ||
bytesDone += sizeof(Id); | ||
ASSERT_EQ(1u, *reinterpret_cast<Id*>(buf + bytesDone)); | ||
EXPECT_EQ(1u, *reinterpret_cast<Id*>(buf + bytesDone)); | ||
bytesDone += sizeof(Id); | ||
ASSERT_EQ(5u, *reinterpret_cast<Id*>(buf + bytesDone)); | ||
EXPECT_EQ(5u, *reinterpret_cast<Id*>(buf + bytesDone)); | ||
bytesDone += sizeof(Id); | ||
// No LHS & RHS | ||
ASSERT_EQ(index._PSO.metaData().getOffsetAfter(), bytesDone); | ||
EXPECT_EQ(index._PSO.metaData().getOffsetAfter(), bytesDone); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Back to ASSERT_EQ
test/SerializerTest.cpp
Outdated
}; | ||
|
||
auto testWithFileSerialization = [](auto testFunction) { | ||
const std::string filename = "serializationTestTmpt.txtx"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
serializationTest.tmp
test/SerializerTest.cpp
Outdated
}; | ||
|
||
TEST(Serializer, Simple) { | ||
auto simpleIntTest = [](auto& writer, auto makeReader) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makeReaderFromWriter ?
test/SerializerTest.cpp
Outdated
}; | ||
|
||
testWithAllSerializers(testmanyPrimitives); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing newline
- It has a symmetric interface, similar to boost::serialization etc. That way, typically only one function has to be written for a type to handle reading AND writing. - It avoids the error-prone handling of manual calls to writing and reading of single bytes - We currently use it for the writing of the IndexMetaData, but it can be used in several more places (e.g. the Vocabulary) in the future.
TODO comment.