Issue metadata
Sign in to add a comment
|
1.8% regression in system_health.memory_mobile at 405859:405886 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Jul 18 2016
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/9006784932846048560
,
Jul 19 2016
=== Auto-CCing suspected CL author nyquist@chromium.org === Hi nyquist@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 : Add BlimpClientContext and factory with real and dummy implementation Author : nyquist Commit description: The BlimpClientContext is the core class which wires up all of the blimp functionality. When it's created, it will get all its dependencies from the embedder. It is currently in charge of creating BlimpContents, but will also own things like the BlimpClientSession. It is part of the public //blimp/client/public API. It is a KeyedService and is typically tied to a Profile. The BlimpClientContextFactory lives in //chrome to be able to access Profile during construction, since //blimp/client currently does not depend on //content where BrowserContext lives, and there are upcoming dependencies that will be implemented by classes in //chrome. The BlimpClientContext has two implementations in //blimp/client/core, an actual one and a dummy implementation. The dummy implementation is to be used when we do not want the blimp code to be used, and the core implementation is the real blimp implementation. Which one of these are in use is controlled by the GN argument enable_blimp which defaults to false. The BlimpClientContext has a public static create-method that is not implemented in //blimp/client/public, but instead it is implemented by //blimp/client/core by both the actual and the dummy implementation, so the right method will be chosen at link-time. This means that a target can only link with either the actual or dummy implementation. Everything in //blimp/client/core is only visible to //blimp/client/* and embedders should only ever depend on //blimp/client/public, which will bring in the correct implementation. This works the same for both C++ and Java, so when using the BlimpClientContextFactory (either in Java or C++), the right implementation (actual or dummy) will always be chosen correctly based on the link-time selection of the Create-method. The BlimpClientContext has a delegate that it can call out to whenever it needs specific functionality from the embedder. As an example, this CL adds a helper for attaching a Profile to every created BlimpContents to make have similar functionality as what is built into WebContents so one can get a Profile and thereby services by only having access to a BlimpContents. BUG= 611094 TBR=erg@chromium.org Review-Url: https://codereview.chromium.org/2132163002 Cr-Commit-Position: refs/heads/master@{#405860} Commit : 9aa0cdee23229ca95a3bea79abcc584a395d016e Date : Fri Jul 15 21:21:47 2016 ===== TESTED REVISIONS ===== Revision Mean Std Dev N Good? chromium@405858 905728 4612.06 8 good chromium@405859 907776 5334.92 8 good chromium@405860 916685 3426.96 5 bad <-- chromium@405862 914227 4486.94 5 bad chromium@405865 913920 1448.15 8 bad chromium@405872 918323 4486.94 5 bad chromium@405886 917504 4096.0 5 bad Bisect job ran on: android_one_perf_bisect Bug ID: 629114 Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests system_health.memory_mobile Test Metric: load_news-memory:chrome:all_processes:reported_by_os:system_memory:ashmem:private_dirty_size_avg/load_news_hackernews Relative Change: 1.27% Score: 99.9 Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_one_perf_bisect/builds/1421 Job details: https://chromeperf.appspot.com/buildbucket_job_status/9006784932846048560 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=5892560732553216 | 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!
,
Jul 19 2016
,
Aug 18 2016
Perf sheriff ping: reminder to follow up on possible performance issues
,
Aug 24 2016
Could this be something else or are the perf numbers very clear here?
,
Aug 24 2016
From looking at the attached charts, the numbers seem to go down again right after.
,
Aug 24 2016
Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/9003451752307157488
,
Aug 24 2016
The numbers are quite clear (#3): ===== TESTED REVISIONS ===== Revision Mean Std Dev N Good? chromium@405858 905728 4612.06 8 good chromium@405859 907776 5334.92 8 good chromium@405860 916685 3426.96 5 bad <-- chromium@405862 914227 4486.94 5 bad chromium@405865 913920 1448.15 8 bad chromium@405872 918323 4486.94 5 bad chromium@405886 917504 4096.0 5 bad but I'm running another bisect to confirm it. Note that the chart might have gone down for a completely unrelated reason, so this should be investigated (assuming the second bisect confirms the suspicion).
,
Aug 25 2016
=== Auto-CCing suspected CL author nyquist@chromium.org === Hi nyquist@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 : Add BlimpClientContext and factory with real and dummy implementation Author : nyquist Commit description: The BlimpClientContext is the core class which wires up all of the blimp functionality. When it's created, it will get all its dependencies from the embedder. It is currently in charge of creating BlimpContents, but will also own things like the BlimpClientSession. It is part of the public //blimp/client/public API. It is a KeyedService and is typically tied to a Profile. The BlimpClientContextFactory lives in //chrome to be able to access Profile during construction, since //blimp/client currently does not depend on //content where BrowserContext lives, and there are upcoming dependencies that will be implemented by classes in //chrome. The BlimpClientContext has two implementations in //blimp/client/core, an actual one and a dummy implementation. The dummy implementation is to be used when we do not want the blimp code to be used, and the core implementation is the real blimp implementation. Which one of these are in use is controlled by the GN argument enable_blimp which defaults to false. The BlimpClientContext has a public static create-method that is not implemented in //blimp/client/public, but instead it is implemented by //blimp/client/core by both the actual and the dummy implementation, so the right method will be chosen at link-time. This means that a target can only link with either the actual or dummy implementation. Everything in //blimp/client/core is only visible to //blimp/client/* and embedders should only ever depend on //blimp/client/public, which will bring in the correct implementation. This works the same for both C++ and Java, so when using the BlimpClientContextFactory (either in Java or C++), the right implementation (actual or dummy) will always be chosen correctly based on the link-time selection of the Create-method. The BlimpClientContext has a delegate that it can call out to whenever it needs specific functionality from the embedder. As an example, this CL adds a helper for attaching a Profile to every created BlimpContents to make have similar functionality as what is built into WebContents so one can get a Profile and thereby services by only having access to a BlimpContents. BUG= 611094 TBR=erg@chromium.org Review-Url: https://codereview.chromium.org/2132163002 Cr-Commit-Position: refs/heads/master@{#405860} Commit : 9aa0cdee23229ca95a3bea79abcc584a395d016e Date : Fri Jul 15 21:21:47 2016 ===== TESTED REVISIONS ===== Revision Mean Std Dev N Good? chromium@405858 904704 6359.64 8 good chromium@405859 903578 3663.57 5 good chromium@405860 914432 3630.72 8 bad <-- chromium@405862 915456 3792.16 8 bad chromium@405865 914432 5250.01 8 bad chromium@405872 918528 1896.08 8 bad chromium@405886 914432 3630.72 8 bad Bisect job ran on: android_one_perf_bisect Bug ID: 629114 Test Command: src/tools/perf/run_benchmark -v --browser=android-chromium --output-format=chartjson --upload-results --also-run-disabled-tests system_health.memory_mobile Test Metric: load_news-memory:chrome:all_processes:reported_by_os:system_memory:ashmem:private_dirty_size_avg/load_news_hackernews Relative Change: 0.72% Score: 99.5 Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/android_one_perf_bisect/builds/1576 Job details: https://chromeperf.appspot.com/buildbucket_job_status/9003451752307157488 Not what you expected? We'll investigate and get back to you! https://chromeperf.appspot.com/bad_bisect?try_job_id=4555827835305984 | 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!
,
Aug 25 2016
nyquist: The second bisect also very clearly marked your patch as the culprit. Could you please investigate the issue?
,
Aug 27 2016
,
Aug 30 2016
,
Aug 30 2016
,
Oct 7 2016
I haven't gotten to the bottom of this yet. dtrainor: Could you see if you could get someone to investigate this while I'm gone?
,
Nov 18 2016
,
Nov 29 2016
,
Dec 9 2016
|
|||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||
Comment 1 by petrcermak@chromium.org
, Jul 18 2016