-
Notifications
You must be signed in to change notification settings - Fork 9.5k
core(insights): implement network-dependency-tree
insight
#16407
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
Conversation
}; | ||
|
||
interface NetworkTree extends BaseDetails { | ||
type: 'network-tree'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This new detail type is introduced to address the differences in how the network tree is displayed in RPP vs the old CRC audit:
- The CRC audit will show individual request durations but the new NDT insight will show the entire chain duration
- The CRC audit will only show any timing/transfer size info for leaf node requests, but the NDT insight will show this info for all requests
- The CRC audit contains extra request timing info like
responseReceivedTime
but we don't need this for the NDT insight. The NDT insight has enough information to fill in this extra data, but I don't want to add extra calculations just to fill in the CRC detail type.
See report/renderer/crc-details-renderer.js
for more info on how these detail types are handled.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The CRC audit contains extra request timing info like responseReceivedTime but we don't need this for the NDT insight. The NDT insight has enough information to fill in this extra data, but I don't want to add extra calculations just to fill in the CRC detail type.
can we stuff that info into a title tooltip?
@@ -139,26 +137,24 @@ class CriticalRequestChainRenderer { | |||
/** | |||
* Recursively builds a tree from segments. | |||
* @param {DOM} dom | |||
* @param {DocumentFragment} tmpl |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tmpl
and details
are never actually used anywhere, they just get passed through the recursive calls.
* @property {boolean} isLastChild | ||
* @property {boolean} hasChildren | ||
* @property {number} startTime | ||
* @property {number} transferSize |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
startTime
and transferSize
are never actually used anywhere. Instead, createChainNode
will just look at the data in node
.
network-dependency-tree
insightnetwork-dependency-tree
insight
could we un-bold the byte portion of each row? it's the timing that is more important |
Ref #16323
This introduces a new audit detail type
network-tree
which will eventually replace thecriticalrequestchain
. They use most of the same rendering code on the report side, but there are some key differences that made a separate detail type compelling: #16407 (comment)