Skip to content

GitLab

T
tangled
Commit 520284a7 authored by sky's avatar sky Committed by Commit bot
Browse files
Options

Revert of Restore tests for single-sample storage in histograms. (patchset #1... 

Revert of Restore tests for single-sample storage in histograms. (patchset #1 id:1 of https://codereview.chromium.org/2850233002/ )

Reason for revert:
The mac dbg builder has started failing again. See https://luci-logdog.appspot.com/v/?s=chromium%2Fbb%2Fchromium.mac%2FMac10.9_Tests__dbg_%2F40054%2F%2B%2Frecipes%2Fsteps%2Fbase_unittests%2F0%2Flogs%2FNonThreadSafeDeathTest.DestructorNotAllowedOnDifferentThreadInDebug%2F0 :
[ RUN      ] NonThreadSafeDeathTest.DestructorNotAllowedOnDifferentThreadInDebug

[WARNING] ../../testing/gtest/src/gtest-death-test.cc:834:: Death tests use fork(), which is unsafe particularly in a threaded context. For this test, Google Test detected 4 threads.
../../base/threading/non_thread_safe_unittest.cc:140: Failure
Death test: { NonThreadSafeClass::DestructorOnDifferentThreadImpl(); }
    Result: died but not with expected error.
  Expected: Check failed
Actual msg:
[  DEATH   ] The process has forked and you cannot use this CoreFoundation functionality safely. You MUST exec().
[  DEATH   ] Break on __THE_PROCESS_HAS_FORKED_AND_YOU_CANNOT_USE_THIS_COREFOUNDATION_FUNCTIONALITY___YOU_MUST_EXEC__() to debug.
[  DEATH   ] The process has forked and you cannot use this CoreFoundation functionality safely. You MUST exec().
[  DEATH   ] Break on __THE_PROCESS_HAS_FORKED_AND_YOU_CANNOT_USE_THIS_COREFOUNDATION_FUNCTIONALITY___YOU_MUST_EXEC__() to debug.
[  DEATH   ] Received signal 11 SEGV_MAPERR 000000000110
[  DEATH   ]  [0x00010bd9c12e]
[  DEATH   ]  [0x00010bd9c1cd]
[  DEATH   ]  [0x00010bd9a65c]
[  DEATH   ]  [0x00010bd9c017]
[  DEATH   ]  [0x7fff915c85aa]
[  DEATH   ]  [0x000000000000]
[  DEATH   ]  [0x7fff95a203f9]
[  DEATH   ]  [0x7fff95a2001b]
[  DEATH   ]  [0x7fff95a1fbed]
[  DEATH   ]  [0x7fff957349cb]
[  DEATH   ]  [0x7fff90a97899]
[  DEATH   ]  [0x7fff90a9772a]
[  DEATH   ]  [0x7fff90a9bfc9]
[  DEATH   ] [end of stack trace]
[  DEATH   ]
[  FAILED  ] NonThreadSafeDeathTest.DestructorNotAllowedOnDifferentThreadInDebug (533 ms)

From looking at 705342 it seems as though your changes failed with a similar error before and were reverted. Specifically NonThreadSafeDeathTest.DestructorNotAllowedOnDifferentThreadInDebug . I'm going to revert and see if it fixes things.

Original issue's description:
> Restore tests for single-sample storage in histograms.
>
> These tests were removed from the main CL because they were
> believed to be the cause of some instability in other tests.
> Without the tests, everything seems fine so now it's time to
> restore these.
>
> Main CL: https://codereview.chromium.org/2811713003/
>
> BUG=705342
>
> Review-Url: https://codereview.chromium.org/2850233002
> Cr-Commit-Position: refs/heads/master@{#468396}
> Committed: https://chromium.googlesource.com/chromium/src/+/da4b99838d459396d412bc278358b7f335ac0fdc

TBR=asvitkine@chromium.org,bcwhite@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=705342

Review-Url: https://codereview.chromium.org/2851363002
Cr-Commit-Position: refs/heads/master@{#468430}
parent 2cbf2e6e
Hide whitespace changes
Inline Side-by-side
Showing with 9 additions and 266 deletions
base/metrics/sample_vector_unittest.cc
View file @ 520284a7
......@@ -7,29 +7,18 @@
#include <limits.h>
#include <stddef.h>
#include <atomic>
#include <memory>
#include <vector>
#include "base/metrics/bucket_ranges.h"
#include "base/metrics/histogram.h"
#include "base/metrics/persistent_memory_allocator.h"
#include "base/test/gtest_util.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace base {
namespace {
// This framework class has "friend" access to the SampleVector for accessing
// non-public methods and fields.
class SampleVectorTest : public testing::Test {
public:
const HistogramBase::AtomicCount* GetSamplesCounts(
const SampleVectorBase& samples) {
return samples.counts();
}
};
TEST_F(SampleVectorTest, Accumulate) {
TEST(SampleVectorTest, AccumulateTest) {
// Custom buckets: [1, 5) [5, 10)
BucketRanges ranges(3);
ranges.set_range(0, 1);
......@@ -56,7 +45,7 @@ TEST_F(SampleVectorTest, Accumulate) {
EXPECT_EQ(samples.TotalCount(), samples.redundant_count());
}
TEST_F(SampleVectorTest, Accumulate_LargeValuesDontOverflow) {
TEST(SampleVectorTest, Accumulate_LargeValuesDontOverflow) {
// Custom buckets: [1, 250000000) [250000000, 500000000)
BucketRanges ranges(3);
ranges.set_range(0, 1);
......@@ -83,7 +72,7 @@ TEST_F(SampleVectorTest, Accumulate_LargeValuesDontOverflow) {
EXPECT_EQ(samples.TotalCount(), samples.redundant_count());
}
TEST_F(SampleVectorTest, AddSubtract) {
TEST(SampleVectorTest, AddSubtractTest) {
// Custom buckets: [0, 1) [1, 2) [2, 3) [3, INT_MAX)
BucketRanges ranges(5);
ranges.set_range(0, 0);
......@@ -127,7 +116,7 @@ TEST_F(SampleVectorTest, AddSubtract) {
EXPECT_EQ(samples1.redundant_count(), samples1.TotalCount());
}
TEST_F(SampleVectorTest, BucketIndexDeath) {
TEST(SampleVectorDeathTest, BucketIndexTest) {
// 8 buckets with exponential layout:
// [0, 1) [1, 2) [2, 4) [4, 8) [8, 16) [16, 32) [32, 64) [64, INT_MAX)
BucketRanges ranges(9);
......@@ -169,7 +158,7 @@ TEST_F(SampleVectorTest, BucketIndexDeath) {
EXPECT_DCHECK_DEATH(samples2.Accumulate(10, 100));
}
TEST_F(SampleVectorTest, AddSubtractBucketNotMatchDeath) {
TEST(SampleVectorDeathTest, AddSubtractBucketNotMatchTest) {
// Custom buckets 1: [1, 3) [3, 5)
BucketRanges ranges1(3);
ranges1.set_range(0, 1);
......@@ -208,7 +197,7 @@ TEST_F(SampleVectorTest, AddSubtractBucketNotMatchDeath) {
EXPECT_DCHECK_DEATH(samples1.Subtract(samples2));
}
TEST_F(SampleVectorTest, Iterate) {
TEST(SampleVectorIteratorTest, IterateTest) {
BucketRanges ranges(5);
ranges.set_range(0, 0);
ranges.set_range(1, 1);
......@@ -268,7 +257,7 @@ TEST_F(SampleVectorTest, Iterate) {
EXPECT_EQ(4, i);
}
TEST_F(SampleVectorTest, IterateDoneDeath) {
TEST(SampleVectorIteratorDeathTest, IterateDoneTest) {
BucketRanges ranges(5);
ranges.set_range(0, 0);
ranges.set_range(1, 1);
......@@ -293,251 +282,5 @@ TEST_F(SampleVectorTest, IterateDoneDeath) {
EXPECT_FALSE(it->Done());
}
TEST_F(SampleVectorTest, SingleSample) {
// Custom buckets: [1, 5) [5, 10)
BucketRanges ranges(3);
ranges.set_range(0, 1);
ranges.set_range(1, 5);
ranges.set_range(2, 10);
SampleVector samples(&ranges);
// Ensure that a single value accumulates correctly.
EXPECT_FALSE(GetSamplesCounts(samples));
samples.Accumulate(3, 200);
EXPECT_EQ(200, samples.GetCount(3));
EXPECT_FALSE(GetSamplesCounts(samples));
samples.Accumulate(3, 400);
EXPECT_EQ(600, samples.GetCount(3));
EXPECT_FALSE(GetSamplesCounts(samples));
EXPECT_EQ(3 * 600, samples.sum());
EXPECT_EQ(600, samples.TotalCount());
EXPECT_EQ(600, samples.redundant_count());
// Ensure that the iterator returns only one value.
HistogramBase::Sample min;
HistogramBase::Sample max;
HistogramBase::Count count;
std::unique_ptr<SampleCountIterator> it = samples.Iterator();
ASSERT_FALSE(it->Done());
it->Get(&min, &max, &count);
EXPECT_EQ(1, min);
EXPECT_EQ(5, max);
EXPECT_EQ(600, count);
it->Next();
EXPECT_TRUE(it->Done());
// Ensure that it can be merged to another single-sample vector.
SampleVector samples_copy(&ranges);
samples_copy.Add(samples);
EXPECT_FALSE(GetSamplesCounts(samples_copy));
EXPECT_EQ(3 * 600, samples_copy.sum());
EXPECT_EQ(600, samples_copy.TotalCount());
EXPECT_EQ(600, samples_copy.redundant_count());
// A different value should cause creation of the counts array.
samples.Accumulate(8, 100);
EXPECT_TRUE(GetSamplesCounts(samples));
EXPECT_EQ(600, samples.GetCount(3));
EXPECT_EQ(100, samples.GetCount(8));
EXPECT_EQ(3 * 600 + 8 * 100, samples.sum());
EXPECT_EQ(600 + 100, samples.TotalCount());
EXPECT_EQ(600 + 100, samples.redundant_count());
// The iterator should now return both values.
it = samples.Iterator();
ASSERT_FALSE(it->Done());
it->Get(&min, &max, &count);
EXPECT_EQ(1, min);
EXPECT_EQ(5, max);
EXPECT_EQ(600, count);
it->Next();
ASSERT_FALSE(it->Done());
it->Get(&min, &max, &count);
EXPECT_EQ(5, min);
EXPECT_EQ(10, max);
EXPECT_EQ(100, count);
it->Next();
EXPECT_TRUE(it->Done());
// Ensure that it can merged to a single-sample vector.
samples_copy.Add(samples);
EXPECT_TRUE(GetSamplesCounts(samples_copy));
EXPECT_EQ(3 * 1200 + 8 * 100, samples_copy.sum());
EXPECT_EQ(1200 + 100, samples_copy.TotalCount());
EXPECT_EQ(1200 + 100, samples_copy.redundant_count());
}
TEST_F(SampleVectorTest, PersistentSampleVector) {
LocalPersistentMemoryAllocator allocator(64 << 10, 0, "");
std::atomic<PersistentMemoryAllocator::Reference> samples_ref;
samples_ref.store(0, std::memory_order_relaxed);
HistogramSamples::Metadata samples_meta;
memset(&samples_meta, 0, sizeof(samples_meta));
// Custom buckets: [1, 5) [5, 10)
BucketRanges ranges(3);
ranges.set_range(0, 1);
ranges.set_range(1, 5);
ranges.set_range(2, 10);
// Persistent allocation.
const size_t counts_bytes =
sizeof(HistogramBase::AtomicCount) * ranges.bucket_count();
const DelayedPersistentAllocation allocation(&allocator, &samples_ref, 1,
counts_bytes, false);
PersistentSampleVector samples1(0, &ranges, &samples_meta, allocation);
EXPECT_FALSE(GetSamplesCounts(samples1));
samples1.Accumulate(3, 200);
EXPECT_EQ(200, samples1.GetCount(3));
EXPECT_FALSE(GetSamplesCounts(samples1));
EXPECT_EQ(0, samples1.GetCount(8));
EXPECT_FALSE(GetSamplesCounts(samples1));
PersistentSampleVector samples2(0, &ranges, &samples_meta, allocation);
EXPECT_EQ(200, samples2.GetCount(3));
EXPECT_FALSE(GetSamplesCounts(samples2));
HistogramBase::Sample min;
HistogramBase::Sample max;
HistogramBase::Count count;
std::unique_ptr<SampleCountIterator> it = samples2.Iterator();
ASSERT_FALSE(it->Done());
it->Get(&min, &max, &count);
EXPECT_EQ(1, min);
EXPECT_EQ(5, max);
EXPECT_EQ(200, count);
it->Next();
EXPECT_TRUE(it->Done());
samples1.Accumulate(8, 100);
EXPECT_TRUE(GetSamplesCounts(samples1));
EXPECT_FALSE(GetSamplesCounts(samples2));
EXPECT_EQ(200, samples2.GetCount(3));
EXPECT_EQ(100, samples2.GetCount(8));
EXPECT_TRUE(GetSamplesCounts(samples2));
EXPECT_EQ(3 * 200 + 8 * 100, samples2.sum());
EXPECT_EQ(300, samples2.TotalCount());
EXPECT_EQ(300, samples2.redundant_count());
it = samples2.Iterator();
ASSERT_FALSE(it->Done());
it->Get(&min, &max, &count);
EXPECT_EQ(1, min);
EXPECT_EQ(5, max);
EXPECT_EQ(200, count);
it->Next();
ASSERT_FALSE(it->Done());
it->Get(&min, &max, &count);
EXPECT_EQ(5, min);
EXPECT_EQ(10, max);
EXPECT_EQ(100, count);
it->Next();
EXPECT_TRUE(it->Done());
PersistentSampleVector samples3(0, &ranges, &samples_meta, allocation);
EXPECT_TRUE(GetSamplesCounts(samples2));
EXPECT_EQ(200, samples3.GetCount(3));
EXPECT_EQ(100, samples3.GetCount(8));
EXPECT_EQ(3 * 200 + 8 * 100, samples3.sum());
EXPECT_EQ(300, samples3.TotalCount());
EXPECT_EQ(300, samples3.redundant_count());
it = samples3.Iterator();
ASSERT_FALSE(it->Done());
it->Get(&min, &max, &count);
EXPECT_EQ(1, min);
EXPECT_EQ(5, max);
EXPECT_EQ(200, count);
it->Next();
ASSERT_FALSE(it->Done());
it->Get(&min, &max, &count);
EXPECT_EQ(5, min);
EXPECT_EQ(10, max);
EXPECT_EQ(100, count);
it->Next();
EXPECT_TRUE(it->Done());
}
TEST_F(SampleVectorTest, PersistentSampleVectorTestWithOutsideAlloc) {
LocalPersistentMemoryAllocator allocator(64 << 10, 0, "");
std::atomic<PersistentMemoryAllocator::Reference> samples_ref;
samples_ref.store(0, std::memory_order_relaxed);
HistogramSamples::Metadata samples_meta;
memset(&samples_meta, 0, sizeof(samples_meta));
// Custom buckets: [1, 5) [5, 10)
BucketRanges ranges(3);
ranges.set_range(0, 1);
ranges.set_range(1, 5);
ranges.set_range(2, 10);
// Persistent allocation.
const size_t counts_bytes =
sizeof(HistogramBase::AtomicCount) * ranges.bucket_count();
const DelayedPersistentAllocation allocation(&allocator, &samples_ref, 1,
counts_bytes, false);
PersistentSampleVector samples1(0, &ranges, &samples_meta, allocation);
EXPECT_FALSE(GetSamplesCounts(samples1));
samples1.Accumulate(3, 200);
EXPECT_EQ(200, samples1.GetCount(3));
EXPECT_FALSE(GetSamplesCounts(samples1));
// Because the delayed allocation can be shared with other objects (the
// |offset| parameter allows concatinating multiple data blocks into the
// same allocation), it's possible that the allocation gets realized from
// the outside even though the data block being accessed is all zero.
allocation.Get();
EXPECT_EQ(200, samples1.GetCount(3));
EXPECT_FALSE(GetSamplesCounts(samples1));
HistogramBase::Sample min;
HistogramBase::Sample max;
HistogramBase::Count count;
std::unique_ptr<SampleCountIterator> it = samples1.Iterator();
ASSERT_FALSE(it->Done());
it->Get(&min, &max, &count);
EXPECT_EQ(1, min);
EXPECT_EQ(5, max);
EXPECT_EQ(200, count);
it->Next();
EXPECT_TRUE(it->Done());
// A duplicate samples object should still see the single-sample entry even
// when storage is available.
PersistentSampleVector samples2(0, &ranges, &samples_meta, allocation);
EXPECT_EQ(200, samples2.GetCount(3));
// New accumulations, in both directions, of the existing value should work.
samples1.Accumulate(3, 50);
EXPECT_EQ(250, samples1.GetCount(3));
EXPECT_EQ(250, samples2.GetCount(3));
samples2.Accumulate(3, 50);
EXPECT_EQ(300, samples1.GetCount(3));
EXPECT_EQ(300, samples2.GetCount(3));
it = samples1.Iterator();
ASSERT_FALSE(it->Done());
it->Get(&min, &max, &count);
EXPECT_EQ(1, min);
EXPECT_EQ(5, max);
EXPECT_EQ(300, count);
it->Next();
EXPECT_TRUE(it->Done());
samples1.Accumulate(8, 100);
EXPECT_TRUE(GetSamplesCounts(samples1));
EXPECT_EQ(300, samples1.GetCount(3));
EXPECT_EQ(300, samples2.GetCount(3));
EXPECT_EQ(100, samples1.GetCount(8));
EXPECT_EQ(100, samples2.GetCount(8));
samples2.Accumulate(8, 100);
EXPECT_EQ(300, samples1.GetCount(3));
EXPECT_EQ(300, samples2.GetCount(3));
EXPECT_EQ(200, samples1.GetCount(8));
EXPECT_EQ(200, samples2.GetCount(8));
}
} // namespace
} // namespace base
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment