Issue metadata
Sign in to add a comment
|
3.8% regression in thread_times.simple_mobile_sites at 392525:392551 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
May 11 2016
=== Auto-CCing suspected CL author ihf@chromium.org === Hi ihf@chromium.org, the bisect results pointed to your CL below as possibly causing a regression. Please have a look at this info and see whether your CL be related. ===== BISECT JOB RESULTS ===== Status: completed ===== SUSPECTED CL(s) ===== Subject : Revert of [Reland 1] Pepper takes ownership of a mailbox before passing it to the texture layer. (patchset #8 id:140001 of https://codereview.chromium.org/1943513002/ ) Author : ihf Commit description: Reason for revert: Breaks WebGL on CrOS, crbug.com/610411 . Original issue's description: > [Reland 1] Pepper takes ownership of a mailbox before passing it to the texture layer. > > This CL makes three changes from the original. > > 1. Replace a call to std::remove_if() with vec.erase(std::remove_if(), ...). > This was a logic error in the original CL that caused a crash any time the size > of the buffer was changed. This CL also adds a test that catches this bug. > > 2. Add some simple reference counting to PepperPluginInstanceImpl to track the > fact that a cc::TextureMailbox may be passed to |texture_layer_| more than once. > > 3. The SyncToken signal is now processed in the context of its own message: > WaitSyncToken. > > > I replaced the IPC message GpuCommandBufferMsg_ProduceFrontBuffer with > > GpuCommandBufferMsg_TakeFrontBuffer and GpuCommandBufferMsg_ReturnFrontBuffer. > > TakeFrontBuffer gives ownership of the front buffer to the client. When the > > client returns it with ReturnFrontBuffer, the command buffer may choose to reuse > > it. > > > > This means that pepper no longer needs to use > > SetTextureMailboxWithoutReleaseCallback. > > BUG= 350204 , 602484 > CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel > TEST=Run Chromium with the Pepper Flash plugin. Visit a site that supports Flash > video, such as http://vudu.com. Start playing a video, and then fullscreen the > video. Observe that Chromium does not crash. Please extensively test Chromium on > Flash 3D games and Flash video and make sure nothing else is working > incorrectly. > TBR=ccameron@chromium.org, bbudge@chromium.org, sky@chromium.org > > Committed: https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99 > Cr-Commit-Position: refs/heads/master@{#391686} TBR=piman@chromium.org,ccameron@chromium.org,bbudge@chromium.org,tsepez@chromium.org,sky@chromium.org,sunnyps@chromium.org,erikchen@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 350204 , 602484 Review-Url: https://codereview.chromium.org/1964793002 Cr-Commit-Position: refs/heads/master@{#392548} Commit : 8c46143b0ca822b010d5f9c284eadc810b0678e8 Date : Tue May 10 05:15:31 2016 ===== TESTED REVISIONS ===== Revision Mean Std Dev N Good? chromium@392524 2.21975 0.100043 5 good chromium@392538 2.16495 0.0174441 4 good chromium@392545 2.20971 0.108635 5 good chromium@392547 2.17411 0.0255259 4 good chromium@392548 2.60741 0.0252424 5 bad <-- chromium@392551 2.61731 0.00567761 5 bad Bisect job ran on: android_nexus5_perf_bisect Bug ID: 611063 Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests thread_times.simple_mobile_sites Test Metric: thread_GPU_cpu_time_per_frame/http___www.nyc.gov Relative Change: 16.00% Score: 99.8 Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5_perf_bisect/builds/3686 Job details: https://chromeperf.appspot.com/buildbucket_job_status/9012945282587496464 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=5788757165342720 | O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq | X | for more information addressing perf regression bugs. For feedback, | / \ | file a bug with component Tests>AutoBisect. Thank you!
,
May 11 2016
This reverts an incorrect change which improved performance.
,
May 11 2016
===== BISECT JOB RESULTS ===== Status: completed ===== SUSPECTED CL(s) ===== Subject : Revert of [Reland 1] Pepper takes ownership of a mailbox before passing it to the texture layer. (patchset #8 id:140001 of https://codereview.chromium.org/1943513002/ ) Author : ihf Commit description: Reason for revert: Breaks WebGL on CrOS, crbug.com/610411 . Original issue's description: > [Reland 1] Pepper takes ownership of a mailbox before passing it to the texture layer. > > This CL makes three changes from the original. > > 1. Replace a call to std::remove_if() with vec.erase(std::remove_if(), ...). > This was a logic error in the original CL that caused a crash any time the size > of the buffer was changed. This CL also adds a test that catches this bug. > > 2. Add some simple reference counting to PepperPluginInstanceImpl to track the > fact that a cc::TextureMailbox may be passed to |texture_layer_| more than once. > > 3. The SyncToken signal is now processed in the context of its own message: > WaitSyncToken. > > > I replaced the IPC message GpuCommandBufferMsg_ProduceFrontBuffer with > > GpuCommandBufferMsg_TakeFrontBuffer and GpuCommandBufferMsg_ReturnFrontBuffer. > > TakeFrontBuffer gives ownership of the front buffer to the client. When the > > client returns it with ReturnFrontBuffer, the command buffer may choose to reuse > > it. > > > > This means that pepper no longer needs to use > > SetTextureMailboxWithoutReleaseCallback. > > BUG= 350204 , 602484 > CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.win:win_optional_gpu_tests_rel > TEST=Run Chromium with the Pepper Flash plugin. Visit a site that supports Flash > video, such as http://vudu.com. Start playing a video, and then fullscreen the > video. Observe that Chromium does not crash. Please extensively test Chromium on > Flash 3D games and Flash video and make sure nothing else is working > incorrectly. > TBR=ccameron@chromium.org, bbudge@chromium.org, sky@chromium.org > > Committed: https://crrev.com/4734ed1b6740b772201b8accb3e2bd88cdea5c99 > Cr-Commit-Position: refs/heads/master@{#391686} TBR=piman@chromium.org,ccameron@chromium.org,bbudge@chromium.org,tsepez@chromium.org,sky@chromium.org,sunnyps@chromium.org,erikchen@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 350204 , 602484 Review-Url: https://codereview.chromium.org/1964793002 Cr-Commit-Position: refs/heads/master@{#392548} Commit : 8c46143b0ca822b010d5f9c284eadc810b0678e8 Date : Tue May 10 05:15:31 2016 ===== TESTED REVISIONS ===== Revision Mean Std Dev N Good? chromium@392524 2.79633 0.310244 15 good chromium@392538 2.87288 0.139815 8 good chromium@392545 2.53249 0.394646 5 good chromium@392547 2.66743 0.382223 5 good chromium@392548 3.01736 0.0178804 5 bad <-- chromium@392551 2.91253 0.248429 18 bad Bisect job ran on: android_nexus5_perf_bisect Bug ID: 611063 Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests thread_times.simple_mobile_sites Test Metric: thread_GPU_cpu_time_per_frame/thread_GPU_cpu_time_per_frame Relative Change: 1.32% Score: 90.0 Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_nexus5_perf_bisect/builds/3685 Job details: https://chromeperf.appspot.com/buildbucket_job_status/9012945295498456576 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=5790983300579328 | O O | Visit http://www.chromium.org/developers/speed-infra/perf-bug-faq | X | for more information addressing perf regression bugs. For feedback, | / \ | file a bug with component Tests>AutoBisect. Thank you! |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by tdres...@chromium.org
, May 11 2016