From 7807cdd731474be53dbfaf2e3a8d9ea3eb91739e Mon Sep 17 00:00:00 2001 From: Simon Shillaker Date: Mon, 4 Oct 2021 13:38:52 +0000 Subject: [PATCH 1/2] Add ignored snapshot regions --- include/faabric/util/snapshot.h | 1 + src/util/snapshot.cpp | 22 +++++++++++++- tests/test/util/test_snapshot.cpp | 48 ++++++++++++++++++++++++------- 3 files changed, 59 insertions(+), 12 deletions(-) diff --git a/include/faabric/util/snapshot.h b/include/faabric/util/snapshot.h index 703b30410..bad839899 100644 --- a/include/faabric/util/snapshot.h +++ b/include/faabric/util/snapshot.h @@ -17,6 +17,7 @@ enum SnapshotDataType enum SnapshotMergeOperation { Overwrite, + Ignore, Sum, Product, Subtract, diff --git a/src/util/snapshot.cpp b/src/util/snapshot.cpp index 5f05c5d63..c39dfb129 100644 --- a/src/util/snapshot.cpp +++ b/src/util/snapshot.cpp @@ -91,6 +91,8 @@ std::vector SnapshotData::getChangeDiffs(const uint8_t* updated, diff.dataType = region.dataType; diff.operation = region.operation; + bool isIgnore = false; + // Modify diff data for certain operations switch (region.dataType) { case (SnapshotDataType::Int): { @@ -139,6 +141,22 @@ std::vector SnapshotData::getChangeDiffs(const uint8_t* updated, break; } + case (SnapshotDataType::Raw): { + switch (region.operation) { + case (SnapshotMergeOperation::Ignore): { + isIgnore = true; + break; + } + default: { + SPDLOG_ERROR( + "Unhandled raw merge operation: {}", + region.operation); + throw std::runtime_error( + "Unhandled raw merge operation"); + } + } + break; + } default: { SPDLOG_ERROR("Merge region for unhandled data type: {}", region.dataType); @@ -148,7 +166,9 @@ std::vector SnapshotData::getChangeDiffs(const uint8_t* updated, } // Add the diff to the list - diffs.emplace_back(diff); + if (!isIgnore) { + diffs.emplace_back(diff); + } // Bump the loop variable to the end of this region (note that // the loop itself will increment onto the next) diff --git a/tests/test/util/test_snapshot.cpp b/tests/test/util/test_snapshot.cpp index 5ac92938f..031fa7245 100644 --- a/tests/test/util/test_snapshot.cpp +++ b/tests/test/util/test_snapshot.cpp @@ -248,6 +248,8 @@ TEST_CASE_METHOD(SnapshotTestFixture, "Test snapshot merge regions", "[util]") faabric::util::SnapshotMergeOperation::Overwrite; size_t dataLength = 0; + int expectedNumDiffs = 1; + SECTION("Integer") { int originalValue = 0; @@ -307,6 +309,28 @@ TEST_CASE_METHOD(SnapshotTestFixture, "Test snapshot merge regions", "[util]") expectedData = faabric::util::valueToBytes(diffValue); } + SECTION("Raw") + { + dataLength = 2 * sizeof(int32_t); + originalData = std::vector(dataLength, 3); + updatedData = originalData; + expectedData = originalData; + + dataType = faabric::util::SnapshotDataType::Raw; + operation = faabric::util::SnapshotMergeOperation::Ignore; + + SECTION("Ignore") + { + // Scatter some modifications through the updated data, to make sure + // none are picked up + updatedData[0] = 1; + updatedData[sizeof(int32_t) - 2] = 1; + updatedData[sizeof(int32_t) + 10] = 1; + + expectedNumDiffs = 0; + } + } + // Write the original data into place std::memcpy(snap.data + offset, originalData.data(), originalData.size()); @@ -333,17 +357,19 @@ TEST_CASE_METHOD(SnapshotTestFixture, "Test snapshot merge regions", "[util]") snap.getChangeDiffs(sharedMem, sharedMemSize); // Check number of diffs - REQUIRE(actualDiffs.size() == 1); - - SnapshotDiff diff = actualDiffs.at(0); - REQUIRE(diff.offset == offset); - REQUIRE(diff.operation == operation); - REQUIRE(diff.dataType == dataType); - REQUIRE(diff.size == dataLength); - - // Check actual and expected - std::vector actualData(diff.data, diff.data + dataLength); - REQUIRE(actualData == expectedData); + REQUIRE(actualDiffs.size() == expectedNumDiffs); + + if (expectedNumDiffs == 1) { + SnapshotDiff diff = actualDiffs.at(0); + REQUIRE(diff.offset == offset); + REQUIRE(diff.operation == operation); + REQUIRE(diff.dataType == dataType); + REQUIRE(diff.size == dataLength); + + // Check actual and expected + std::vector actualData(diff.data, diff.data + dataLength); + REQUIRE(actualData == expectedData); + } deallocatePages(snap.data, snapPages); } From 2aa0a2a39c7f831b1eaee4d9cea60aa674b42757 Mon Sep 17 00:00:00 2001 From: Simon Shillaker Date: Mon, 4 Oct 2021 14:05:29 +0000 Subject: [PATCH 2/2] Correct error in expected message --- tests/test/util/test_snapshot.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test/util/test_snapshot.cpp b/tests/test/util/test_snapshot.cpp index 031fa7245..db8b88089 100644 --- a/tests/test/util/test_snapshot.cpp +++ b/tests/test/util/test_snapshot.cpp @@ -403,7 +403,7 @@ TEST_CASE_METHOD(SnapshotTestFixture, "Test invalid snapshot merges", "[util]") { dataType = faabric::util::SnapshotDataType::Raw; dataLength = 123; - expectedMsg = "Merge region for unhandled data type"; + expectedMsg = "Unhandled raw merge operation"; } // Take the snapshot