Skip to content

core(lantern): maximize throughput under HTTP/2#11666

Merged
patrickhulce merged 5 commits intomasterfrom
simulator_slow_download
Dec 7, 2020
Merged

core(lantern): maximize throughput under HTTP/2#11666
patrickhulce merged 5 commits intomasterfrom
simulator_slow_download

Conversation

@patrickhulce
Copy link
Collaborator

@patrickhulce patrickhulce commented Nov 13, 2020

Summary
Basically we were creating too many connections under HTTP/2 simulation in our connection pool that meant warm connections with excess bandwidth didn't get used properly.

It doesn't really have a huge effect in most real-world CLI or DevTools usage because we can usually trust the connectionReused boolean data from the network records there, but the effect in the tests @Beytoven was creating was large because most of our toy examples all share a connection ID (one of the conditions where we discard observed connection warmth).

Metric                    p50 (% Error)        p90 (% Error)        p95 (% Error)
optimisticFCP             29.3% ↔  0.0%        52.9% ↔  0.0%        62.3% ↔  0.0%
pessimisticFCP            27.7% ↔  0.0%        50.1% ↔  0.0%        62.3% ↔  0.0%
roughEstimateOfFCP        29.2% ↔  0.0%        50.1% ↔  0.0%        58.0% ↔  0.0%
optimisticFMP             33.5% ↔  0.2%        66.5% ↔  0.0%        71.1% ↔  0.0%
pessimisticFMP            29.3% ↔  0.0%        51.9% ↔  0.0%        62.3% ↔  0.0%
roughEstimateOfFMP        30.2% ↔  0.0%        53.9% ↔  0.0%        66.4% ↔  0.0%
optimisticTTFCPUI         30.3% ↓  0.8%        66.4% ↔  0.5%        95.0% ↔  0.0%
pessimisticTTFCPUI        25.2% ↔  0.0%        76.9% ↔  0.0%        135.0% ↔  0.0%
roughEstimateOfTTFCPUI    28.5% ↓  1.7%        65.1% ↓  1.5%        81.0% ↓↓ 13.9%
optimisticTTI             40.9% ↑  1.2%        70.3% ↓  1.1%        81.0% ↔  0.0%
pessimisticTTI            24.7% ↔  0.0%        65.3% ↔  0.0%        77.8% ↔  0.0%
roughEstimateOfTTI        30.6% ↑  2.1%        65.8% ↔  0.0%        74.4% ↔  0.0%
optimisticSI              75.7% ↔  0.0%        86.0% ↔  0.0%        89.3% ↔  0.0%
pessimisticSI             38.4% ↔  0.0%        79.6% ↔  0.0%        125.0% ↔  0.0%
roughEstimateOfSI         27.5% ↔  0.0%        70.2% ↔  0.0%        100.8% ↔  0.0%
optimisticLCP             21.9% ↓  1.3%        71.4% ↑↑ 6.9%        87.7% ↔  0.0%
pessimisticLCP            20.8% ↔  0.0%        74.8% ↔  0.0%        96.6% ↔  0.0%
roughEstimateOfLCP        20.4% ↓  0.7%        64.6% ↔  0.0%        87.7% ↔  0.0%

 ------- Bucket Summary -------
Good:      151   ↑  2
OK:        228   ↔  1
Bad:       81    ↓  1

 ------- % Error Summary -------
p50:       28.4% ↔  0.0%
p90:       59.6% ↔  0.1%
p95:       72.4% ↓  0.9%

Related Issues/PRs
Fixes #11657

@patrickhulce patrickhulce requested a review from a team as a code owner November 13, 2020 19:22
@patrickhulce patrickhulce requested review from Beytoven and removed request for a team November 13, 2020 19:22
@google-cla google-cla bot added the cla: yes label Nov 13, 2020
connorjclark
connorjclark previously requested changes Nov 16, 2020
View reviewed changes
Copy link
Collaborator

@connorjclark connorjclark left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait for v7

@patrickhulce patrickhulce mentioned this pull request Nov 17, 2020
Open
@patrickhulce patrickhulce dismissed connorjclark’s stale review December 7, 2020 20:34

we're landing for v7 these days :)

@connorjclark
Copy link
Collaborator

connorjclark commented Dec 7, 2020

Still good to merge?

@patrickhulce
Copy link
Collaborator Author

patrickhulce commented Dec 7, 2020

yup 👍

@patrickhulce patrickhulce merged commit a582a67 into master Dec 7, 2020
@patrickhulce patrickhulce deleted the simulator_slow_download branch December 7, 2020 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@connorjclark connorjclark connorjclark left review comments

+1 more reviewer

@Beytoven Beytoven Beytoven approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@Beytoven Beytoven

Labels

breaking chillin ❄ cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Investigate simulator behavior given test data

4 participants

@patrickhulce @connorjclark @Beytoven @devtools-bot