All graphs for this bug:
  https://chromeperf.appspot.com/group_report?bug_id=639754

Original alerts at time of bug-filing:
  https://chromeperf.appspot.com/group_report?keys=agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgrsmy_wgM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg7t-sqgoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg7tzIpQoM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICgrs2T-ggM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg7qTb8AkM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg7qDfswsM,agxzfmNocm9tZXBlcmZyFAsSB0Fub21hbHkYgICg7sjN9AkM


Bot(s) for this bug's original alert(s):

chromium-rel-mac-retina
chromium-rel-mac10
chromium-rel-mac11
chromium-rel-win7-gpu-ati
chromium-rel-win7-gpu-intel
chromium-rel-win7-gpu-nvidia
linux-release

Started bisect job https://chromeperf.appspot.com/buildbucket_job_status/9003644594455713632

=== Auto-CCing suspected CL author nhiroki@chromium.org ===

Hi nhiroki@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 : Worker: Fix broken GC logic on Dedicated Worker while DOMTimer is set
Author  : nhiroki
Commit description:
  
V8GC detemines whether a Dedicated Worker can be collectable by checking
pending activities on WorkerGlobalScope. This activity state is managed by
InProcessWorkerMessagingProxy on the main thread. In some situation, this
logic is broken and this CL fixes it.

<Before this CL>

InProcessWorkerObjectProxy on the worker thread reports pending activities
to the main thread in following cases:

(1) After DedicatedWorkerThread is initialized, reports that there are no
    pending activities regardless of whether they exist.
(2) After MessageEvent is dispatched, checks a pending activity using
    V8GCController and reports a result.

This logic does not work when a DOMTimer is set on initial script evaluation or
on MessageEvent. DOMTimer must be treated as a pending activity[*], but both
(1) and (2) do not take care of it (regarding 2, DOMTimer is not a
ScriptWrappable and V8GCController does not count such an object). As a result,
a worker can be GC'ed even if there is still an active timer.

<After this CL>

After initial script evaluation or MessageEvent, the object proxy starts an
exponential backoff timer to periodically check an activity state and reports
to the messaging proxy when all activities are done.

The messaging proxy reports to the GC that the worker object is collectable
after all posted messages and pending activities are completed.

[*] https://html.spec.whatwg.org/multipage/workers.html#the-worker's-lifetime

BUG= 572226 ,  584851 

Review-Url: https://codereview.chromium.org/2124693002
Cr-Commit-Position: refs/heads/master@{#413052}
Commit  : 9af6c91f5bc79d12247a5780071c44025e4c026d
Date    : Fri Aug 19 04:59:03 2016


===== TESTED REVISIONS =====
Revision         Mean     Std Dev   N  Good?
chromium@413041  29.0204  1.32448   5  good
chromium@413051  29.049   0.839761  5  good
chromium@413052  30.7256  0.931444  5  bad    <--
chromium@413053  32.2366  0.81565   5  bad
chromium@413054  32.5016  0.871213  5  bad
chromium@413056  32.6094  0.203294  5  bad
chromium@413060  31.766   0.643148  8  bad

Bisect job ran on: winx64intel_perf_bisect
Bug ID: 639754

Test Command: src/tools/perf/run_benchmark -v --browser=release_x64 --output-format=chartjson --upload-results --also-run-disabled-tests blink_perf.bindings
Test Metric: post-message/post-message
Relative Change: 9.11%
Score: 99.9

Buildbot stdio: http://build.chromium.org/p/tryserver.chromium.perf/builders/winx64intel_perf_bisect/builds/1124
Job details: https://chromeperf.appspot.com/buildbucket_job_status/9003644594455713632


Not what you expected? We'll investigate and get back to you!
  https://chromeperf.appspot.com/bad_bisect?try_job_id=5847159144972288

| 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!

According to the perf dashboard, post-message.html[1] has been degraded. This test crates a dedicated worker and does pingpong between a document and the worker by postMessage. My patch schedules a blink::Timer to lazily check activities on WorkerGlobalScope for V8 GC when a message is posted from a document to a worker, so that is likely to degrade the test.

[1] https://cs.chromium.org/chromium/src/third_party/WebKit/PerformanceTests/Bindings/post-message.html?q=post-message&sq=package:chromium&dr

Would you help me understand why the CL regresses performance? I can expect it might regress memory though...

I'm now suspecting that V8GCController::hasPendingActivity() takes time, I haven't closely investigated this yet though (I'll do it tomorrow), 

Before my patch, the function didn't perform well because WorkerGlobalScope.idl wasn't be annotated with [ActiveScriptWrappable]. After my patch, it correctly iterates objects on WorkerGlobalScope and could take time.

I confirmed that my patch has slightly degraded the test on my local env (Linux, Release build).

I ran the test[*] on following release builds:

  (A) The commit before my commit (master@{#413051})
  (B) The commit before my commit with some modification that annotates WorkerGlobalScope with [AcriveScriptWrappable].
  (C) My commit (master@{#413052})

[*] ./tools/perf/run_benchmark run blink_perf.bindings --story-filter=post-message.html --browser=exact --browser-executable=out/Release/content_shell

Results are as follows:

  (A) Avg: 84.09900000000002
  (B) Avg: 84.84000000000003
  (C) Avg: 93.60200000000003


> Before my patch, the function didn't perform well because WorkerGlobalScope.idl wasn't be annotated with [ActiveScriptWrappable]. After my patch, it correctly iterates objects on WorkerGlobalScope and could take time.

I assumed that, before my patch, V8GCController completely didn't iterate objects on WorkerGlobalScope because lack of the annotation, but actually only WorkerGlobalScope is not iterated in the case, so the difference between (A) and (B) is trivial :p

By the way, how seriously should I consider this regression?

Degradation is 10+ ms/test and the test posts 1000 round-trip messages, so the degradation/message is 0.01+ ms. This looks not so big to me.

WDYT?

Yeah, I'm okay with marking it as wontfix.

OK, then I'll mark this as wontfix.