Reduce overhead of adding client hints [chromium/src : main]
Patrick Meenan (Gerrit)
Patrick Meenan added 1 comment![]( "Open in Gerrit")
yoav@ could you PTAL when you get a chance since client hints is something you've been heavily involved in?
Related details
- Yoav Weiss (@Shopify)
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
AI Code Reviewer (Gerrit)
AI Code Reviewer added 1 comment![]( "Open in Gerrit")
// https://crbug.com/512514655nit: Blink Style Guide: Naming - Precede boolean values with words like βisβ and βdidβ. Consider renaming this member variable to 'is_fast_memory_cache_with_devtools_enabled_'.
To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **OK But Won't Fix**: reason | **Later**: b/<bug_id> | **Invalid:** reason
_This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent)._
_AI reviews can sometimes be inaccurate; We appreciate your π feedback π to help us improve._
_[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
Related details
- Yoav Weiss (@Shopify)
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Yoav Weiss (@Shopify) (Gerrit)
Yoav Weiss (@Shopify) voted and added 1 comment![]( "Open in Gerrit")
Votes added by Yoav Weiss (@Shopify)
| Code-Review | +1 |
1 comment
- Patrick Meenan
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Patrick Meenan (Gerrit)
Patrick Meenan added 1 comment![]( "Open in Gerrit")
nit: Blink Style Guide: Naming - Precede boolean values with words like βisβ and βdidβ. Consider renaming this member variable to 'is_fast_memory_cache_with_devtools_enabled_'.
To keep this interaction as brief and non-intrusive as possible, please consider responding with one of following options:
**Done** | **OK But Won't Fix**: reason | **Later**: b/<bug_id> | **Invalid:** reason
_This comment was generated by [Experimental Blink C++ Code Review Agent](http://go/blink-c++-code-review-agent)._
_AI reviews can sometimes be inaccurate; We appreciate your π feedback π to help us improve._
_[File a bug](http://go/blink-c++-code-review-agent-feedback) | [Provide feedback on chat](https://chat.google.com/room/AAQA0zhQHe0?cls=4) | [Opt-out](https://ganpati2.corp.google.com/group/peep-genai-blink-agent-optout.prod)_
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Patrick Meenan (Gerrit)
Patrick Meenan voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
1 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: third_party/blink/renderer/core/loader/frame_fetch_context.cc
Insertions: 2, Deletions: 2.
The diff is too large to show. Please review the diff.
```
```
The name of the file: third_party/blink/renderer/core/loader/frame_fetch_context.h
Insertions: 1, Deletions: 1.
The diff is too large to show. Please review the diff.
```
Change information
Reduce overhead of adding client hints
This is work targeted at reducing the renderer main thread overhead of
resource loading. Specifically when loading resources from memory cache
in the context of something like Speedometer 3's NewsSite stories.
~25% of the time spent in resource loading in the benchmark is from
AddClientHintsIfNecessary but it's largely a tooling issue because
chromedriver is used to automate the testing through devtools and
devtools triggers the resource loader to manually add headers to the
request that it doesn't otherwise use.
The main improvement here is in more accurate performance testing when
webdriver/chromedriver/devtools are being used and resources come
from the memory cache. We no longer explicitly set the
"UpgradeForLoader" bit just because devtools is attached (it will
still do it for all requests that miss the memory cache). That alone
eliminated ~30% of the resource request time on the main renderer
thread.
The CL also includes a couple of improvements to the underlying
client hints processing for when it does run:
- The is_1p_origin flag was not used by ShouldSendClientHint() so
it was removed and no longer needed to be calculated.
- The UA platform string is now cached on only changed when the UA
changes (similar to the other static headers).
- The client hint preferences are returned by reference instead of
by value.
- M third_party/blink/renderer/core/loader/frame_fetch_context.cc
- M third_party/blink/renderer/core/loader/frame_fetch_context.h
Code-Review: +1 by Yoav Weiss (@Shopify)
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |