3.8%-4.2% regression in system_health.memory_desktop at 528427:528547 |
||||||||||||||||||||
Issue descriptionSee the link to graphs below.
,
Jan 12 2018
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/1185469f040000
,
Jan 12 2018
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/12d33fcf040000
,
Jan 12 2018
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/14e557ef040000
,
Jan 12 2018
📍 Found a significant difference after 1 commit. https://pinpoint-dot-chromeperf.appspot.com/job/14e557ef040000 Reland: Enable --harmony-function-tostring by default By jwolfe@igalia.com · Wed Jan 10 17:29:46 2018 v8 @ 6fe75e30aacff1f2e9b10a01e20bebd68fe56474 Understanding performance regressions: http://g.co/ChromePerformanceRegressions
,
Jan 12 2018
📍 Found a significant difference after 1 commit. https://pinpoint-dot-chromeperf.appspot.com/job/1185469f040000 Reland: Enable --harmony-function-tostring by default By jwolfe@igalia.com · Wed Jan 10 17:29:46 2018 v8 @ 6fe75e30aacff1f2e9b10a01e20bebd68fe56474 Understanding performance regressions: http://g.co/ChromePerformanceRegressions
,
Jan 16 2018
I'm having trouble reproducing this locally. That is, I don't know how to (I filed https://github.com/catapult-project/catapult/issues/4162 to point out that the new Pinpoint bot isn't documented on the linked "understanding performance regressions" page). After some investigating, I tried running: $ tools/perf/run_benchmark --browser-executable=out/rel/chrome --story-filter=load.news.qq system_health.memory_desktop but it didn't show me anything about render_processes in the results. I worry that that's because of this: (WARNING) 2018-01-16 14:14:12,517 memory_debug.LogHostMemoryUsage:44 psutil 1.2.1 too old, upgrade to version 2.0 or higher for memory usage information. but I don't see instructions about how to get a newer version of psutil onto my Ubuntu Trusty machine. jgruber, any chance you can point me (and jwolfe) in the right direction for reproducing this?
,
Jan 16 2018
+simonhatch, dtu in case they can help point me in the right direction.
,
Jan 17 2018
#7: that looks correct to me. I use '--browser=exact --browser-executable=out/release/chrome' locally, not sure if that makes a difference. I haven't seen that warning before. Normally, the result html file should contain everything we need, including numbers for v8::heap:allocated_objects_size_avg.
,
Jan 17 2018
+perezju benchmark owner for ideas
,
Jan 17 2018
Updated psutil via pip, and still no v8 metrics.
,
Jan 17 2018
Assigning to perezju in the hope of getting instructions for getting these metrics out of this benchmark. Pinpoint folks: the lack of reproduceability on these bug comments seems like a major problem to me.
,
Jan 18 2018
psutil should not be the problem here, you can safely ignore that warning. Can you share the results.html file you obtained while running locally? Here there is some more information specific about memory benchmarks (also linked to from the doc on performance regressions): https://chromium.googlesource.com/chromium/src/+/master/docs/memory-infra/memory_benchmarks.md Regarding pinpoint, I think it did a spot on job here. All three jobs found the same culprit with little or no noise. Or did you mean lack of "steps on how to reproduce locally"?
,
Jan 18 2018
Attached results.html. Note that the metric in the pinpoint results, "memory:chrome:renderer_processes:reported_by_chrome:v8:heap:allocated_objects_size", isn't present in my results. My comment on pinpoint was indeed about the "steps to reproduce locally". Given that this change wasn't expected to significantly affect memory use, some debugging is required to understand the difference, which requires local reproducability (at least for reasonable turnaround times).
,
Jan 18 2018
Maybe it would be useful for Pinpoint to provide some documentation on running locally, like the old bisect did?
,
Jan 19 2018
I'd recommend using Pinpoint's "test a patch" button to reproduce, which has a few advantages over running locally. * Guaranteed to be the same command-line as the bots. * Same hardware, os configs, and background processes as the perf waterfall. * Repeats the test multiple times over multiple devices. Sometimes the result doesn't reproduce on a device or is noisy enough that it requires multiple runs.
,
Jan 19 2018
Re #14: Values are there, but you need to tick the "Show all" checkbox (see screenshot) +eakuefner maybe there could be some UI tweak to make this easier to discover (e.g. when trying to search for a metric that is present but is currently hidden)? I'll also update our memory_benchmark docs to explicitly point this out.
,
Jan 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/33b0d14d0f6ef4e0cef5566438bfb0807a9f0c6f commit 33b0d14d0f6ef4e0cef5566438bfb0807a9f0c6f Author: Juan Antonio Navarro Perez <perezju@google.com> Date: Fri Jan 19 14:54:35 2018 [Docs] Memory benchmarks: note "show all" metrics in results.html Bug: 801556 Change-Id: I1bfce60f07e7a74eefebcbc262f1a06250fd54ee Reviewed-on: https://chromium-review.googlesource.com/876083 Commit-Queue: Primiano Tucci <primiano@chromium.org> Reviewed-by: Primiano Tucci <primiano@chromium.org> Cr-Commit-Position: refs/heads/master@{#530508} [modify] https://crrev.com/33b0d14d0f6ef4e0cef5566438bfb0807a9f0c6f/docs/memory-infra/memory_benchmarks.md
,
Jan 19 2018
@perezju, thanks for the pointer and the UI update. @dtu, where is the "test a patch" button? I searched for test a patch on the linked pages, and on the documentation page linked from the "?" on the Pinpoint landing page, and couldn't find anything. If it's on the "Analyze benchmark results" page, that page isn't working for these alerts (see https://github.com/catapult-project/catapult/issues/4168).
,
Jan 19 2018
Some raw data from local runs: it looks like we see 13 total calls to Function.prototype.toString when loading this pageset, but none of them creates a string larger than 739 characters, so this gives no explanation for the memory increase (total # of characters is 1679).
,
Jan 19 2018
It does appear that there are 13818 calls to CreateDynamicFunction (that is, the Function constructor), which _does_ use slightly more memory with this flag enabled. I suspect this is the culprit, though the magnitude seems a bit off (13818 calls should only result in ~135kb increase in heap size). Still, such a large number of calls to the Function constructor definitely makes this page an outlier, which would explain why it's the only benchmark which showed a bump after this patch. I'm still struggling to get a non-noisy repro locally; if I can get dtu's suggestion working, and use Pinpoint to get a non-noisy repro with a patch applied, it should be easy to create a tweaked version of CreateDynamicFunction that won't have any additional memory requirements.
,
Jan 19 2018
On the Job page, it's the pink button with the arrow in the bottom right corner. https://pinpoint-dot-chromeperf.appspot.com/job/1185469f040000
,
Jan 19 2018
You will have to sign in in the top right corner to use it.
,
Jan 19 2018
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/15c7dd48840000
,
Jan 19 2018
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/12fde388840000
,
Jan 20 2018
📍 Couldn't reproduce a difference. https://pinpoint-dot-chromeperf.appspot.com/job/15c7dd48840000
,
Jan 20 2018
📍 Found a significant difference after 1 commit. https://pinpoint-dot-chromeperf.appspot.com/job/12fde388840000 Add |ttsEngine.updateVoices| api By dtseng@chromium.org · Fri Jan 19 23:43:45 2018 chromium @ 6fc593926726c21e5ab868d7cd778bb0a4c4f9f1 Understanding performance regressions: http://g.co/ChromePerformanceRegressions
,
Jan 20 2018
,
Jan 22 2018
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/128142d8840000
,
Jan 22 2018
📍 Couldn't reproduce a difference. https://pinpoint-dot-chromeperf.appspot.com/job/128142d8840000
,
Jan 23 2018
,
Jan 23 2018
,
Jan 23 2018
+benjhayden for results2 FR in Comment 17.
,
Jan 29 2018
📍 Pinpoint job started. https://pinpoint-dot-chromeperf.appspot.com/job/12a3f67a840000
,
Jan 30 2018
📍 Found a significant difference after 1 commit. https://pinpoint-dot-chromeperf.appspot.com/job/12a3f67a840000 WebUI: Update Polymer iron-location version 0.8.8 -> 2.0.3 By dpapad@chromium.org · Mon Jan 29 23:04:42 2018 chromium @ 81a7d54a90c9ed42a492c56b88202d550f3230e3 Understanding performance regressions: http://g.co/ChromePerformanceRegressions
,
Jan 30 2018
,
Jan 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/catapult/+/a4c62a10f2be36026434add3dfb0ccc83d06f8f6 commit a4c62a10f2be36026434add3dfb0ccc83d06f8f6 Author: benshayden <benjhayden@chromium.org> Date: Tue Jan 30 21:06:00 2018 [results.html] Auto Show All when empty search results. Currently, the table is empty when the user searches for a non-top-level histogram name when Show All is unchecked. That behavior can lead users to incorrectly assume that the metric failed to record the histogram. This CL ensures that users can find the desired histogram if it exists by detecting when the search results would be empty because Show All is unchecked, and automatically checks it. Demo of fix: http://www/~benjhayden/801556.html Bug: chromium:801556 Change-Id: I02060cbd5c013a05975bd1a027ff2b91c0a7362a Reviewed-on: https://chromium-review.googlesource.com/893904 Reviewed-by: Ethan Kuefner <eakuefner@chromium.org> Commit-Queue: Ben Hayden <benjhayden@chromium.org> [modify] https://crrev.com/a4c62a10f2be36026434add3dfb0ccc83d06f8f6/tracing/tracing/value/ui/histogram_set_table_test.html [modify] https://crrev.com/a4c62a10f2be36026434add3dfb0ccc83d06f8f6/tracing/tracing/value/ui/histogram_set_table.html
,
Jan 31 2018
,
Jan 31 2018
,
Jan 31 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/483c0313c3cda06c0422f5c33289e69fe5a151df commit 483c0313c3cda06c0422f5c33289e69fe5a151df Author: Adam Klein <adamk@chromium.org> Date: Wed Jan 31 23:14:42 2018 Revert "Reland: Enable --harmony-function-tostring by default" This reverts commit 6fe75e30aacff1f2e9b10a01e20bebd68fe56474. Enabling this flag caused a number of memory and performance regressions, so disabling on the 6.5 branch while I try to fix on tip-of-tree. Bug: v8:4958 , chromium:801556 , chromium:802400 , chromium:807192 Change-Id: Id5a2bb5362038f19140ba995af56bee6b1cf5b4f Reviewed-on: https://chromium-review.googlesource.com/896168 Reviewed-by: Michael Hablich <hablich@chromium.org> Cr-Commit-Position: refs/branch-heads/6.5@{#19} Cr-Branched-From: 73c55f57fe8506011ff854b15026ca765b669700-refs/heads/6.5.254@{#1} Cr-Branched-From: 594a1a0b6e551397cfdf50870f6230da34db2dc8-refs/heads/master@{#50664} [modify] https://crrev.com/483c0313c3cda06c0422f5c33289e69fe5a151df/src/flag-definitions.h [modify] https://crrev.com/483c0313c3cda06c0422f5c33289e69fe5a151df/test/message/fail/paren_in_arg_string.out [modify] https://crrev.com/483c0313c3cda06c0422f5c33289e69fe5a151df/test/mjsunit/harmony/async-generators-basic.js [modify] https://crrev.com/483c0313c3cda06c0422f5c33289e69fe5a151df/test/mjsunit/regress/regress-2470.js [modify] https://crrev.com/483c0313c3cda06c0422f5c33289e69fe5a151df/test/mjsunit/regress/regress-crbug-109362.js [modify] https://crrev.com/483c0313c3cda06c0422f5c33289e69fe5a151df/test/mjsunit/regress/regress-crbug-663410.js [modify] https://crrev.com/483c0313c3cda06c0422f5c33289e69fe5a151df/test/mozilla/mozilla.status
,
Jan 31 2018
I've reverted the flag flip on the 6.5 branch, will continue to investigate on tip-of-tree. Marking as a release-blocker for M66 so this doesn't get dropped on the floor.
,
Feb 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/v8/v8.git/+/537e89a350f2b5642aab3980c88a5a5adea94fa3 commit 537e89a350f2b5642aab3980c88a5a5adea94fa3 Author: Adam Klein <adamk@chromium.org> Date: Fri Feb 02 18:13:22 2018 Fix eval cache with --harmony-function-tostring The code was using the "correct" cache key for lookups, but not for creating new entries, leading to us never hitting the cache for some Function-constructor cases. Bug: v8:4958 , chromium:801556 , chromium:802400 , chromium:807192 Change-Id: I4ac2234b97a9f5f71957ef936dc4b588d020916b Reviewed-on: https://chromium-review.googlesource.com/898096 Reviewed-by: Jakob Kummerow <jkummerow@chromium.org> Commit-Queue: Adam Klein <adamk@chromium.org> Cr-Commit-Position: refs/heads/master@{#51081} [modify] https://crrev.com/537e89a350f2b5642aab3980c88a5a5adea94fa3/src/compiler.cc
,
Feb 5 2018
Graphs seem to have recovered with cache fix. |
||||||||||||||||||||
►
Sign in to add a comment |
||||||||||||||||||||
Comment 1 by 42576172...@developer.gserviceaccount.com
, Jan 12 2018