Looking at the above list, I can somewhat -- but not totally -- agree that they are creating excessively many workers. fetch-event.https.html creates 15. activation.https.html creates 12. windowclient-navigate.https.html creates 10. Our test files tend to create one worker per test case.

It seems empirically then, we can have say 10/2 = 5 workers per test file with this change, which is a bit harsh. But I think some other test files are doing more than 5 workers, so it could be something else in these particular tests also contributes to slowness, e.g., navigate is also doing hard work navigating to URLs.

For real world performance, currently it should not have no regression because the feature is disabled by default.

And for the issue to be 3x-5x slower, yes, I think DCHECK matters. In V8 they add some additional validators of dcbeck_is_always_on, and they can be the reason of the slowness.

Correction: It should have no regression for the real world, because it is disabled by default.

Re #1;
I think the number of workers is not directly critical, but the amount  of works done in each worker is.

Ah that's useful to know. What is the launch plan for this feature? It'd be nice to do an A/B test so we can see ServiceWorker.StartWorker.Time with and without the change. We trying very hard to optimize sw start time especially in the upcoming milestones.

Regarding #4, then I much not be understanding something. I thought that the slowdown was caused because snapshotting made isolate creation slower, and each worker thread startup requires isolate creation. Can you explain more why the amount of work the worker is the main factor?

Is it possible to separate tests w/ the context snapshot as 'virtual' tests? I'm a bit concerned that marking a bunch of tests as SLOW could hide other perf regressions.

c#6: I'm kinda less enthusiastic about that... each virtual test suite adds more complexity with expectations and configurations...

I think it's OK to start splitting these files up and we can use real UMA and perf tests for perf tracking, rather than using layout tests for perf tracking.

c#7: Fair enough. Keeping multiple configurations are cumbersome, and perf regression should be detected by perf tests, not by layout tests.

I've started to split up the tests.

peria-san: Can you kick the discussion with the V8 team so that we can stop passing the reference table to workers? We should fix the performance issue (I think it's fixable) rather than justifying the regression via A/B testing etc.

Yes, will do.
JFYI,  crbug.com/v8/6433  is the long term solution, and  crbug.com/v8/6448  can be a short term solution.

I'll delegate the decision to falken@ and nhiroki@ but we could consider reverting the CL because we'll anyway need to fix the performance issue before enabling the feature :)

Thanks for investigating and having a plan to fix. My reasoning was that it’d be acceptable to regress performance for DCHECK builds only, if that's really what's going on and if it’d be hard to fix. But I’d be more confident that real-world performance is not impacted if there was no regression anywhere, which is why I suggested the A/B rollout.

That said, fixing DCHECK build perf, or reverting in the meantime, would be highly appreciated. I frequently run SW tests with DCHECKs on in my development cycle and now running the layout tests on Z620 takes 6 minutes with several timeout failures. When I run with the RuntimeEnabled flag disabled it takes 2 minutes (does that completely disable the feature? I thought it was faster, could be misremembering though).

Splitting up the files is a good idea anyway but it won't solve the absolute runtime.

I'm still interested to know why the # of workers is not the key factor but the amount of work done is. These test files have pretty small, simple workers. 

# I just ran with DCHECKs disabled and it takes about 2 minutes again regardless of the RuntimeEnabled flag, so it does indeed look like a DCHECK thing.

V8 team made a CL to work for this issue, and I checked it resolved this regression at least on my machine.

Wow, thanks for the fast fix then!

c#14: Can you share the link to the CL / issue so that we can track the progress?

It is https://chromium-review.googlesource.com/c/v8/v8/+/632098
it passes all layout tests, and waiting for reviews.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cb2d46a78f539c413b38540edf1ece9d23b53b7d

commit cb2d46a78f539c413b38540edf1ece9d23b53b7d
Author: Matt Falkenhagen <falken@chromium.org>
Date: Mon Aug 28 08:54:16 2017

service worker WPT tests: split very big registration tests into multiple files

registration.https.html and link-element-register.https.html do about 70
register calls, most of which result in a service worker starting up. These
test files always came close to the test harness timeout, and with the new
snapshotting change r496290 which does a lot of work in Debug, started timing
out on Debug. The snapshotting change is supposed to be fixed soon, but split
them up into smaller files anyway.

I verified that no tests were lost by generating the new files then
`cat registration-*expected* | sort | uniq` and comparing to
the existing test output (and similar for link-element-register*).

Bug:  758481 
Change-Id: I7e522e3c4e87df11fcb5197da59005e2d9e25f92
Reviewed-on: https://chromium-review.googlesource.com/635065
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#497717}
[modify] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/SlowTests
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/link-element-register-basic.https.html
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/link-element-register-mime-types.https-expected.txt
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/link-element-register-mime-types.https.html
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/link-element-register-scope.https-expected.txt
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/link-element-register-scope.https.html
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/link-element-register-script-url.https-expected.txt
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/link-element-register-script-url.https.html
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/link-element-register-script.https-expected.txt
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/link-element-register-script.https.html
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/link-element-register-security-error.https-expected.txt
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/link-element-register-security-error.https.html
[delete] https://crrev.com/fa42bdc356f6595d682bcfff7cb08a4a0374686e/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/register-link-element.https.html
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/registration-basic.https.html
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/registration-mime-types.https-expected.txt
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/registration-mime-types.https.html
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/registration-scope.https.html
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/registration-script-url.https.html
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/registration-script.https.html
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/registration-security-error.https.html
[delete] https://crrev.com/fa42bdc356f6595d682bcfff7cb08a4a0374686e/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/registration.https.html
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/registration-tests-basic.js
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/registration-tests-mime-types.js
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/registration-tests-scope.js
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/registration-tests-script-url.js
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/registration-tests-script.js
[add] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/registration-tests-security-error.js
[modify] https://crrev.com/cb2d46a78f539c413b38540edf1ece9d23b53b7d/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/test-helpers.sub.js

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7b7939dfcb29ddcac2e5eb20ee0b891376365ad4

commit 7b7939dfcb29ddcac2e5eb20ee0b891376365ad4
Author: Hitoshi Yoshida <peria@chromium.org>
Date: Tue Aug 29 07:58:43 2017

bindings: Do not use external reference table on worker threads

V8 has a performance issue in creating an isolate with a large
sized external reference table, and conceptually we don't need
the table on workers.
Now the validator in  crbug.com/v8/6448  was relaxed, and hence
we don't have to set a large table on workers' isolates.


Bug:  758481 
Change-Id: I274c5cd5ad5ac8ef5c7344fb987e9f1bde8fa70d
Reviewed-on: https://chromium-review.googlesource.com/640051
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Hitoshi Yoshida <peria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498043}
[modify] https://crrev.com/7b7939dfcb29ddcac2e5eb20ee0b891376365ad4/third_party/WebKit/Source/core/workers/WorkerBackingThread.cpp

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aad197db6fed7099309d586a62079c755824b841

commit aad197db6fed7099309d586a62079c755824b841
Author: Hitoshi Yoshida <peria@chromium.org>
Date: Wed Aug 30 06:14:31 2017

LayoutTest: Remove layout tests for service worker from Slow list

The regression introduced by the snapshot feature should be fixed
by V8 change and https://chromium-review.googlesource.com/c/chromium/src/+/640051
so we don't have to label these tests slow.


Bug:  758481 
Change-Id: I1387e020a6808d751d1e3b4230ee315c5fc15230
Reviewed-on: https://chromium-review.googlesource.com/642631
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Hitoshi Yoshida <peria@chromium.org>
Cr-Commit-Position: refs/heads/master@{#498378}
[modify] https://crrev.com/aad197db6fed7099309d586a62079c755824b841/third_party/WebKit/LayoutTests/SlowTests

Thanks for fixing this and removing the SlowTests!

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/83a535528ca34f867466888a09d62c9c154586e8

commit 83a535528ca34f867466888a09d62c9c154586e8
Author: Matt Falkenhagen <falken@chromium.org>
Date: Mon Sep 04 04:58:46 2017

service worker WPT: remove used resources/registration-tests.js file

I forgot to remove this as part of
https://github.com/w3c/web-platform-tests/commit/4e7404d96051d

Bug:  758481 
TBR: nhiroki
Change-Id: I192b3c99de69ba3cf37c12d30589fbf54739a412
Reviewed-on: https://chromium-review.googlesource.com/647389
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499436}
[delete] https://crrev.com/51083bfc335f95dd0ed0484532231062b2cbeb9b/third_party/WebKit/LayoutTests/external/wpt/service-workers/service-worker/resources/registration-tests.js