Commit 699f67f7 authored by Nicolas Pena's avatar Nicolas Pena Committed by Commit Bot

Speculative fix for 0-valued performance.memory

This CL ensures that the first call to MaybeUpdate() results in calling
Update() so that MemoryInfo is populated. Technically the monotonic
clock could be initialized to a small value and in this case the first
time this check occurs it could fail, whereas we want it to pass:
"now - last_update_time_ >= delta_allowed"

Bug: 883490
Change-Id: I7143d453709db38068919cff9585c50e06e789ac
Reviewed-on: https://chromium-review.googlesource.com/1228886Reviewed-by: default avatarSteve Kobes <skobes@chromium.org>
Commit-Queue: Nicolás Peña Moreno <npm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592071}
parent 07c179d9
...@@ -81,7 +81,8 @@ class HeapSizeCache { ...@@ -81,7 +81,8 @@ class HeapSizeCache {
TimeDelta delta_allowed = precision == MemoryInfo::Precision::Bucketized TimeDelta delta_allowed = precision == MemoryInfo::Precision::Bucketized
? kTwentyMinutes ? kTwentyMinutes
: kFiftyMs; : kFiftyMs;
if (now - last_update_time_ >= delta_allowed) { if (!last_update_time_.has_value() ||
now - last_update_time_.value() >= delta_allowed) {
Update(precision); Update(precision);
last_update_time_ = now; last_update_time_ = now;
} }
...@@ -97,7 +98,7 @@ class HeapSizeCache { ...@@ -97,7 +98,7 @@ class HeapSizeCache {
info_.js_heap_size_limit = QuantizeMemorySize(info_.js_heap_size_limit); info_.js_heap_size_limit = QuantizeMemorySize(info_.js_heap_size_limit);
} }
TimeTicks last_update_time_; base::Optional<TimeTicks> last_update_time_;
HeapInfo info_; HeapInfo info_;
DISALLOW_COPY_AND_ASSIGN(HeapSizeCache); DISALLOW_COPY_AND_ASSIGN(HeapSizeCache);
...@@ -164,6 +165,9 @@ MemoryInfo::MemoryInfo(Precision precision) { ...@@ -164,6 +165,9 @@ MemoryInfo::MemoryInfo(Precision precision) {
GetHeapSize(info_); GetHeapSize(info_);
else else
HeapSizeCache::ForCurrentThread().GetCachedHeapSize(info_, precision); HeapSizeCache::ForCurrentThread().GetCachedHeapSize(info_, precision);
// The values must have been computed, so totalJSHeapSize must be greater than
// 0.
DCHECK_GT(totalJSHeapSize(), 0u);
} }
} // namespace blink } // namespace blink
...@@ -231,4 +231,24 @@ TEST_F(MemoryInfoTest, FlagEnabled) { ...@@ -231,4 +231,24 @@ TEST_F(MemoryInfoTest, FlagEnabled) {
precise_memory->usedJSHeapSize()); precise_memory->usedJSHeapSize());
} }
TEST_F(MemoryInfoTest, ZeroTime) {
// In this test, we make sure that even if the CurrentTimeTicks() value is
// very close to 0, we still obtain memory information from the first call to
// MemoryInfo::Create. We cannot just subtract CurrentTimeTicks() here
// because many places have DCHECKs for !time.is_null(), which would be hit if
// we set the clock to be exactly 0.
AdvanceClock(-CurrentTimeTicksInSeconds() + 0.0001);
V8TestingScope scope;
v8::Isolate* isolate = scope.GetIsolate();
std::vector<v8::Local<v8::ArrayBuffer>> objects;
objects.push_back(v8::ArrayBuffer::New(isolate, 100));
MemoryInfo* precise_memory =
MemoryInfo::Create(MemoryInfo::Precision::Precise);
CheckValues(precise_memory, MemoryInfo::Precision::Precise);
EXPECT_LT(0u, precise_memory->usedJSHeapSize());
EXPECT_LT(0u, precise_memory->totalJSHeapSize());
EXPECT_LT(0u, precise_memory->jsHeapSizeLimit());
}
} // namespace blink } // namespace blink
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