Skip to content
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.

Sign up for GitHub

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

Add ignored snapshot regions #148

Merged
merged 2 commits into from
Oct 4, 2021
Merged

Add ignored snapshot regions #148

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7807cdd
Add ignored snapshot regions
Shillaker Oct 4, 2021
2aa0a2a
Correct error in expected message
Shillaker Oct 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions include/faabric/util/snapshot.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ enum SnapshotDataType
enum SnapshotMergeOperation
{
Overwrite,
Ignore,
Sum,
Product,
Subtract,
Expand Down
22 changes: 21 additions & 1 deletion src/util/snapshot.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ std::vector<SnapshotDiff> 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): {
Expand Down Expand Up @@ -139,6 +141,22 @@ std::vector<SnapshotDiff> 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);
Expand All @@ -148,7 +166,9 @@ std::vector<SnapshotDiff> 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)
Expand Down
50 changes: 38 additions & 12 deletions tests/test/util/test_snapshot.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -307,6 +309,28 @@ TEST_CASE_METHOD(SnapshotTestFixture, "Test snapshot merge regions", "[util]")
expectedData = faabric::util::valueToBytes<int>(diffValue);
}

SECTION("Raw")
{
dataLength = 2 * sizeof(int32_t);
originalData = std::vector<uint8_t>(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());

Expand All @@ -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<uint8_t> 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<uint8_t> actualData(diff.data, diff.data + dataLength);
REQUIRE(actualData == expectedData);
}

deallocatePages(snap.data, snapPages);
}
Expand Down Expand Up @@ -377,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
Expand Down