compute pressure: Add ownContributionEstimate [devtools/devtools-frontend : main]
Arnaud Mandy (Gerrit)
Arnaud Mandy added 3 comments![]( "Open in Gerrit")
I couldn't find a very good solution here, and typescript is not my main strength.
I wanted to have a slider that would trigger a CDP call, through emulationModel.
like we did with the pressure state.
When the slider value is changed (here, let s have a 1s gap to avoid updates for each value) then it will trigger an event on EmulationModel and then we call the CDP function.
I couldn't find a proper implementation for the slider. If someone would have a suggestion, I would be happy to hear it.
Thanks
Actually, here I would be interested to have like the "cpu-pressure" state above (line 523), a trigger, that would trigger directly a call on EmulationModel.
How can I do that with a slider? I didn't find any example.
for (const emulationModel of SDK.TargetManager.TargetManager.instance().models(This should be applied only to the current emulationModel, not on all the tabs.
That is also something I didn't find a solution for. Would you have a suggestion?
Related details
- Alex Rudenko
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arnaud Mandy (Gerrit)
Arnaud Mandy has uploaded the change for review![]( "Open in Gerrit")
Arnaud Mandy would like devtools...@chromium.org to review this change.
Commit message
compute pressure: Add ownContributionEstimate
Adding ownContributionEstimate slider and also using
setPressureDataOverride instead of setPressureStateOverride
to pass state and also own contribution estimate value.
Change diff
Change information
- M front_end/core/sdk/EmulationModel.ts
- M front_end/panels/sensors/SensorsView.ts
Related details
- Alex Rudenko
- devtools...@chromium.org
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Rudenko (Gerrit)
Alex Rudenko added 1 comment![]( "Open in Gerrit")
- Arnaud Mandy
- Jack Franklin
- devtools...@chromium.org
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jack Franklin (Gerrit)
Jack Franklin added 1 comment![]( "Open in Gerrit")
Hey, thanks for this CL. Is there a design doc / bug with more context on exactly what this feature is? The attached bug doesn't have much description. I'd like to make sure I understand the feature and why it's needed before reviewing properly. Thanks!
Related details
- Arnaud Mandy
- devtools...@chromium.org
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arnaud Mandy (Gerrit)
Arnaud Mandy added 1 comment![]( "Open in Gerrit")
Hey, thanks for this CL. Is there a design doc / bug with more context on exactly what this feature is? The attached bug doesn't have much description. I'd like to make sure I understand the feature and why it's needed before reviewing properly. Thanks!
Sure, I fully understand!
I think, the only document we have is this:
https://w3c.github.io/compute-pressure/?experimental=1#dfn-owncontributionestimate
https://w3c.github.io/compute-pressure/?experimental=1#update-virtual-pressure-source
I ll try to explain briefly in a document later and share it with you.
Related details
- Arnaud Mandy
- devtools...@chromium.org
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
fabian athayde (Gerrit)
fabian athayde added 1 comment![]( "Open in Gerrit")
Message
On Mon, Jun 2, 2025, 3:17 AM Arnaud Mandy (Gerrit) <noreply-gerritcodereview+kRHynMC6C5486BCA920FQQ==@> wrote: chromium.org
1 comment
On Mon, Jun 2, 2025, 3:17 AM Arnaud Mandy (Gerrit) <noreply-gerritcodereview+kRHynMC6C5486BCA920FQQ==@> wrote: chromium.org
Arnaud Mandy (Gerrit)
Arnaud Mandy added 1 comment![]( "Open in Gerrit")
Arnaud MandyHey, thanks for this CL. Is there a design doc / bug with more context on exactly what this feature is? The attached bug doesn't have much description. I'd like to make sure I understand the feature and why it's needed before reviewing properly. Thanks!
Sure, I fully understand!
I think, the only document we have is this:https://w3c.github.io/compute-pressure/?experimental=1#dfn-owncontributionestimate
https://w3c.github.io/compute-pressure/?experimental=1#update-virtual-pressure-sourceI ll try to explain briefly in a document later and share it with you.
@jacktf...@chromium.org I sent you a document with a short explanation on what we want to achieve/test with devtools for the Compute Pressure feature.
Let me know if you need more info.
Related details
- Arnaud Mandy
- Jack Franklin
- devtools...@chromium.org
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
fabian athayde (Gerrit)
fabian athayde added 1 comment![]( "Open in Gerrit")
Message
On Mon, Jun 2, 2025, 6:24 AM Arnaud Mandy (Gerrit) <noreply-gerritcodereview+kRHynMC6C5486BCA920FQQ==@> wrote: chromium.org
1 comment
On Mon, Jun 2, 2025, 6:24 AM Arnaud Mandy (Gerrit) <noreply-gerritcodereview+kRHynMC6C5486BCA920FQQ==@> wrote: chromium.org
Related details
- Jack Franklin
- devtools...@chromium.org
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arnaud Mandy (Gerrit)
Arnaud Mandy added 2 comments![]( "Open in Gerrit")
I couldn't find a very good solution here, and typescript is not my main strength.
I wanted to have a slider that would trigger a CDP call, through emulationModel.
like we did with the pressure state.When the slider value is changed (here, let s have a 1s gap to avoid updates for each value) then it will trigger an event on EmulationModel and then we call the CDP function.
I couldn't find a proper implementation for the slider. If someone would have a suggestion, I would be happy to hear it.
Thanks
Done
for (const emulationModel of SDK.TargetManager.TargetManager.instance().models(This should be applied only to the current emulationModel, not on all the tabs.
That is also something I didn't find a solution for. Would you have a suggestion?
Done
Related details
- Jack Franklin
- devtools...@chromium.org
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arnaud Mandy (Gerrit)
Arnaud Mandy added 1 comment![]( "Open in Gerrit")
Actually, here I would be interested to have like the "cpu-pressure" state above (line 523), a trigger, that would trigger directly a call on EmulationModel.
How can I do that with a slider? I didn't find any example.
Done
Related details
- Jack Franklin
- devtools...@chromium.org
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arnaud Mandy (Gerrit)
Arnaud Mandy added 1 comment![]( "Open in Gerrit")
@jacktf...@chromium.org, did you get access to the document I shared (by email)
The end of the document describes the usage of compute pressure virtual source on DevTools.
I think all my concerns, have been now fixed. Can you please have a look if it is ok?
Thanks!
Related details
- Jack Franklin
- devtools...@chromium.org
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arnaud Mandy (Gerrit)
Arnaud Mandy added 1 comment![]( "Open in Gerrit")
Related details
- Jack Franklin
- devtools...@chromium.org
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arnaud Mandy (Gerrit)
Arnaud Mandy added 1 comment![]( "Open in Gerrit")
PTAL,
@paul...@chromium.org can you help me with reviewing this patch since you already have reviewed @kenneth.r.c...@intel.com patch on this matter.
https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5816509
Thanks in advance.
Related details
- Jack Franklin
- Paul Irish
- devtools...@chromium.org
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Paul Irish (Gerrit)
Paul Irish added 8 comments![]( "Open in Gerrit")
Cool stuff. A few items below, mostly improving on the UX.
this.#cpuPressureOwnContributionEstimate = 0;just verifying you want the default of the slider to be 0. (fine with me)
based on below, a 0 turns into undefined in the pressureRecord, yeah?
(again, fine with me but .. just making sure its all intended)
forcedSelectedPressureOwnContribution: 'Site contribution to global CPU pressure',maybe lets connect it to the field name.
```suggestion
forcedSelectedPressureOwnContribution: 'Site contribution reported as ownContributionEstimate',
```
private createPressureSection(): void {lets add
.reload-warning {
margin: var(--sys-size-3) var(--sys-size-5);
}to the bottom of sensors.css
component.addEventListener('change', () => {```suggestion
component.addEventListener('input', () => {
```
input event will make this feel a lot more responsive
timerId = setTimeout(() => {good call on debouncing the setting/cdp work!
It kinda makes sense to disable the slider if the main setting is in 'no override'.
something like this will take care of that:
const dropDown = control?.querySelector('select');
function maybeDisableSlider(): void {
const noOverride = dropDown?.value === 'none';
cpuSlider.classList.toggle('disabled', noOverride);
component.disabled = noOverride;
}
maybeDisableSlider();
dropDown?.addEventListener('change', maybeDisableSlider);
plus some styles at the bottom of sensors.css
.own-contribution-estimate input:disabled + span {
opacity: 38%;
}this 38% matches something similar for disabled checkboxes.
const oceSection = this.contentElement.createChild('div', 'own-contribution-estimate');
oceSection.appendChild(control2);
container.appendChild(oceSection);if we skip the extra div
```suggestion
control2.classList.add('own-contribution-estimate');
container.appendChild(control2);
```
and add this to the bottom of sensors.css:
```
.own-contribution-estimate {
margin: var(--sys-size-8) var(--sys-size-10);
gap: var(--sys-size-5);
display: grid;
label {
color: var(--sys-color-token-subtle);
}
}
```itll look a lot better
Related details
- Arnaud Mandy
- Jack Franklin
- devtools...@chromium.org
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arnaud Mandy (Gerrit)
Arnaud Mandy added 10 comments![]( "Open in Gerrit")
Arnaud MandyHey, thanks for this CL. Is there a design doc / bug with more context on exactly what this feature is? The attached bug doesn't have much description. I'd like to make sure I understand the feature and why it's needed before reviewing properly. Thanks!
Arnaud MandySure, I fully understand!
I think, the only document we have is this:https://w3c.github.io/compute-pressure/?experimental=1#dfn-owncontributionestimate
https://w3c.github.io/compute-pressure/?experimental=1#update-virtual-pressure-sourceI ll try to explain briefly in a document later and share it with you.
@jacktf...@chromium.org I sent you a document with a short explanation on what we want to achieve/test with devtools for the Compute Pressure feature.
Let me know if you need more info.
Done
PTAL,
@paul...@chromium.org can you help me with reviewing this patch since you already have reviewed @kenneth.r.c...@intel.com patch on this matter.
https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5816509
Thanks in advance.
Done
@jacktf...@chromium.org, did you get access to the document I shared (by email)
The end of the document describes the usage of compute pressure virtual source on DevTools.
I think all my concerns, have been now fixed. Can you please have a look if it is ok?
Thanks!
Done
Thanks Paul for that thorough review. I appreciate the effort in improving the feature.
I took care of all the comments.
Thanks again
just verifying you want the default of the slider to be 0. (fine with me)
based on below, a 0 turns into undefined in the pressureRecord, yeah?
(again, fine with me but .. just making sure its all intended)
yes.
Having 1% or 0% is about the same, so 0% can be used as undefined
forcedSelectedPressureOwnContribution: 'Site contribution to global CPU pressure',maybe lets connect it to the field name.
```suggestion
forcedSelectedPressureOwnContribution: 'Site contribution reported as ownContributionEstimate',
```
Done
lets add
.reload-warning {
margin: var(--sys-size-3) var(--sys-size-5);
}to the bottom of sensors.css
Done
```suggestion
component.addEventListener('input', () => {
```input event will make this feel a lot more responsive
Done
It kinda makes sense to disable the slider if the main setting is in 'no override'.
something like this will take care of that:
const dropDown = control?.querySelector('select');
function maybeDisableSlider(): void {
const noOverride = dropDown?.value === 'none';
cpuSlider.classList.toggle('disabled', noOverride);
component.disabled = noOverride;
}
maybeDisableSlider();
dropDown?.addEventListener('change', maybeDisableSlider);
plus some styles at the bottom of sensors.css.own-contribution-estimate input:disabled + span {
opacity: 38%;
}this 38% matches something similar for disabled checkboxes.
Done
const oceSection = this.contentElement.createChild('div', 'own-contribution-estimate');
oceSection.appendChild(control2);
container.appendChild(oceSection);if we skip the extra div
```suggestion
control2.classList.add('own-contribution-estimate');
container.appendChild(control2);
```and add this to the bottom of sensors.css:
```
.own-contribution-estimate {
margin: var(--sys-size-8) var(--sys-size-10);
gap: var(--sys-size-5);
display: grid;label {
color: var(--sys-color-token-subtle);
}
}
```itll look a lot better
- Jack Franklin
- Paul Irish
- devtools...@chromium.org
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Paul Irish (Gerrit)
Paul Irish voted Code-Review+1![]( "Open in Gerrit")
- Arnaud Mandy
- Jack Franklin
- devtools...@chromium.org
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arnaud Mandy (Gerrit)
Arnaud Mandy voted and added 1 comment![]( "Open in Gerrit")
Votes added by Arnaud Mandy
| Auto-Submit | +1 |
1 comment
@paul...@chromium.org Do you know anyone else that would be ready to review?
Apparently I need 2 reviews to submit this patch.
Thanks.
Related details
- Jack Franklin
- devtools...@chromium.org
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Paul Irish (Gerrit)
Paul Irish added 1 comment![]( "Open in Gerrit")
- Arnaud Mandy
- Jack Franklin
- devtools...@chromium.org
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Paul Irish (Gerrit)
Arnaud Mandy has uploaded the change for review![]( "Open in Gerrit")
Commit message
compute pressure: Add ownContributionEstimate
This patch is adding a slider to the CPU Pressure section in Sensors
tools.
Screenshot: https://imgur.com/a/s8U10E4.png
The slider controls the ownContributionEstimate parameter.
Specifications:
https://w3c.github.io/compute-pressure/?experimental=1#update-virtual-pressure-source
setPressureDataOverride call is used instead of setPressureStateOverride
to pass state and also own contribution estimate value.
Change diff
Change information
- M front_end/core/sdk/EmulationModel.ts
- M front_end/panels/sensors/SensorsView.ts
- M front_end/panels/sensors/sensors.css
Related details
- Arnaud Mandy
- Jack Franklin
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arnaud Mandy (Gerrit)
Arnaud Mandy added 1 comment![]( "Open in Gerrit")
@paul...@chromium.org Do you know anyone else that would be ready to review?
Apparently I need 2 reviews to submit this patch.
Thanks.
- Jack Franklin
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arnaud Mandy (Gerrit)
Arnaud Mandy added 1 comment![]( "Open in Gerrit")
@mat...@chromium.org PTAL. This was already reviewed by Paul, I just need another +1. Thanks
Related details
- Jack Franklin
- Mathias Bynens
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arnaud Mandy (Gerrit)
Arnaud Mandy added 1 comment![]( "Open in Gerrit")
@alexr...@chromium.org Can you please have a look.
Apparently I m having issue to get another +1, though Paul Irish made a thorough and complete review. Thanks!
Related details
- Alex Rudenko
- Jack Franklin
- Mathias Bynens
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Rudenko (Gerrit)
Alex Rudenko added 3 comments![]( "Open in Gerrit")
async setPressureDataOverride(pressureState: string, ownContributionEstimate: number): Promise<void> {could we add test coverage for these to front_end/core/sdk/EmulationModel.test.ts ?
void this.setPressureDataOverride(settingValue, this.#cpuPressureOwnContributionEstimate);any reason not to await the call here?
const cpuSlider = this.contentElement.createChild('div', 'own-contribution-slider');let's add a e2e test for these to test/e2e_non_hosted/sensors/compute_pressure_test.ts?
Related details
- Arnaud Mandy
- Jack Franklin
- Mathias Bynens
Mathias Bynens (Gerrit)
Mathias Bynens added 1 comment![]( "Open in Gerrit")
(Removing myself as reviewer as I’m OOO)
Related details
- Arnaud Mandy
- Jack Franklin
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Arnaud Mandy (Gerrit)
Arnaud Mandy added 2 comments![]( "Open in Gerrit")
async setPressureDataOverride(pressureState: string, ownContributionEstimate: number): Promise<void> {could we add test coverage for these to front_end/core/sdk/EmulationModel.test.ts ?
Sorry, can you elaborate, since there is no test for any other functions from this file in front_end/core/sdk/EmulationModel.test.ts.
void this.setPressureDataOverride(settingValue, this.#cpuPressureOwnContributionEstimate);any reason not to await the call here?
- Alex Rudenko
- Jack Franklin
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alex Rudenko (Gerrit)
Alex Rudenko added 1 comment![]( "Open in Gerrit")
async setPressureDataOverride(pressureState: string, ownContributionEstimate: number): Promise<void> {Arnaud Mandycould we add test coverage for these to front_end/core/sdk/EmulationModel.test.ts ?
Sorry, can you elaborate, since there is no test for any other functions from this file in front_end/core/sdk/EmulationModel.test.ts.
in front_end/core/sdk/EmulationModel.test.ts there are tests covering the emulate touch logic, could we add similar tests for the new functions?
1) for setPressureDataOverride verify that ownContributionEstimate is computed correctly
2) setPressureOwnContributionEstimate verify that it does not send anything (enable/disable) is the setting is none; Another test could test that the override is enabled if the settings allow that.
Related details
- Arnaud Mandy
- Jack Franklin
Code-Owners
Code-Review
No-Unresolved-Comments
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |