core(preconnect): use lantern to compute savings #5070
Conversation
| @@ -2894,27 +2894,27 @@ | |||
| { | |||
| "url": "http://localhost:10200/dobetterweb/dbw_tester.css?delay=100", | |||
| "totalBytes": 821, | |||
| "wastedMs": 873 | |||
| "wastedMs": 330 | |||
There was a problem hiding this comment.
No clue why these changed I'm looking into it...
patrickhulce
force-pushed
the
lantern_preconnect_opp
branch
from
bfc0e5f
to
41d4fca
Compare
| // DNS time can be large but can also be 0 if a commonly used origin that's cached, so make | ||
| // no assumption about DNS. | ||
| const additionalRtt = additionalRttByOrigin.get(securityOrigin) || 0; | ||
| let connectionTime = rtt + additionalRtt; |
There was a problem hiding this comment.
is rtt not number | undefined? (based on LH.Gatherer.Simulation.Options, at least)
There was a problem hiding this comment.
oh, the file isn't type checked yet :) It would be good to handle it now, though
There was a problem hiding this comment.
fixed on the simulator side 👍
typings/web-inspector.d.ts
Outdated
| @@ -17,7 +17,6 @@ declare global { | |||
| url: string; | |||
| _url: string; | |||
| protocol: string; | |||
| origin: string | null; | |||
There was a problem hiding this comment.
I know this was from the last PR, but should we put a warning on origin to not use it? like
/** use `parsedURL.securityOrigin()` instead */
origin: never;or something. It could probably sneak back in in the future otherwise (just a comment might also work :)
There was a problem hiding this comment.
yeah good call done
| // no assumption about DNS. | ||
| const additionalRtt = additionalRttByOrigin.get(securityOrigin) || 0; | ||
| let connectionTime = rtt + additionalRtt; | ||
| if (firstRecordOfOrigin.parsedURL.scheme === 'https') connectionTime = connectionTime * 2; |
There was a problem hiding this comment.
comment on where the *2 comes from (is that the TLS mentioned above? Why *2?)
patrickhulce
force-pushed
the
lantern_preconnect_opp
branch
from
41d4fca
to
d09956e
Compare
blocked on #5071Relatively simple change, approximates the savings using just the TCP handshake time from lantern. The simulator itself doesn't have a notion of preconnect, so we won't estimate savings on a particular metric and bringing in the whole graph isn't necessary.