core(critical-requests): refactor to use lantern graph #11533
Conversation
|
@adamraine FYI I requested you too because @connorjclark wrote significant portions of this. |
| // By default `traverse` will discover nodes in BFS-order regardless of dependencies, but | ||
| // here we need traversal in a topological sort order. We'll visit a node only when its | ||
| // dependencies have been met. | ||
| const seenNodes = new Set(); |
There was a problem hiding this comment.
Maybe we should pull this into graph, and make a topologicalTraverse ?
There was a problem hiding this comment.
Are you thinking there's another use for this? If we plan on using it again anywhere, sounds good :)
There was a problem hiding this comment.
I don't know yet, but it does seem like a basic functionality of a graph, especially one representing nodes with logical dependencies.
There was a problem hiding this comment.
I guess my hesitation is that this is the first time we've needed topological sort traversal in ~3 years of working with our graphs, so I'm skeptical it will come up again soon enough to justify the preemptive DRY work (and send this logic further away if we need to modify it in the next immediate step of the CRC refactor).
| @@ -5587,31 +5587,31 @@ | |||
| }, | |||
| { | |||
| "startTime": 0, | |||
| "name": "lh:audit:redirects", | |||
| "name": "lh:computed:PageDependencyGraph", | |||
There was a problem hiding this comment.
all that's happening here is PageDependencyGraph is moving before the others now that we request it as part of critical-request-chains
Co-authored-by: Adam Raine <6752989+adamraine@users.noreply.github.com>
Summary
Builds off the work of @connorjclark in #11410. Refactors critical request chains computed artifact to make eliminating the
isCriticalstep much easier.This phase should largely be a noop except eliminating a few corner cases that aren't possible with a valid graph (like a load without a network request).
Related Issues/PRs
ref #11408 (but doesn't close yet)