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

Add more correctness checks to network analyzer #7045

Closed
patrickhulce opened this issue Jan 17, 2019 · 1 comment
Closed

Add more correctness checks to network analyzer #7045

patrickhulce opened this issue Jan 17, 2019 · 1 comment
Assignees

Comments

@patrickhulce
Copy link
Collaborator

patrickhulce commented Jan 17, 2019

Summary
There's a lot of estimates happening in network-analyzer. We can sanity check more of these to make sure there's nothing fishy going on.

  • In estimateIfConnectionWasReused, compute the maximum number of parallel requests (N) and ensure we have at least N requests that required new connections.
  • Expand the findMainDocument handling of edge cases
@connorjclark connorjclark changed the title Add more sanity checks to network analyzer Add more correctness checks to network analyzer Sep 16, 2020
@patrickhulce
Copy link
Collaborator Author

I'm no longer convinced this is worth pursuing.

In estimateIfConnectionWasReused, compute the maximum number of parallel requests (N) and ensure we have at least N requests that required new connections.

In general prevalence of H2 has made this much less relevant and we've moved away from fatal errors when things seem just a little fishy (🎉). It's not clear what we action Lighthouse would take if our estimates determined there were only N-1 available connections instead of N.

Expand the findMainDocument handling of edge cases

Definitely worth doing, but super complicated and I'm already addressing pieces as we go for #8984

I'll close for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants