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

Re-evaluate whether we need the "implementation" concept, or if subcategories are enough #3713

Open
mstange opened this issue Dec 4, 2021 · 8 comments

Comments

@mstange
Copy link
Contributor

mstange commented Dec 4, 2021

Long ago, we added "implementation" information to our UI so that we could differentiate between JS code that was running in a JavaScript JIT, JS code that was running in the interepreter, and non-JS code.
This was before we had subcategories.

Now we have subcategories, which carry much of the same information. We should re-evaluate whether we still need the implementation info, and maybe remove it entirely. Both from the UI and from the profile data.

┆Issue is synchronized with this Jira Task

@mstange
Copy link
Contributor Author

mstange commented Dec 4, 2021

Example profile: https://share.firefox.dev/3dlkdfI

Screen Shot 2021-12-03 at 8 26 49 PM

@mstange
Copy link
Contributor Author

mstange commented Dec 4, 2021

My inclination is that the implementation should be removed. It can often be wrong. We were using it because we didn't have the more precise subcategory data at the time.

As an example of a stack where the two disagree, here's a stack from the above profile where the implementation says "JS JIT (blinterp)", but the subcategory says "Parsing": https://share.firefox.dev/3DnkyJp
The subcategory is right.

I'll ask denispal and jandem for feedback next week.

@gregtatum
Copy link
Member

It would definitely simplify the (um err) implementation of the code and the profile processing pipeline, then you could also lean into to making the categories more useful.

@mstange
Copy link
Contributor Author

mstange commented Dec 6, 2021

I found #3572 which is at odds with the idea of removing the implementation entirely.
Maybe we could, for JS functions only, expose that function's implementation. And then not display any implementation-related information on non-JS functions. However, the implementation of a single JS function can change over time; it's a property of the frame, and multiple frames may have collapsed into the same function. So at that point you'd need some kind of breakdown again.

@mstange
Copy link
Contributor Author

mstange commented Jan 20, 2022

I found #3572 which is at odds with the idea of removing the implementation entirely.

Actually, I think this subcategories could also solve the flame graph tooltip use case.

@moztcampbell, @dpalmeiro, what are your opinions on removing "Implementation" and just using the subcategories to differentiate the various JIT tiers?

@parttimenerd
Copy link
Contributor

A lesser change would be to implement a flag in the profile to disable it (#3709), this would be beneficial to external tools.

@mstange
Copy link
Contributor Author

mstange commented Aug 20, 2022

Yes, once this implementation lands, let's discuss what to do for Firefox in #3709. My opinion is that we should just always display the category tooltip and completely remove anything related to the implementation, but we can do this independently of this PR.

@mstange
Copy link
Contributor Author

mstange commented Apr 26, 2024

My opinion is that we should just always display the category tooltip and completely remove anything related to the implementation, but we can do this independently of this PR.

The tooltip part has now been done in #4966. We still have code to display "implementation" things in other places, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants