From 9fd7ce83d0436a604fdea7043b94521665483ec1 Mon Sep 17 00:00:00 2001 From: Mark Abraham Date: Mon, 30 Dec 2019 12:44:17 +0000 Subject: [PATCH] Tests that verify serializer behaviours xdr_vector serializing char uses four bytes per char, which is wasteful when that char buffer is just a binary dump such as with the tpr file body. This test ensures that our assumption when writing tpr files is that we can reinterpret as a wider data type and avoid excess padding. It also tests that xdr_opaque does not introduce such padding. Refs #3269 Refs #3276 Change-Id: I9c40b8ab7b9dabe461cb4dd146bb132d8f5a9681 --- src/gromacs/fileio/tests/CMakeLists.txt | 1 + src/gromacs/fileio/tests/fileioxdrserializer.cpp | 150 +++++++++++++++++++++++ src/gromacs/utility/inmemoryserializer.cpp | 17 ++- src/gromacs/utility/tests/inmemoryserializer.cpp | 22 ++++ 4 files changed, 185 insertions(+), 5 deletions(-) create mode 100644 src/gromacs/fileio/tests/fileioxdrserializer.cpp diff --git a/src/gromacs/fileio/tests/CMakeLists.txt b/src/gromacs/fileio/tests/CMakeLists.txt index 788cdaa6da..2f6e3e5dee 100644 --- a/src/gromacs/fileio/tests/CMakeLists.txt +++ b/src/gromacs/fileio/tests/CMakeLists.txt @@ -39,6 +39,7 @@ set(test_sources mrcdensitymap.cpp mrcdensitymapheader.cpp readinp.cpp + fileioxdrserializer.cpp ) if (GMX_USE_TNG) list(APPEND test_sources tngio.cpp) diff --git a/src/gromacs/fileio/tests/fileioxdrserializer.cpp b/src/gromacs/fileio/tests/fileioxdrserializer.cpp new file mode 100644 index 0000000000..1523ab20db --- /dev/null +++ b/src/gromacs/fileio/tests/fileioxdrserializer.cpp @@ -0,0 +1,150 @@ +/* + * This file is part of the GROMACS molecular simulation package. + * + * Copyright (c) 2019, by the GROMACS development team, led by + * Mark Abraham, David van der Spoel, Berk Hess, and Erik Lindahl, + * and including many others, as listed in the AUTHORS file in the + * top-level source directory and at http://www.gromacs.org. + * + * GROMACS is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public License + * as published by the Free Software Foundation; either version 2.1 + * of the License, or (at your option) any later version. + * + * GROMACS is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with GROMACS; if not, see + * http://www.gnu.org/licenses, or write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + * + * If you want to redistribute modifications to GROMACS, please + * consider that scientific software is very special. Version + * control is crucial - bugs must be traceable. We will be happy to + * consider code for inclusion in the official distribution, but + * derived work must not be called official GROMACS. Details are found + * in the README & COPYING files - if they are missing, get the + * official version at http://www.gromacs.org. + * + * To help us fund GROMACS development, we humbly ask that you cite + * the research papers on the package. Check out http://www.gromacs.org. + */ +/*! \internal \file + * \brief + * Tests for gmx::FileIOXdrSerializer. + * + * \author Mark Abraham + * \ingroup module_fileio + */ + +#include "gmxpre.h" + +#include + +#include "gromacs/fileio/gmxfio.h" +#include "gromacs/fileio/gmxfio_xdr.h" +#include "gromacs/utility/futil.h" + +#include "testutils/testfilemanager.h" + +namespace gmx +{ +namespace test +{ +namespace +{ +union IntAndFloat32 { + std::int32_t int32Value_; + float floatValue_; +}; + +union IntAndFloat64 { + std::int64_t int64Value_; + double doubleValue_; +}; + +//! Constants used for testing endian swap operations +//! \{ +constexpr std::int16_t c_int16Value = static_cast(0x7A2B); +constexpr std::int32_t c_int32Value = static_cast(0x78ABCDEF); +constexpr std::int64_t c_int64Value = static_cast(0x78ABCDEF12345678); + +constexpr const IntAndFloat32 c_intAndFloat32{ c_int32Value }; +constexpr const IntAndFloat64 c_intAndFloat64{ c_int64Value }; +//! \} + +//! Return the integer used for testing, depending on the size of int. +constexpr int integerSizeDependentTestingValue() +{ + return sizeof(int) == 4 ? c_int32Value : sizeof(int) == 8 ? c_int64Value : c_int16Value; +} + +class FileIOXdrSerializerTest : public ::testing::Test +{ +public: + ~FileIOXdrSerializerTest() override + { + if (file_) + { + gmx_fio_close(file_); + } + } + struct SerializerValues + { + bool boolValue_ = true; + unsigned char unsignedCharValue_ = 0x78; + char charValue_ = 0x78; + unsigned short unsignedShortValue_ = static_cast(c_int16Value); + std::int32_t int32Value_ = c_int32Value; + float floatValue_ = c_intAndFloat32.floatValue_; + std::int64_t int64Value_ = c_int64Value; + double doubleValue_ = c_intAndFloat64.doubleValue_; + int intValue_ = integerSizeDependentTestingValue(); + real realValue_ = std::is_same::value + ? static_cast(c_intAndFloat64.doubleValue_) + : static_cast(c_intAndFloat32.floatValue_); + } defaultValues_; + + TestFileManager fileManager_; + // Make sure the file extension is one that gmx_fio_open will + // recognize to open as binary, even though we're just abusing it + // to write arbitrary XDR output. + std::string filename_ = fileManager_.getTemporaryFilePath("data.edr"); + t_fileio* file_ = nullptr; +}; + +TEST_F(FileIOXdrSerializerTest, SizeIsCorrect) +{ + file_ = gmx_fio_open(filename_.c_str(), "w"); + FileIOXdrSerializer serializer(file_); + // These types all have well-defined widths in bytes AFTER XDR serialization, + // which we can test below. + serializer.doBool(&defaultValues_.boolValue_); // 4 bytes + serializer.doChar(&defaultValues_.charValue_); // 4 bytes + serializer.doInt32(&defaultValues_.int32Value_); // 4 bytes + serializer.doInt64(&defaultValues_.int64Value_); // 8 bytes + serializer.doFloat(&defaultValues_.floatValue_); // 4 bytes + serializer.doDouble(&defaultValues_.doubleValue_); // 8 bytes + std::vector charBuffer = { 'a', 'b', 'c' }; + serializer.doCharArray(charBuffer.data(), charBuffer.size()); // 12 bytes + serializer.doOpaque(charBuffer.data(), charBuffer.size()); // 4 bytes + std::vector int32Buffer = { 0x1BCDEF78, 0x654321FE }; + serializer.doInt32Array(int32Buffer.data(), int32Buffer.size()); // 8 bytes + std::vector int64Buffer = { 0x1BCDEF78654321FE, 0x3726ABFEAB34716C }; + serializer.doInt64Array(int64Buffer.data(), int64Buffer.size()); // 16 bytes + gmx_fio_close(file_); + + file_ = gmx_fio_open(filename_.c_str(), "r"); + + // Determine file size + gmx_fseek(gmx_fio_getfp(file_), 0, SEEK_END); + gmx_off_t fileSize = gmx_fio_ftell(file_); + EXPECT_EQ(fileSize, 72); +} + +} // namespace +} // namespace test +} // namespace gmx diff --git a/src/gromacs/utility/inmemoryserializer.cpp b/src/gromacs/utility/inmemoryserializer.cpp index 89edcf07d0..2dec0caec9 100644 --- a/src/gromacs/utility/inmemoryserializer.cpp +++ b/src/gromacs/utility/inmemoryserializer.cpp @@ -32,6 +32,13 @@ * To help us fund GROMACS development, we humbly ask that you cite * the research papers on the package. Check out http://www.gromacs.org. */ +/*! \internal \file + * \brief + * Defines gmx::ISerializer implementation for in-memory serialization. + * + * \author Teemu Murtola + * \ingroup module_utility + */ #include "gmxpre.h" #include "inmemoryserializer.h" @@ -90,11 +97,11 @@ T swapEndian(const T& value) return endianessSwappedValue.value_; } -//! \brief Change the host-dependent endian settings to either Swap or DoNotSwap. -// -// \param endianSwapBehavior input swap behavior, might depend on host. -// -// \return Host-independent setting, either Swap or DoNotSwap. +/*! \brief Change the host-dependent endian settings to either Swap or DoNotSwap. + * + * \param endianSwapBehavior input swap behavior, might depend on host. + * + * \return Host-independent setting, either Swap or DoNotSwap. */ EndianSwapBehavior setEndianSwapBehaviorFromHost(EndianSwapBehavior endianSwapBehavior) { if (endianSwapBehavior == EndianSwapBehavior::SwapIfHostIsBigEndian) diff --git a/src/gromacs/utility/tests/inmemoryserializer.cpp b/src/gromacs/utility/tests/inmemoryserializer.cpp index 81bf81cbc2..f17a83894f 100644 --- a/src/gromacs/utility/tests/inmemoryserializer.cpp +++ b/src/gromacs/utility/tests/inmemoryserializer.cpp @@ -241,6 +241,28 @@ TEST_F(InMemorySerializerTest, DeserializerExplicitEndianessSwap) checkSerializerValuesforEquality(endianessSwappedValues_, deserialisedValues); } +TEST_F(InMemorySerializerTest, SizeIsCorrect) +{ + InMemorySerializer serializer; + // These types all have well-defined widths in bytes, + // which we can test below. + serializer.doBool(&defaultValues_.boolValue_); // 1 bytes + serializer.doChar(&defaultValues_.charValue_); // 1 bytes + serializer.doInt32(&defaultValues_.int32Value_); // 4 bytes + serializer.doInt64(&defaultValues_.int64Value_); // 8 bytes + serializer.doFloat(&defaultValues_.floatValue_); // 4 bytes + serializer.doDouble(&defaultValues_.doubleValue_); // 8 bytes + std::vector charBuffer = { 'a', 'b', 'c' }; + serializer.doCharArray(charBuffer.data(), charBuffer.size()); // 3 bytes + serializer.doOpaque(charBuffer.data(), charBuffer.size()); // 3 bytes + std::vector int32Buffer = { 0x1BCDEF78, 0x654321FE }; + serializer.doInt32Array(int32Buffer.data(), int32Buffer.size()); // 8 bytes + std::vector int64Buffer = { 0x1BCDEF78654321FE, 0x3726ABFEAB34716C }; + serializer.doInt64Array(int64Buffer.data(), int64Buffer.size()); // 16 bytes + auto buffer = serializer.finishAndGetBuffer(); + EXPECT_EQ(buffer.size(), 56); +} + } // namespace } // namespace test } // namespace gmx -- 2.11.4.GIT