core(page-functions): expose simulated throttling requestIdleCallback shim #11032

LukasAuerMstage · 2020-06-28T19:31:52Z

Summary
This PR introduces a request idle callback shim that gets applied for lighthouse tests that are run with simulated throttling.

The feature is needed as the use of the request idle callback deadline defeats the simulated throttling logic. Until the deadline hits 0 tasks can be processed within the request idle callback, which adds up to a maximum of 50ms. Lighthouse will then multiply these 50ms with 4 if simulated throttling is active, which results in a long task.

This riC shim calculates the remainig deadline with this issue in mind and will deliver a value of 12ms or less, so that once the simulated throttling is applied, the overall time consumed does not surpass 50ms and no long task gets registered.

Related Issues/PRs
#10693

into ric-shim

-disable-storage-reset -clear origin storage

…re applying the shim

…etect

patrickhulce

thank you very much @LukasAuerMstage this looks great! 🎉

patrickhulce · 2020-06-29T14:51:43Z

lighthouse-core/gather/driver.js

+   */
+  async registerRequestIdleCallbackWrap(settings) {
+    if (settings.throttlingMethod === 'simulate') {
+      const scriptStr = `(${pageFunctions.wrapRequestIdleCallbackString})()`;


let's pass through the multiplier to the function so it doesn't have to hardcode the deadline

Suggested change

const scriptStr = `(${pageFunctions.wrapRequestIdleCallbackString})()`;

const scriptStr = `(${pageFunctions.wrapRequestIdleCallbackString})(${settings.throttling.cpuSlowdownMultiplier)`;

patrickhulce · 2020-06-29T14:53:42Z

lighthouse-core/lib/page-functions.js

+ * RequestIdleCallback shim that calculates the remaining deadline time in order to avoid a potential lighthouse
+ * penalty for tests run with simulated throttling. As the throttling factor is x4 the maximum remaining time is
+ * reduced to approximately 50/4 => ~12
+ * @return {null}
+ */
+/* istanbul ignore next */
+function wrapRequestIdleCallback() {


Suggested change

* RequestIdleCallback shim that calculates the remaining deadline time in order to avoid a potential lighthouse

* penalty for tests run with simulated throttling. As the throttling factor is x4 the maximum remaining time is

* reduced to approximately 50/4 => ~12

* @return {null}

*/

/* istanbul ignore next */

function wrapRequestIdleCallback() {

* RequestIdleCallback shim that calculates the remaining deadline time in order to avoid a potential lighthouse

* penalty for tests run with simulated throttling. Reduces the deadline time to (50 - safetyAllowance) / cpuSlowdownMultiplier to

* ensure a long task is very unlikely if using the API correctly.

* @param {number} cpuSlowdownMultiplier

* @return {null}

*/

/* istanbul ignore next */

function wrapRequestIdleCallback(cpuSlowdownMultiplier) {

const safetyAllowanceMs = 10;

const maxExecutionTimeMs = Math.floor((50 - safetyAllowanceMs) / cpuSlowdownMultiplier);

patrickhulce · 2020-06-29T14:56:18Z

lighthouse-core/lib/page-functions.js

+      const start = Date.now();
+      deadline.__timeRemaining = deadline.timeRemaining;
+      deadline.timeRemaining = () => {
+        return Math.min(deadline.__timeRemaining(), Math.max(0, 12 - (Date.now() - start)));


Suggested change

return Math.min(deadline.__timeRemaining(), Math.max(0, 12 - (Date.now() - start)));

return Math.min(deadline.__timeRemaining(), Math.max(0, maxExecutionTimeMs - (Date.now() - start)));

might need to reflow with line length :/

patrickhulce · 2020-06-29T14:57:39Z

lighthouse-cli/test/smokehouse/test-definitions/lantern/lantern-expectations.js

+      requestedUrl: 'http://localhost:10200/ric-shim.html?short',
+      finalUrl: 'http://localhost:10200/ric-shim.html?short',
+      audits: {
+        'interactive': {


TTI can be super flaky on test machines depending on scheduling of network too, let's use total blocking time instead which is strictly time spent in tasks beyond 50ms :)

patrickhulce · 2020-06-29T14:59:00Z

lighthouse-cli/test/fixtures/ric-shim.html

+    const timeStart = Date.now();
+    while (true) {
+      let elapsedTime = Date.now() - timeStart;
+      if (elapsedTime > milliseconds) {


Suggested change

if (elapsedTime > milliseconds) {

if (elapsedTime >= milliseconds) {

otherwise sleep(1) will always sleep for at least 2 ms, right?

yeah thats right, i missed that

patrickhulce · 2020-06-29T14:59:29Z

lighthouse-cli/test/fixtures/ric-shim.html

+  const iterations = 40;
+
+  if (window.location.search.includes('short')) {
+    window.ricTaskDuration = 1;


nit: seems like this window should just be a let ricTaskDuration = 0

If i remember correctly let/const dont move up into the global scope if they are in some sort of curly braces but i could do something like this, wdyt? Or just use var if you prefer that because that should just move up.

let ricTaskDuration;
if (window.location.search.includes('short')) {
ricTaskDuration = 1;
}
if (window.location.search.includes('long')) {
ricTaskDuration = 50;
}

yeah sorry that's what I meant, declaring let ricTaskDuration = 0 up with iterations and then overriding it if the query param matches

lighthouse-cli/test/fixtures/ric-shim.html

- pass cpu slowdown multiplier to rIC shim wrapper and calculate the maximum execution time dynamically - adjust rIC shim function comment - refactor smoke tests so that they use tbt instead of tti - fix bug in sleep function (was previously 1ms off) - add copyright header to smoke test html

patrickhulce

Beautiful work! Just a few last nits, but LGTM :)

patrickhulce · 2020-06-29T17:27:32Z

lighthouse-cli/test/fixtures/ric-shim.html

@@ -0,0 +1,47 @@
+<!DOCTYPE html>
+<!--
+ * Copyright 2018 The Lighthouse Authors. All Rights Reserved.


Suggested change

* Copyright 2018 The Lighthouse Authors. All Rights Reserved.

* Copyright 2020 The Lighthouse Authors. All Rights Reserved.

great :) done

patrickhulce · 2020-06-29T17:29:00Z

lighthouse-cli/test/smokehouse/test-definitions/lantern/lantern-expectations.js

+        'total-blocking-time': {
+          // With the requestIdleCallback shim in place 1ms tasks should not block at all and should max add up to
+          // 12.5 ms each, which would result in 50ms under a 4x simulated throttling multiplier and therefore in 0 tbt
+          numericValue: '<=0',


I think we can give it a little bit of slack to avoid flakiness, maybe <=100? going from 5500 -> 100 is still great :)

makes sense 👍 done

- change copyright header date to 2020 - change tbt smoke test value to <=100 (from previously 0) to add some room for a little variance

patrickhulce · 2020-06-29T17:59:10Z

Thanks @LukasAuerMstage looks all good! Unfortunately you'll need to rebase after #11035 lands to fix the required status checks that are failing atm.

brendankenny · 2020-06-29T18:06:09Z

@patrickhulce as a follow up at some point, could we document this somewhere? It'll be easy to forget this is happening at runtime (since most lantern stuff happens after the fact).

Would lantern.md be best? Maybe we should make a general simulated throttling explainer doc (or section in throttling.md) at some point.

patrickhulce · 2020-06-29T18:42:53Z

Would lantern.md be best? Maybe we should make a general simulated throttling explainer doc

Yeah a little "Lantern Considerations" type deal might be good to get a running list of things that need to happen for lantern to work well.

connorjclark · 2020-06-29T18:51:28Z

lighthouse-core/lib/page-functions.js

@@ -303,6 +303,39 @@ function getNodeLabel(node) {
  return tagName;
 }

+/**
+ * RequestIdleCallback shim that calculates the remaining deadline time in order to avoid a potential lighthouse


Suggested change

* RequestIdleCallback shim that calculates the remaining deadline time in order to avoid a potential lighthouse

* RequestIdleCallback shim that calculates the remaining deadline time in order to avoid a potential

connorjclark · 2020-06-29T18:53:57Z

lighthouse-core/lib/page-functions.js

+  const nativeRequestIdleCallback = window.requestIdleCallback;
+  window.requestIdleCallback = (cb) => {
+    const cbWrap = (deadline, timeout) => {
+      const start = Date.now();


will performance.now() work too?

it will, we're like split 50/50 throughout the smokes on Date.now vs. performance.now

I think Date.now() is faster than performance.now() thats why i chose it, but i dont know if it matters all that much in this case. Do you want me to refactor to performance.now()?

I don't think it's necessary @LukasAuerMstage but @connorjclark were you hoping to slowly standardize on performance.now?

… shim (GoogleChrome#11032)

LukasAuerMstage added 4 commits May 6, 2020 22:08

ric-shim

46eddb7

fix timout passing

4de0fed

Request Idle Callback Shim Smoke Tests

82403ee

Add Comments and code styling fixes

899ae25

LukasAuerMstage requested a review from a team as a code owner June 28, 2020 19:31

LukasAuerMstage requested review from paulirish and removed request for a team June 28, 2020 19:31

vercel bot deployed to Preview June 28, 2020 19:31 View deployment

This comment has been minimized.

Sign in to view

googlebot added the cla: no label Jun 28, 2020

Merge branch 'master' into ric-shim

7dc9c22

vercel bot deployed to Preview June 28, 2020 19:33 View deployment

This comment has been minimized.

Sign in to view

googlebot added cla: yes and removed cla: no labels Jun 28, 2020

LukasAuerMstage added 2 commits June 28, 2020 21:45

Add code styling fixes

e61ad1d

Merge branch 'ric-shim' of https://github.com/LukasAuerMstage/lighthouse

58d4af9

into ric-shim

vercel bot deployed to Preview June 28, 2020 19:46 View deployment

patrickhulce self-assigned this Jun 28, 2020

patrickhulce self-requested a review June 28, 2020 19:57

patrickhulce added the waiting4reviewer label Jun 28, 2020

LukasAuerMstage changed the title ~~Ric shim~~ core(page-functions): Expose simulated throttling specific requestIdleCallback shim Jun 29, 2020

LukasAuerMstage changed the title ~~core(page-functions): Expose simulated throttling specific requestIdleCallback shim~~ core(page-functions): Expose simulated throttling requestIdleCallback shim Jun 29, 2020

LukasAuerMstage changed the title ~~core(page-functions): Expose simulated throttling requestIdleCallback shim~~ core(page-functions): expose simulated throttling requestIdleCallback shim Jun 29, 2020

Add ric shim registration to fake driver and gather runner test

2704a90

vercel bot deployed to Preview June 29, 2020 06:30 View deployment

Add ric shim registration to additional gather runner test:

d8f36ea

-disable-storage-reset -clear origin storage

vercel bot deployed to Preview June 29, 2020 06:42 View deployment

Check if request idle callback is available on the window object befo…

bffa3d7

…re applying the shim

Add "istanbul ignore next" comment to rIC shim + remove rIC feature d…

f53b6ed

…etect

vercel bot deployed to Preview June 29, 2020 14:27 View deployment

patrickhulce suggested changes Jun 29, 2020

View reviewed changes

patrickhulce added waiting4committer and removed waiting4reviewer labels Jun 29, 2020

vercel bot deployed to Preview June 29, 2020 16:34 View deployment

LukasAuerMstage requested a review from patrickhulce June 29, 2020 17:02

patrickhulce added waiting4reviewer and removed waiting4committer labels Jun 29, 2020

patrickhulce approved these changes Jun 29, 2020

View reviewed changes

patrickhulce added waiting4committer and removed waiting4reviewer labels Jun 29, 2020

Add changes according to PR suggestions GoogleChrome#2:

5b45443

- change copyright header date to 2020 - change tbt smoke test value to <=100 (from previously 0) to add some room for a little variance

vercel bot deployed to Preview June 29, 2020 17:51 View deployment

LukasAuerMstage requested a review from patrickhulce June 29, 2020 17:51

patrickhulce added land-when-ci-is-green and removed waiting4committer labels Jun 29, 2020

connorjclark reviewed Jun 29, 2020

View reviewed changes

Merge branch 'master' into ric-shim

d821f4c

vercel bot deployed to Preview June 29, 2020 20:04 View deployment

devtools-bot merged commit 3085942 into GoogleChrome:master Jun 29, 2020

devtools-bot removed the land-when-ci-is-green label Jun 29, 2020

patrickhulce mentioned this pull request Jul 2, 2020

Expose simulated throttling specific requestIdleCallback params #10693

Closed

gMakunde pushed a commit to gMakunde/lighthouse that referenced this pull request Jul 6, 2020

core(page-functions): expose simulated throttling requestIdleCallback…

96b5cac

… shim (GoogleChrome#11032)

gMakunde pushed a commit to gMakunde/lighthouse that referenced this pull request Aug 28, 2020

core(page-functions): expose simulated throttling requestIdleCallback…

37d764a

… shim (GoogleChrome#11032)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

core(page-functions): expose simulated throttling requestIdleCallback shim #11032

core(page-functions): expose simulated throttling requestIdleCallback shim #11032

LukasAuerMstage commented Jun 28, 2020

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

patrickhulce left a comment

patrickhulce Jun 29, 2020

patrickhulce Jun 29, 2020

patrickhulce Jun 29, 2020

patrickhulce Jun 29, 2020

patrickhulce Jun 29, 2020

patrickhulce Jun 29, 2020

patrickhulce Jun 29, 2020

LukasAuerMstage Jun 29, 2020

patrickhulce Jun 29, 2020

LukasAuerMstage Jun 29, 2020

patrickhulce Jun 29, 2020

patrickhulce left a comment

patrickhulce Jun 29, 2020

LukasAuerMstage Jun 29, 2020

patrickhulce Jun 29, 2020

LukasAuerMstage Jun 29, 2020

patrickhulce commented Jun 29, 2020

brendankenny commented Jun 29, 2020

patrickhulce commented Jun 29, 2020

connorjclark Jun 29, 2020

connorjclark Jun 29, 2020 •

edited

Loading

patrickhulce Jun 29, 2020

LukasAuerMstage Jun 29, 2020

patrickhulce Jun 29, 2020

	const scriptStr = `(${pageFunctions.wrapRequestIdleCallbackString})()`;
	const scriptStr = `(${pageFunctions.wrapRequestIdleCallbackString})(${settings.throttling.cpuSlowdownMultiplier)`;

	return Math.min(deadline.__timeRemaining(), Math.max(0, 12 - (Date.now() - start)));
	return Math.min(deadline.__timeRemaining(), Math.max(0, maxExecutionTimeMs - (Date.now() - start)));

	if (elapsedTime > milliseconds) {
	if (elapsedTime >= milliseconds) {

	* Copyright 2018 The Lighthouse Authors. All Rights Reserved.
	* Copyright 2020 The Lighthouse Authors. All Rights Reserved.

	* RequestIdleCallback shim that calculates the remaining deadline time in order to avoid a potential lighthouse
	* RequestIdleCallback shim that calculates the remaining deadline time in order to avoid a potential

core(page-functions): expose simulated throttling requestIdleCallback shim #11032

core(page-functions): expose simulated throttling requestIdleCallback shim #11032

Conversation

LukasAuerMstage commented Jun 28, 2020

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

patrickhulce left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patrickhulce left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

patrickhulce commented Jun 29, 2020

brendankenny commented Jun 29, 2020

patrickhulce commented Jun 29, 2020

Choose a reason for hiding this comment

connorjclark Jun 29, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

connorjclark Jun 29, 2020 •

edited

Loading