New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 834832 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Apr 2018
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

Make SamplingHeapProfilerRateAgnosticEstimates more robust

Project Member Reported by u...@chromium.org, Apr 19 2018

Issue description

The test started failing after incremental marking step size heuristic change:
https://chromium-review.googlesource.com/1017123

Test: cctest/test-heap-profiler/SamplingHeapProfilerRateAgnosticEstimates (flaky in a repeated run)
Flags: test-heap-profiler/SamplingHeapProfilerRateAgnosticEstimates --random-seed=1571735677 --stress-incremental-marking --nohard-abort --enable-slow-asserts --verify-heap
Command: out/Debug/cctest test-heap-profiler/SamplingHeapProfilerRateAgnosticEstimates --random-seed=1571735677 --stress-incremental-marking --nohard-abort --enable-slow-asserts --verify-heap
Build environment:
 gn_args: is_component_build = true is_debug = true target_cpu = "x64" use_goma = true v8_enable_backtrace = true v8_enable_slow_dchecks = true
Run #1
Exit code: -6
Result: FAIL
Expected outcomes: PASS
Duration: 00:02:166
Stderr:
#
# Fatal error in ../../test/cctest/test-heap-profiler.cc, line 3358
# Check failed: percent_difference < 0.15 (0.159683 vs. 0.15).
#
#
#
#FailureMessage Object: 0x7ffd1cc9e090

 
Project Member

Comment 1 by bugdroid1@chromium.org, Apr 19 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/187c1e2ac12ed9d906b2394651097d04174a38ca

commit 187c1e2ac12ed9d906b2394651097d04174a38ca
Author: Ulan Degenbaev <ulan@chromium.org>
Date: Thu Apr 19 16:44:54 2018

Temporarily increase the threshold in sampling heap profiler test.

This allows percent difference of up to 0.2 in
SamplingHeapProfilerRateAgnosticEstimates.

Bug:  chromium:834832 
Tbr: ofrobots@chromium.org
No-Tree-Checks: true
No-Try: true
Change-Id: I2f38ac886700eed31840dc19d65103b84d155592
Reviewed-on: https://chromium-review.googlesource.com/1019781
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52697}
[modify] https://crrev.com/187c1e2ac12ed9d906b2394651097d04174a38ca/test/cctest/test-heap-profiler.cc

Comment 2 by u...@chromium.org, Apr 20 2018

The flakiness is caused by inlining of function bar into function foo.
All allocations in function bar that happen after inlining are not accounted.


Allocation counts for sampling interval 1024:
[(root)]: 0
....[]: 0
........[foo]: 71
............[bar]: 1024
....[(V8 API)]: 1
....[(EXTERNAL)]: 17

Allocation counts for sampling interval 128:
[(root)]: 0
....[]: 4
........[foo]: 130
............[bar]: 919
....[(V8 API)]: 1
....[(EXTERNAL)]: 1


We should either disable inlining or use the sum of allocations in foo and bar.

I think summing up is better.
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 20 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/6b129066a9a765471051f0d4896b02d8a942875b

commit 6b129066a9a765471051f0d4896b02d8a942875b
Author: Ulan Degenbaev <ulan@chromium.org>
Date: Fri Apr 20 13:56:14 2018

[test] Make SamplingHeapProfilerRateAgnosticEstimates more robust.

The function allocating objects in the test can be inlined in the middle
of the run. All allocations after inlining are currently not accounted.
This patch sums up allocations of the function and its outer function.

The difference between counts is now about 4%-6% (down from 15%).

Bug:  chromium:834832 
Change-Id: Iad071bd5bf53bb3527c9cb24d0a9ea38618c833c
Reviewed-on: https://chromium-review.googlesource.com/1021734
Reviewed-by: Ali Ijaz Sheikh <ofrobots@google.com>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#52713}
[modify] https://crrev.com/6b129066a9a765471051f0d4896b02d8a942875b/test/cctest/test-heap-profiler.cc

Comment 4 by u...@chromium.org, Apr 20 2018

Status: Fixed (was: Assigned)

Sign in to add a comment