"org.chromium.chrome.browser.profiling_host.ProfilingProcessHostAndroidTest#testModeRendererPseudo" is flaky |
||||
Issue description"org.chromium.chrome.browser.profiling_host.ProfilingProcessHostAndroidTest#testModeRendererPseudo" is flaky. This issue was created automatically by the chromium-try-flakes app. Please find the right owner to fix the respective test/step and assign this issue to them. If the step/test is infrastructure-related, please add Infra-Troopers label and change issue status to Untriaged. When done, please remove the issue from Sheriff Bug Queue by removing the Sheriff-Chromium label. We have detected 3 recent flakes. List of all flakes can be found at https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNybAsSBUZsYWtlImFvcmcuY2hyb21pdW0uY2hyb21lLmJyb3dzZXIucHJvZmlsaW5nX2hvc3QuUHJvZmlsaW5nUHJvY2Vzc0hvc3RBbmRyb2lkVGVzdCN0ZXN0TW9kZVJlbmRlcmVyUHNldWRvDA. Flaky tests should be disabled within 30 minutes unless culprit CL is found and reverted. Please see more details here: https://sites.google.com/a/chromium.org/dev/developers/tree-sheriffs/sheriffing-bug-queues#triaging-auto-filed-flakiness-bugs This flaky test/step was previously tracked in issue 804412 .
,
Apr 17 2018
Thanks, I believe this should be fixed by: https://chromium-review.googlesource.com/c/chromium/src/+/1012542
,
Apr 18 2018
Still flaky: https://ci.chromium.org/buildbot/tryserver.chromium.android/linux_android_rel_ng/531485 https://test-results.appspot.com/dashboards/flakiness_dashboard.html#testType=chrome_public_test_apk&tests=org.chromium.chrome.browser.profiling_host.ProfilingProcessHostAndroidTest%23testModeRendererPseudo
,
Apr 18 2018
I'll disable the test while I investigate.
,
Apr 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/419cce880a15bdcec9035da068dc2554a6877e01 commit 419cce880a15bdcec9035da068dc2554a6877e01 Author: Erik Chen <erikchen@chromium.org> Date: Wed Apr 18 20:34:46 2018 Disable ProfilingProcessHostAndroidTest#testModeRendererPseudo The test is flaky, likely due to recent refactors to the heap profiling code to support Android Webview. I'm disabling it temporarily while I investigate. Bug: 833590 Change-Id: Idd7dd982d9e5586f76f991455d1ae717323c519d TBR: thakis@chromium.org Reviewed-on: https://chromium-review.googlesource.com/1017598 Reviewed-by: Erik Chen <erikchen@chromium.org> Commit-Queue: Erik Chen <erikchen@chromium.org> Cr-Commit-Position: refs/heads/master@{#551808} [modify] https://crrev.com/419cce880a15bdcec9035da068dc2554a6877e01/chrome/android/javatests/src/org/chromium/chrome/browser/profiling_host/ProfilingProcessHostAndroidTest.java
,
Apr 19 2018
,
Apr 19 2018
Issue 833998 has been merged into this issue.
,
Apr 19 2018
On timeouts, there are no logcats. That makes this issue very hard to debug. I'll filed a feature request: https://bugs.chromium.org/p/chromium/issues/detail?id=834903
,
Apr 19 2018
This issue started with: https://chromium-review.googlesource.com/c/chromium/src/+/1006096 That suggests that this removed code is responsible: """ // On Android, there is a warm-up renderer that is sometimes started // before the ProfilingProcessHost can be started. This renderer will // therefore not be profiled, even if Chrome is started with the // --memlog=all-renderers switch. content::BrowserThread::PostTask( content::BrowserThread::UI, FROM_HERE, base::Bind(&ProfilingProcessHost::StartProfilingRenderersForTesting, base::Unretained(ProfilingProcessHost::GetInstance()))); ... void ProfilingProcessHost::StartProfilingRenderersForTesting() { DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); for (auto iter = content::RenderProcessHost::AllHostsIterator(); !iter.IsAtEnd(); iter.Advance()) { StartProfilingRenderer(iter.GetCurrentValue()); } } """ The current code does this: """ // Start profiling connected renderers. for (auto iter = content::RenderProcessHost::AllHostsIterator(); !iter.IsAtEnd(); iter.Advance()) { if (ShouldProfileNewRenderer(iter.GetCurrentValue()) && iter.GetCurrentValue()->GetProcess().Handle() != base::kNullProcessHandle) { StartProfilingRenderer(iter.GetCurrentValue()); } } """ I wonder if the problem is that the logic which waits for renderers to start up is wrong: """ 65 bool RenderersAreBeingProfiled( 66 const std::vector<base::ProcessId>& profiled_pids) { 67 DCHECK(content::BrowserThread::CurrentlyOn(content::BrowserThread::UI)); 68 size_t renderer_count = 0; 69 for (auto iter = content::RenderProcessHost::AllHostsIterator(); 70 !iter.IsAtEnd(); iter.Advance()) { 71 base::ProcessId pid = iter.GetCurrentValue()->GetProcess().Pid(); 72 if (std::find(profiled_pids.begin(), profiled_pids.end(), pid) == 73 profiled_pids.end()) { 74 return false; 75 } 76 ++renderer_count; 77 } 78 79 return renderer_count != 0; 80 } """ This assumes taht RPHIs are initialized. If there is an RPHI that stays uninitialized... [somehow??], then the test will stall.
,
Apr 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c32009dc704fa611bcc69906f23f1be0d7981e63 commit c32009dc704fa611bcc69906f23f1be0d7981e63 Author: Erik Chen <erikchen@chromium.org> Date: Thu Apr 19 20:51:44 2018 Tentative fix ProfilingProcessHostAndroidTest#testModeRendererPseudo. The test started flaking recently due to some refactoring I landed, specifically: https://chromium-review.googlesource.com/c/chromium/src/+/1006096. The only theory I have for how this could happen is if there's a RenderProcessHostImpl that stays stalled in the uninitialized state. This CL updates the waiting logic in the test to match the logic being tested - to only look at initialized RenderProcessHostImpls. Bug: 833590 Change-Id: Ia0d49f5c569625b3ccfc01c9dd17c8e7659f105d Reviewed-on: https://chromium-review.googlesource.com/1020071 Reviewed-by: Nico Weber <thakis@chromium.org> Commit-Queue: Erik Chen <erikchen@chromium.org> Cr-Commit-Position: refs/heads/master@{#552150} [modify] https://crrev.com/c32009dc704fa611bcc69906f23f1be0d7981e63/chrome/android/javatests/src/org/chromium/chrome/browser/profiling_host/ProfilingProcessHostAndroidTest.java [modify] https://crrev.com/c32009dc704fa611bcc69906f23f1be0d7981e63/components/heap_profiling/test_driver.cc
,
Apr 20 2018
Detected 4 new flakes for test/step "org.chromium.chrome.browser.profiling_host.ProfilingProcessHostAndroidTest#testModeRendererPseudo". To see the actual flakes, please visit https://chromium-try-flakes.appspot.com/all_flake_occurrences?key=ahVzfmNocm9taXVtLXRyeS1mbGFrZXNybAsSBUZsYWtlImFvcmcuY2hyb21pdW0uY2hyb21lLmJyb3dzZXIucHJvZmlsaW5nX2hvc3QuUHJvZmlsaW5nUHJvY2Vzc0hvc3RBbmRyb2lkVGVzdCN0ZXN0TW9kZVJlbmRlcmVyUHNldWRvDA. This message was posted automatically by the chromium-try-flakes app. Since flakiness is ongoing, the issue was moved back into Sheriff Bug Queue (unless already there).
,
Apr 20 2018
Bah. Without logcats and without a local repro, I have no idea what could be causing the flakes. I guess I will disable for now.
,
Apr 20 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/64bb35e6954cc3889005909cfd47d20c7dbf70ea commit 64bb35e6954cc3889005909cfd47d20c7dbf70ea Author: Erik Chen <erikchen@chromium.org> Date: Fri Apr 20 20:05:59 2018 Disable flaky test testModeRendererPseudo. Bug: 833590 Change-Id: Idde436685e6f5400271c2b850c2740c9ec5c9fb0 TBR: thakis@chromium.org Reviewed-on: https://chromium-review.googlesource.com/1022394 Reviewed-by: Erik Chen <erikchen@chromium.org> Commit-Queue: Erik Chen <erikchen@chromium.org> Cr-Commit-Position: refs/heads/master@{#552442} [modify] https://crrev.com/64bb35e6954cc3889005909cfd47d20c7dbf70ea/chrome/android/javatests/src/org/chromium/chrome/browser/profiling_host/ProfilingProcessHostAndroidTest.java
,
Apr 20 2018
,
Apr 23 2018
Ah, there was another CL that landed in the same time period which I suspect might be related: https://chromium-review.googlesource.com/963405 There's something odd going on with spare renderers and OOP HP.
,
Apr 24 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7bffeb9aa927652e7b8e2194540c751d4029cf29 commit 7bffeb9aa927652e7b8e2194540c751d4029cf29 Author: Erik Chen <erikchen@chromium.org> Date: Tue Apr 24 16:56:51 2018 Loosen restriction on profiled renderers for memlog e2e tests. Previously, we checked that at least 1 renderer exists and that all renderers are being profiled. This recently started causing the tests to flake. I suspect this is related to warm-up/spare renderers. So instead, we check that at least 1 renderer exists and is being profiled. Bug: 835826 , 833590 Change-Id: I45ef8e1ad9ba1689ae3f0292ab80306b9d7a731f Reviewed-on: https://chromium-review.googlesource.com/1023716 Reviewed-by: Nico Weber <thakis@chromium.org> Commit-Queue: Erik Chen <erikchen@chromium.org> Cr-Commit-Position: refs/heads/master@{#553164} [modify] https://crrev.com/7bffeb9aa927652e7b8e2194540c751d4029cf29/components/heap_profiling/test_driver.cc |
||||
►
Sign in to add a comment |
||||
Comment 1 by moh...@chromium.org
, Apr 17 2018Status: Assigned (was: Untriaged)