[System Monitor] Fix samples used and crash
This CL changes which samples are used in a given GUI column. The
prior math was inclusive on both ends; the new math is exclusive on
the later value. This means that samples that were on the boundary
between two columns where counted in both columns. Now they are no
longer being double counted.
Second, this CL checks against failing to find the *any* samples in
a range, which could lead to a crash.
Test: garnet/lib/system_monitor/dockyard/dockyard_unittests.cc
Change-Id: I8d7f686c4236e75e5e6628d9420148a4601d7fbd
diff --git a/garnet/lib/system_monitor/dockyard/dockyard.cc b/garnet/lib/system_monitor/dockyard/dockyard.cc
index 3d21e0af..b5d43d69 100644
--- a/garnet/lib/system_monitor/dockyard/dockyard.cc
+++ b/garnet/lib/system_monitor/dockyard/dockyard.cc
@@ -103,6 +103,10 @@
} // namespace
+bool StreamSetsRequest::HasFlag(StreamSetsRequestFlags flag) const {
+ return (flags & flag) != 0;
+}
+
Dockyard::Dockyard()
: server_thread_(nullptr),
stream_name_handler_(nullptr),
@@ -238,7 +242,7 @@
default:
break;
}
- if ((request.flags & StreamSetsRequest::NORMALIZE) != 0) {
+ if (request.HasFlag(StreamSetsRequest::NORMALIZE)) {
NormalizeResponse(*stream_id, sample_stream, request, &samples);
}
}
@@ -258,9 +262,13 @@
for (uint64_t sample_n = 0; sample_n < request.sample_count; ++sample_n) {
auto start_time = request.start_time_ns + sample_n * stride;
auto begin = sample_stream.lower_bound(start_time);
- auto end = sample_stream.upper_bound(start_time + stride);
+ if (begin == sample_stream.end()) {
+ samples->push_back(NO_DATA);
+ continue;
+ }
+ auto end = sample_stream.lower_bound(start_time + stride);
SampleValue accumulator = 0ULL;
- SampleValue count = 0ULL;
+ uint_fast32_t count = 0ULL;
for (auto i = begin; i != end; ++i) {
accumulator += i->second;
++count;
@@ -268,7 +276,7 @@
if (count) {
samples->push_back(accumulator / count);
} else {
- samples->push_back(-2ULL);
+ samples->push_back(NO_DATA);
}
}
}
@@ -280,9 +288,13 @@
for (uint64_t sample_n = 0; sample_n < request.sample_count; ++sample_n) {
auto start_time = request.start_time_ns + sample_n * stride;
auto begin = sample_stream.lower_bound(start_time);
- auto end = sample_stream.upper_bound(start_time + stride);
+ if (begin == sample_stream.end()) {
+ samples->push_back(NO_DATA);
+ continue;
+ }
+ auto end = sample_stream.lower_bound(start_time + stride);
SampleValue highest = 0ULL;
- SampleValue count = 0ULL;
+ uint_fast32_t count = 0ULL;
for (auto i = begin; i != end; ++i) {
if (highest < i->second) {
highest = i->second;
@@ -292,7 +304,7 @@
if (count) {
samples->push_back(highest);
} else {
- samples->push_back(-2ULL);
+ samples->push_back(NO_DATA);
}
}
}
@@ -305,9 +317,13 @@
for (uint64_t sample_n = 0; sample_n < request.sample_count; ++sample_n) {
auto start_time = request.start_time_ns + sample_n * stride;
auto begin = sample_stream.lower_bound(start_time);
- auto end = sample_stream.upper_bound(start_time + stride);
+ if (begin == sample_stream.end()) {
+ samples->push_back(NO_DATA);
+ continue;
+ }
+ auto end = sample_stream.lower_bound(start_time + stride);
SampleValue lowest = SAMPLE_MAX_VALUE;
- SampleValue count = 0ULL;
+ uint_fast32_t count = 0ULL;
for (auto i = begin; i != end; ++i) {
if (lowest > i->second) {
lowest = i->second;
@@ -317,7 +333,7 @@
if (count) {
samples->push_back(lowest);
} else {
- samples->push_back(-2ULL);
+ samples->push_back(NO_DATA);
}
}
}
@@ -352,11 +368,15 @@
for (uint64_t sample_n = 0; sample_n < request.sample_count; ++sample_n) {
auto start_time = request.start_time_ns + sample_n * stride;
auto begin = sample_stream.lower_bound(start_time);
- auto end = sample_stream.upper_bound(start_time + stride);
+ if (begin == sample_stream.end()) {
+ samples->push_back(NO_DATA);
+ continue;
+ }
+ auto end = sample_stream.lower_bound(start_time + stride);
SampleValue accumulator = 0ULL;
SampleValue highest = 0ULL;
SampleValue lowest = SAMPLE_MAX_VALUE;
- SampleValue count = 0ULL;
+ uint_fast32_t count = 0ULL;
for (auto i = begin; i != end; ++i) {
auto value = i->second;
accumulator += value;
@@ -373,7 +393,7 @@
auto final = average >= overall_average ? highest : lowest;
samples->push_back(final);
} else {
- samples->push_back(-2ULL);
+ samples->push_back(NO_DATA);
}
}
}
@@ -386,9 +406,13 @@
for (uint64_t sample_n = 0; sample_n < request.sample_count; ++sample_n) {
auto start_time = request.start_time_ns + sample_n * stride;
auto begin = sample_stream.lower_bound(start_time - stride);
- auto end = sample_stream.upper_bound(start_time + stride * 2);
+ if (begin == sample_stream.end()) {
+ samples->push_back(NO_DATA);
+ continue;
+ }
+ auto end = sample_stream.lower_bound(start_time + stride * 2);
SampleValue accumulator = 0ULL;
- SampleValue count = 0ULL;
+ uint_fast32_t count = 0ULL;
for (auto i = begin; i != end; ++i) {
accumulator += i->second;
++count;
@@ -396,7 +420,7 @@
if (count) {
samples->push_back(accumulator / count);
} else {
- samples->push_back(-2ULL);
+ samples->push_back(NO_DATA);
}
}
}
diff --git a/garnet/lib/system_monitor/dockyard/dockyard.h b/garnet/lib/system_monitor/dockyard/dockyard.h
index fbe49ab..f63cf24 100644
--- a/garnet/lib/system_monitor/dockyard/dockyard.h
+++ b/garnet/lib/system_monitor/dockyard/dockyard.h
@@ -112,6 +112,8 @@
// A StreamInfo::id for each stream of interest.
std::vector<SampleStreamId> stream_ids;
+
+ bool HasFlag(StreamSetsRequestFlags flag) const;
};
struct StreamSetsResponse {
diff --git a/garnet/lib/system_monitor/dockyard/dockyard_unittests.cc b/garnet/lib/system_monitor/dockyard/dockyard_unittests.cc
index e4ef307..d1baac9 100644
--- a/garnet/lib/system_monitor/dockyard/dockyard_unittests.cc
+++ b/garnet/lib/system_monitor/dockyard/dockyard_unittests.cc
@@ -99,8 +99,8 @@
ASSERT_EQ(SAMPLE_COUNT, _response.data_sets[0].size());
// Check the samples themselves.
EXPECT_EQ(8ULL, _response.data_sets[0][0]);
- EXPECT_EQ(10ULL, _response.data_sets[0][1]);
- EXPECT_EQ(15ULL, _response.data_sets[0][2]);
+ EXPECT_EQ(dockyard::NO_DATA, _response.data_sets[0][1]);
+ EXPECT_EQ(10ULL, _response.data_sets[0][2]);
EXPECT_EQ(20ULL, _response.data_sets[0][3]);
EXPECT_EQ(dockyard::NO_DATA, _response.data_sets[0][4]);
EXPECT_EQ(dockyard::NO_DATA, _response.data_sets[0][5]);
@@ -138,7 +138,7 @@
EXPECT_EQ(10ULL, _response.data_sets[0][6]);
EXPECT_EQ(dockyard::NO_DATA, _response.data_sets[0][7]);
EXPECT_EQ(dockyard::NO_DATA, _response.data_sets[0][8]);
- EXPECT_EQ(20ULL, _response.data_sets[0][9]);
+ EXPECT_EQ(dockyard::NO_DATA, _response.data_sets[0][9]);
}
TEST_F(SystemMonitorDockyardTest, RawDataSetsCpu1) {
@@ -193,10 +193,10 @@
ASSERT_EQ(1UL, _response.data_sets.size());
ASSERT_EQ(SAMPLE_COUNT, _response.data_sets[0].size());
// Check the samples themselves.
- EXPECT_EQ(30ULL, _response.data_sets[0][0]);
- EXPECT_EQ(71ULL, _response.data_sets[0][1]);
- EXPECT_EQ(31ULL, _response.data_sets[0][2]);
- EXPECT_EQ(7ULL, _response.data_sets[0][3]);
+ EXPECT_EQ(15ULL, _response.data_sets[0][0]);
+ EXPECT_EQ(78ULL, _response.data_sets[0][1]);
+ EXPECT_EQ(38ULL, _response.data_sets[0][2]);
+ EXPECT_EQ(8ULL, _response.data_sets[0][3]);
EXPECT_EQ(18ULL, _response.data_sets[0][4]);
}
@@ -226,10 +226,10 @@
// Check the samples themselves.
// CPU 0.
EXPECT_EQ(dockyard::NO_DATA, _response.data_sets[0][0]);
- EXPECT_EQ(10ULL, _response.data_sets[0][1]);
+ EXPECT_EQ(dockyard::NO_DATA, _response.data_sets[0][1]);
// CPU 1.
EXPECT_EQ(10ULL, _response.data_sets[1][0]);
- EXPECT_EQ(100ULL, _response.data_sets[1][1]);
+ EXPECT_EQ(dockyard::NO_DATA, _response.data_sets[1][1]);
// CPU 2.
EXPECT_EQ(46ULL, _response.data_sets[2][0]);
EXPECT_EQ(17ULL, _response.data_sets[2][1]);
@@ -314,10 +314,10 @@
ASSERT_EQ(1UL, _response.data_sets.size());
ASSERT_EQ(SAMPLE_COUNT, _response.data_sets[0].size());
// Check the samples themselves.
- EXPECT_EQ(278350ULL, _response.data_sets[0][0]);
- EXPECT_EQ(701030ULL, _response.data_sets[0][1]);
- EXPECT_EQ(288659ULL, _response.data_sets[0][2]);
- EXPECT_EQ(41237ULL, _response.data_sets[0][3]);
+ EXPECT_EQ(123711ULL, _response.data_sets[0][0]);
+ EXPECT_EQ(773195ULL, _response.data_sets[0][1]);
+ EXPECT_EQ(360824ULL, _response.data_sets[0][2]);
+ EXPECT_EQ(51546ULL, _response.data_sets[0][3]);
EXPECT_EQ(154639ULL, _response.data_sets[0][4]);
}
@@ -341,9 +341,9 @@
ASSERT_EQ(1UL, _response.data_sets.size());
ASSERT_EQ(SAMPLE_COUNT, _response.data_sets[0].size());
// Check the samples themselves.
- EXPECT_EQ(46ULL, _response.data_sets[0][0]);
- EXPECT_EQ(41ULL, _response.data_sets[0][1]);
- EXPECT_EQ(38ULL, _response.data_sets[0][2]);
+ EXPECT_EQ(47ULL, _response.data_sets[0][0]);
+ EXPECT_EQ(44ULL, _response.data_sets[0][1]);
+ EXPECT_EQ(42ULL, _response.data_sets[0][2]);
EXPECT_EQ(20ULL, _response.data_sets[0][3]);
EXPECT_EQ(13ULL, _response.data_sets[0][4]);
}
@@ -370,8 +370,8 @@
// Check the samples themselves.
EXPECT_EQ(3ULL, _response.data_sets[0][0]);
EXPECT_EQ(100ULL, _response.data_sets[0][1]);
- EXPECT_EQ(12ULL, _response.data_sets[0][2]);
- EXPECT_EQ(3ULL, _response.data_sets[0][3]);
+ EXPECT_EQ(30ULL, _response.data_sets[0][2]);
+ EXPECT_EQ(5ULL, _response.data_sets[0][3]);
EXPECT_EQ(3ULL, _response.data_sets[0][4]);
}
@@ -407,10 +407,10 @@
ASSERT_EQ(1UL, _response.data_sets.size());
ASSERT_EQ(SAMPLE_COUNT, _response.data_sets[0].size());
// Check the samples themselves.
- EXPECT_EQ(59ULL, _response.data_sets[0][0]);
- EXPECT_EQ(97ULL, _response.data_sets[0][9]);
- EXPECT_EQ(26ULL, _response.data_sets[0][19]);
- EXPECT_EQ(33ULL, _response.data_sets[0][29]);
+ EXPECT_EQ(55ULL, _response.data_sets[0][0]);
+ EXPECT_EQ(99ULL, _response.data_sets[0][9]);
+ EXPECT_EQ(29ULL, _response.data_sets[0][19]);
+ EXPECT_EQ(29ULL, _response.data_sets[0][29]);
EXPECT_EQ(99ULL, _response.data_sets[0][39]);
}
