Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable simulated throttling by default. #120

Merged
merged 1 commit into from
Sep 5, 2019

Conversation

jburger424
Copy link
Contributor

This is a temporary change until simulated results can be verified as accurate.

This is a temporary change until simulated results can be verified as accurate.
@jburger424 jburger424 added chore config Configuration related issues and PRs simulated-throttling PRs and issues related to simulated throttling labels Sep 5, 2019
index.js Show resolved Hide resolved
@jburger424 jburger424 merged commit 8980c9f into googleads:master Sep 5, 2019
@jburger424 jburger424 deleted the disable-simulation branch September 5, 2019 18:55
@patrickhulce
Copy link

patrickhulce commented Sep 6, 2019

FYI not sure if you've already read through the LH variability and accuracy analysis doc, but it should hopefully be a good starting point for your verification process :)

The tl;dr is...

  • Got time to wait for ~5-10 minutes per URL? Use many runs of packet-level throttling for highest accuracy.
  • Got time to wait for ~3-5 minutes per URL and don't care about HTTP/2 accuracy? Use multiple runs of devtools throttling. (Though I personally feel there's no point in picking this option because one should care about HTTP/2 accuracy and might as well wait 7 minutes if you're gonna wait 4 already to get actually accurate results)
  • Got time for none of that and want least variable results? Use simulated throttling (i.e. the default for a human running LH live to get a report).

I'm not sure what use case this script falls into but large scale repeated runs for "true" accuracy should definitely strive to use packet-level throttling. Moving from simulated throttling to no throttling though would definitely be a step backwards in terms of mobile performance accuracy.

@jburger424
Copy link
Contributor Author

@patrickhulce I think I've briefly read it before, but I should definitely read it closer to better understand everything here.

The real issue we've been running into here is the simulation of network timing, this is primarily in preparation for #121 which switches our first paint audit to use a network ping rather than the firstContentfulPaint event due to issues raised in GoogleChrome/lighthouse#8979. Specifically, the network impression ping is showing up before the ad request when looking at the simulation, this could never happen in reality. Even when drawing an edge from the network request to the impression ping, it is still showing up about 50% (~20s) earlier than the event was prior, when in reality they should be quite close in timing.

Long term, we are planning to reenable simulation, but we need to spend more time ensuring that our results make sense before doing that.

@patrickhulce
Copy link

Ah, gotcha thanks for the context! :)

Specifically, the network impression ping is showing up before the ad request when looking at the simulation, this could never happen in reality.

Interesting, we're probably missing whatever is needed to connect the dots then. Is it just an ordinary request made within the frame? Image request? XHR?

Even when drawing an edge from the network request to the impression ping, it is still showing up about 50% (~20s) earlier than the event was prior, when in reality they should be quite close in timing.

50% earlier than the observed time of the request and the ad was loaded 40s in? I'm not sure I follow this bit sorry :( FWIW, this is stretching a bit the original intention of simulation since until this point we haven't been too concerned about the ordering of specific resources so long as all the dependencies are captured and the final end time is the same. You might end up running into more of these types of situations when focusing on start times of specific resources, but we can come up with some changes to make this part more accurate if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore config Configuration related issues and PRs simulated-throttling PRs and issues related to simulated throttling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants