Issue metadata
Sign in to add a comment
|
50% regression in media.tough_video_cases_tbmv2 at 488673:488772 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Jul 24 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8973169294722494848
,
Jul 24 2017
=== Auto-CCing suspected CL author catherinechung@google.com === Hi catherinechung@google.com, the bisect results pointed to your CL, please take a look at the results. === BISECT JOB RESULTS === Perf regression found with culprit Suspected Commit Author : Catherine Chung Commit : de9437e08630e7949592e4c37d56105252698c95 Date : Fri Jul 21 22:10:51 2017 Subject: Notify the NewTabFeatureEngagementTracker of tab creation and omnibox navigations Bisect Details Configuration: winx64nvidia_perf_bisect Benchmark : media.tough_video_cases_tbmv2 Metric : memory:chrome:browser_process:reported_by_chrome:leveldb:effective_size_avg/video.html?src_smpte_3840x2160_60fps_vp9.webm_seek Change : 50.00% | 8208.0 -> 12312.0 Revision Result N chromium@488672 8208.0 +- 0.0 6 good chromium@488722 8208.0 +- 0.0 6 good chromium@488747 8208.0 +- 0.0 6 good chromium@488760 8208.0 +- 0.0 6 good chromium@488766 8208.0 +- 0.0 6 good chromium@488769 8208.0 +- 0.0 6 good chromium@488770 8208.0 +- 0.0 6 good chromium@488771 12312.0 +- 0.0 6 bad <-- chromium@488772 12312.0 +- 0.0 6 bad To Run This Test src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=video.html.src.smpte.3840x2160.60fps.vp9.webm.seek media.tough_video_cases_tbmv2 More information on addressing performance regressions: http://g.co/ChromePerformanceRegressions Debug information about this bisect: https://chromeperf.appspot.com/buildbucket_job_status/8973169294722494848 For feedback, file a bug with component Speed>Bisection
,
Jul 24 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8973163338397007616
,
Jul 24 2017
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/8973160460256704192
,
Jul 24 2017
=== BISECT JOB RESULTS === Perf regression found with culprit Suspected Commit Author : Catherine Chung Commit : de9437e08630e7949592e4c37d56105252698c95 Date : Fri Jul 21 22:10:51 2017 Subject: Notify the NewTabFeatureEngagementTracker of tab creation and omnibox navigations Bisect Details Configuration: winx64nvidia_perf_bisect Benchmark : media.tough_video_cases_tbmv2 Metric : memory:chrome:browser_process:reported_by_chrome:leveldb:effective_size_avg/video.html?src_smpte_3840x2160_60fps_vp9.webm_seek Change : 50.00% | 8208.0 -> 12312.0 Revision Result N chromium@488672 8208.0 +- 0.0 6 good chromium@488722 8208.0 +- 0.0 6 good chromium@488747 8208.0 +- 0.0 6 good chromium@488760 8208.0 +- 0.0 6 good chromium@488766 8208.0 +- 0.0 6 good chromium@488769 8208.0 +- 0.0 6 good chromium@488770 8208.0 +- 0.0 6 good chromium@488771 12312.0 +- 0.0 6 bad <-- chromium@488772 12312.0 +- 0.0 6 bad To Run This Test src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=video.html.src.smpte.3840x2160.60fps.vp9.webm.seek media.tough_video_cases_tbmv2 More information on addressing performance regressions: http://g.co/ChromePerformanceRegressions Debug information about this bisect: https://chromeperf.appspot.com/buildbucket_job_status/8973163338397007616 For feedback, file a bug with component Speed>Bisection
,
Jul 24 2017
=== BISECT JOB RESULTS === Perf regression found with culprit Suspected Commit Author : Catherine Chung Commit : de9437e08630e7949592e4c37d56105252698c95 Date : Fri Jul 21 22:10:51 2017 Subject: Notify the NewTabFeatureEngagementTracker of tab creation and omnibox navigations Bisect Details Configuration: winx64nvidia_perf_bisect Benchmark : media.tough_video_cases_tbmv2 Metric : memory:chrome:browser_process:reported_by_chrome:leveldb:effective_size_avg/video.html?src_smpte_3840x2160_60fps_vp9.webm_seek Change : 50.00% | 8208.0 -> 12312.0 Revision Result N chromium@488672 8208.0 +- 0.0 6 good chromium@488722 8208.0 +- 0.0 6 good chromium@488747 8208.0 +- 0.0 6 good chromium@488760 8208.0 +- 0.0 6 good chromium@488766 8208.0 +- 0.0 6 good chromium@488769 8208.0 +- 0.0 6 good chromium@488770 8208.0 +- 0.0 6 good chromium@488771 12312.0 +- 0.0 6 bad <-- chromium@488772 12312.0 +- 0.0 6 bad To Run This Test src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --pageset-repeat=1 --also-run-disabled-tests --story-filter=video.html.src.smpte.3840x2160.60fps.vp9.webm.seek media.tough_video_cases_tbmv2 More information on addressing performance regressions: http://g.co/ChromePerformanceRegressions Debug information about this bisect: https://chromeperf.appspot.com/buildbucket_job_status/8973160460256704192 For feedback, file a bug with component Speed>Bisection
,
Jul 24 2017
The bisect is for this graph: https://chromeperf.appspot.com/report?sid=ef4ab64c90c2017deef482d761c68d8e76cf9c2bddd21d6d544020806531d48b&rev=488772 Please don't start any more bisects. They use a lot of bot time to run, and we already have a clear culprit.
,
Jul 24 2017
I believe that this regression is justified. The CL in question is a part of adding a "feature engagement tracker" for the New Tab. Every time the tracker "notifies an event" that notification is recorded in LevelDB. Hence, the LevelDB is expected to increase.
,
Jul 24 2017
For now, I'll revert the commit and see if it's possible to reduce the memory increase.
,
Jul 25 2017
,
Jul 25 2017
The revert Cl can be found here: https://chromium-review.googlesource.com/c/584033
,
Jul 28 2017
The original CL (#489638) is now logging to a lazily opened database where there was no logging before. If I'm reading the Telemetry JSON correctly leveldb:effective_size_avg went up an average of 4104 bytes. Not sure if that's per Chrome process, or ~4K total. This sounds reasonable for opening a leveldb.
,
Jul 31 2017
Since the component feature_engagement::Tracker is used in this CL, it uses a LevelDB. The database is created in feature_engagement::Create() and then passed into PersistentEventStore. The CL moves up the creation of a LevelDB instance (which is the class that wraps the leveldb library). It isn't a new 4MB leak; I'm just using it earlier on (ie. opening a new tab, starting a new session with the omnibox). Because of this justification, it is reasonable to re-upload the same CL.
,
Aug 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/beb884227d7b8759c461fc6ab43effb8ad479e2e commit beb884227d7b8759c461fc6ab43effb8ad479e2e Author: Catherine Chung <catherinechung@google.com> Date: Tue Aug 01 15:47:55 2017 Notify the NewTabTracker of new tab and session creations This CL uses the feature_engagement:NewTabTracker, which you can view in this CL: http://crrev.com/7d54c1b52c11ef28e9a19f3c6a5a7a41a25fbf5d The feature_engagement::NewTabTracker will be notified if: * The user presses the New Tab button * The user presses Control+T * The user uses the tabstrip context menu, window frame context menu, or the app menu to open a New Tab. * The user uses the omnibox to start a session in the same tab. * The user focuses on the omnibox. Original CL: http://crrev.com/de9437e08630e7949592e4c37d56105252698c95 Revert CL: http://crrev.com/c6bf5c334be6a671eb6a9e37d031b1f48d46ca88 This CL re-lands the original CL. NOTE: The original CL was initially reverted due to an increase in memory usage, but was later resolved as a justifiable increase. There were no changes made during this revert and the bug 748082 explains why in more detail. TBR= pkasting, justincohen Bug: 734132, 748082 Change-Id: Id4909b567f563dd0b3afe425705079e1598ee53f Reviewed-on: https://chromium-review.googlesource.com/549074 Reviewed-by: Justin Cohen <justincohen@chromium.org> Reviewed-by: Robert Liao <robliao@chromium.org> Reviewed-by: Peter Kasting <pkasting@chromium.org> Reviewed-by: Erik Chen <erikchen@chromium.org> Reviewed-by: Dominick Ng <dominickn@chromium.org> Commit-Queue: Catherine Chung <catherinechung@google.com> Cr-Original-Commit-Position: refs/heads/master@{#488771} Reviewed-on: https://chromium-review.googlesource.com/594548 Cr-Commit-Position: refs/heads/master@{#491000} [modify] https://crrev.com/beb884227d7b8759c461fc6ab43effb8ad479e2e/chrome/browser/ui/browser_command_controller.cc [modify] https://crrev.com/beb884227d7b8759c461fc6ab43effb8ad479e2e/chrome/browser/ui/omnibox/chrome_omnibox_client.cc [modify] https://crrev.com/beb884227d7b8759c461fc6ab43effb8ad479e2e/chrome/browser/ui/omnibox/chrome_omnibox_client.h [modify] https://crrev.com/beb884227d7b8759c461fc6ab43effb8ad479e2e/chrome/browser/ui/tabs/tab_strip_model.cc [modify] https://crrev.com/beb884227d7b8759c461fc6ab43effb8ad479e2e/chrome/browser/ui/views/omnibox/omnibox_view_views.cc [modify] https://crrev.com/beb884227d7b8759c461fc6ab43effb8ad479e2e/chrome/browser/ui/views/omnibox/omnibox_view_views_unittest.cc [modify] https://crrev.com/beb884227d7b8759c461fc6ab43effb8ad479e2e/chrome/browser/ui/views/tabs/browser_tab_strip_controller.cc [modify] https://crrev.com/beb884227d7b8759c461fc6ab43effb8ad479e2e/components/omnibox/browser/omnibox_client.cc [modify] https://crrev.com/beb884227d7b8759c461fc6ab43effb8ad479e2e/components/omnibox/browser/omnibox_client.h [modify] https://crrev.com/beb884227d7b8759c461fc6ab43effb8ad479e2e/components/omnibox/browser/omnibox_edit_model.cc [modify] https://crrev.com/beb884227d7b8759c461fc6ab43effb8ad479e2e/ios/chrome/browser/ui/omnibox/chrome_omnibox_client_ios.h [modify] https://crrev.com/beb884227d7b8759c461fc6ab43effb8ad479e2e/ios/chrome/browser/ui/omnibox/chrome_omnibox_client_ios.mm
,
Oct 19 2017
This is actually WontFix I think since nothing was fixed or changed. |
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Jul 24 2017