Skip to content
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

State extension for JS Self-Profiling API. #682

Closed
1 task done
cnpsc opened this issue Oct 23, 2021 · 11 comments
Closed
1 task done

State extension for JS Self-Profiling API. #682

cnpsc opened this issue Oct 23, 2021 · 11 comments
Assignees
Labels
Missing: Multi-stakeholder support Lack of multi-stakeholder support Progress: stalled Review type: CG early review An early review of general direction from a Community Group Topic: performance Venue: WICG

Comments

@cnpsc
Copy link

cnpsc commented Oct 23, 2021

Braw mornin' TAG!

I'm requesting a TAG review of State extension for JS Self-Profiling API.

Non javascript execution is hard to identify in traces captured with JS Self-Profiling API:
From a trace we cannot differentiate idle activity from top level UA activity, so for example code that triggers asynchronous rendering work cannot be measured properly.

Further details:

  • I have reviewed the TAG's Web Platform Design Principles
  • The group where the incubation/design work on this is being done (or is intended to be done in the future): WICG
  • The group where standardization of this work is intended to be done ("unknown" if not known): unknown

You should also know that...

We'd prefer the TAG provide feedback as (please delete all but the desired option):

💬 leave review feedback as a comment in this issue and @-notify @cnpsc @acomminos

@torgo
Copy link
Member

torgo commented Dec 7, 2021

Hi folks! Regarding the mitigation against cross-origin leakage that you've described - can you expand on this a little bit and be a bit more clear about what the mitigation strategy is? Are there ways that you could envision this API being used to intentionally and maliciously leak data across origin boundaries? It would be good if you could elaborate on the potential misuse (and mitigations).

@kenchris
Copy link

kenchris commented Dec 7, 2021

I see that you mention this is good for identifying different behavior across UAs, but given that this will mostly be available on the same engine for a long time (Chromium) do you expect developers to correlate this data with other data, e.g. specific OS's, devices etc, like identifying that layout is a big issue in say India as it takes very long on a Moto G5 phone etc?

@plinss plinss modified the milestones: 2021-11-22-week, 2022-01-10-week, 2022-02-14-week Dec 12, 2021
@cynthia cynthia self-assigned this Feb 15, 2022
@torgo
Copy link
Member

torgo commented Feb 15, 2022

Hi @cnpsc we're just coming back to this in our call today. Can you give us an update on progress? We'd like to understand: what is the multi-stakeholder situation (has this been discussed with anyone outside of the Chromium community?) The explainer also does not start from the user need as we've tried to emphasise in our explainer explainer. What user needs is this in service of? Also would this go to Web Performance working group after incubation in WICG? Have you had any discussions with that WG?

@torgo torgo modified the milestones: 2022-02-14-week, 2022-02-21-week Feb 15, 2022
@maxpassion maxpassion self-assigned this Feb 15, 2022
@cnpsc
Copy link
Author

cnpsc commented Feb 17, 2022

Hi @torgo !
We've introduced the feature to the Web Performance working to get initial feedback, we had 2 presentations to the group including one at TPAC 2021. ( https://github.com/WICG/js-self-profiling/blob/main/doc/state-extensions-slides.pdf
https://github.com/WICG/js-self-profiling/blob/main/doc/tpac-2021-slides.pdf)

For the user's need I used the explainer's template and put it in the goals' section https://github.com/WICG/js-self-profiling/blob/main/markers.md#goals
Is this description too low level ? I can add a higher level description of the user's need to that paragraph then. We want to provide web developers with better insights in the browser activity during a profiler trace. Right now they are limited to only the scripting activity we is very limiting.

@torgo
Copy link
Member

torgo commented Feb 17, 2022

Hi @cnpsc yes it's describing the problem from the developer perspective rather than from the end-user perspective. It may seem like make-work but we're really trying to emphasize (in the TAG review process) the idea of end-user-centricity - in other words, that people who are working on new APIs for the web are thinking about new functionality from the end-user perspective. Also remember the explainer is not just for TAG review. For example - what you write in the explainer could help ti inform eventual developer documentation (e.g. on MDN). So yes please if you could start with the (end) user need that would be much appreciated. And thanks for the pointer to the discussion with the Web Perf working group - this is exactly the kind of "trajectory" information we are looking for when we review something in WICG or another community group.

@torgo torgo removed their assignment Feb 22, 2022
@torgo torgo self-assigned this Feb 22, 2022
@torgo torgo modified the milestones: 2022-02-21-week, 2022-02-28-week Feb 22, 2022
@torgo
Copy link
Member

torgo commented Mar 1, 2022

@cnpsc @acomminos any update?

@torgo torgo added the Missing: Multi-stakeholder support Lack of multi-stakeholder support label Mar 24, 2022
@torgo
Copy link
Member

torgo commented Mar 24, 2022

Just taking another look at this at our face-to-face.

I'm still concerned about the multi-stakeholder issue - ChromeStatus shows no additional implementer interest.

From @atanassov : What is the story around user consent – because essentially you are using cycles on the user's device. Should it be gated behind a permission?

From @cynthia : what about main thread CPU starvation caused by a worker or an external process? Also ProfilerTrace and others should be annotated as serializable. Also why a document policy over a permission policy?

@torgo torgo added the Progress: pending external feedback The TAG is waiting on response to comments/questions asked by the TAG during the review label Mar 24, 2022
@cnpsc
Copy link
Author

cnpsc commented Mar 30, 2022

Thanks for the feedback, I tried to address the comments below !

What is the story around user consent – because essentially you are using cycles on the user's device. Should it be gated behind a permission?

For the user consent story, this has been explored in the original tag review for the feature, reasoning is that this is a source of bias in the performance profile and it limits ability to profile first load.
See #366 (comment)

what about main thread CPU starvation caused by a worker or an external process?

External processes are out of scope, the goal is to help developers understand how their own application behave in the wild.
This include hotspots and unecessary code execution wasting the user's CPU cycles.

ProfilerTrace and others should be annotated as serializable.

Thanks adding the annotations !

Also why a document policy over a permission policy?

AFAIK at the time there was no 'disabled by default' support for Feature/Permission policy when this was added to the spec.
Also Document Policy header serves as very early signal to UA's JS engine that the document has intent to profile and start storing the necessary metadata.

@cynthia
Copy link
Member

cynthia commented Apr 5, 2022

Thank you for the response. Just to not lose context here - is there multiple stakeholder interest for this?

External processes are out of scope, the goal is to help developers understand how their own application behave in the wild.
This include hotspots and unecessary code execution wasting the user's CPU cycles.

What about workers? The main concern was about the main thread profile data missing context about high compute pressure, caused either by a worker or an external process. It would be useful if there was a way to share some context when main thread is not getting enough CPU time, as the perf timings for those scenarios would be very different.

@cnpsc
Copy link
Author

cnpsc commented May 2, 2022

Hi @cynthia, this development has been paused to gather more stakeholder feedback.

Thanks !

@cynthia cynthia added Progress: stalled and removed Progress: pending external feedback The TAG is waiting on response to comments/questions asked by the TAG during the review labels May 24, 2022
@cynthia
Copy link
Member

cynthia commented May 24, 2022

Thanks @cnpsc, feel free to request a re-open once you feel like you have reached a point where this warrants a review.

@cynthia cynthia closed this as completed May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Missing: Multi-stakeholder support Lack of multi-stakeholder support Progress: stalled Review type: CG early review An early review of general direction from a Community Group Topic: performance Venue: WICG
Projects
None yet
Development

No branches or pull requests

6 participants