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

dynamically use perfetto #1967

Merged
merged 1 commit into from
Sep 16, 2024
Merged

dynamically use perfetto #1967

merged 1 commit into from
Sep 16, 2024

Conversation

s7tya
Copy link
Contributor

@s7tya s7tya commented Aug 22, 2024

Run npm run install-perfetto before running website.
Then, access to the link inside of the compare page detail. ("query trace" button)

@s7tya s7tya force-pushed the integrate-perfetto branch from 0226ade to 3aaebff Compare August 26, 2024 17:42
@s7tya s7tya marked this pull request as ready for review September 2, 2024 07:41
@s7tya s7tya force-pushed the integrate-perfetto branch 2 times, most recently from aea1c24 to 4b87fba Compare September 7, 2024 06:56
Copy link
Contributor

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried using the embedded Perfetto, and it loaded fine, but it also reloads itself every few seconds. We can discuss it at our meeting.

site/frontend/src/components/perfetto-link.vue Outdated Show resolved Hide resolved
site/frontend/src/components/perfetto-link.vue Outdated Show resolved Hide resolved
@s7tya s7tya force-pushed the integrate-perfetto branch from 4b87fba to 54c2704 Compare September 9, 2024 13:04
@s7tya s7tya force-pushed the integrate-perfetto branch from 54c2704 to fbcaef8 Compare September 15, 2024 21:27
@s7tya
Copy link
Contributor Author

s7tya commented Sep 16, 2024

@LalitMaganti
Hello, sorry to bother you.

I’m currently working on embedding Perfetto's UI into perf.rust-lang.org, and it’s almost working perfectly. However, the page keeps reloading every few seconds. The only modification I made was patching the CSP URLs, so I’m unsure why this issue is happening.

Would you happen to have any idea what might be causing this?

@s7tya s7tya force-pushed the integrate-perfetto branch from fbcaef8 to c1ecbf6 Compare September 16, 2024 01:50
@LalitMaganti
Copy link

Never heard someone run into something like that before, that seems very strange.

I'd suggest using the SHA c4577456a1be9d41853015cd194850f972383db7 corresponding to the previous-stable version of the UI. Unlikely that will fix the issue but it will reduce one variable.

Does Devtools give any hint (log messages which might indicate the originator of the reload)? I don't think there's any code in the UI which would try and reload (modulo version updates and plugin reloads) but there may be some adverse interaction with how you're embedding the UI.

@s7tya
Copy link
Contributor Author

s7tya commented Sep 16, 2024

Thank you for the reply! It occurs only if this flag is true.

https://github.com/google/perfetto/blob/56b5e6fdd77f3d86ad60d402197d91bda911b1f4/ui/src/frontend/trace_url_handler.ts#L218

I'll also paste my dev tools console. Though I've added some debug info.

CleanShot 2024-09-15 at 19 18 58

@s7tya
Copy link
Contributor Author

s7tya commented Sep 16, 2024

Also I'll switch to the stable commit you gave. Thanks!

@LalitMaganti
Copy link

Can we just patch that line to return false? That's a bit of an edge case in the fact we use localhost to post files to the UI from the command line. Likely we could fix it upstream but would need some investigation and I don't want to block you on that.

We can open a Perfetto bug upstream to track the root cause.

@s7tya
Copy link
Contributor Author

s7tya commented Sep 16, 2024

Alright. I'll file an issue on upstream and patch that line for now.

@s7tya
Copy link
Contributor Author

s7tya commented Sep 16, 2024

It should work fine now. I'll migrate the details page to Vue.js later and update the "Open in Perfetto" link on that page so it functions the same way as the one on the compare details page.

Copy link
Contributor

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might as well just use a forked version of perfetto to avoid this monkey patching 😆 But let's keep it as it is, if we need to make more changes in the future, we can switch to a fork.

site/frontend/src/pages/compare.ts Outdated Show resolved Hide resolved
site/frontend/src/pages/compare.ts Outdated Show resolved Hide resolved
site/frontend/src/perfetto.ts Outdated Show resolved Hide resolved
@s7tya s7tya force-pushed the integrate-perfetto branch from 27c3aeb to d309b2c Compare September 16, 2024 16:13
Copy link
Contributor

@Kobzol Kobzol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! :)

For onlookers: this PR adds the ability to inline the Perfetto trace visualizer on the website, but it is opt-in, Perfetto needs to be downloaded offline first. It takes too long to build to do it on CI, but I will try to build it manually (once) on the server.

@Kobzol Kobzol merged commit 4866c36 into rust-lang:master Sep 16, 2024
11 checks passed
@s7tya s7tya deleted the integrate-perfetto branch September 16, 2024 16:33
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

Successfully merging this pull request may close these issues.

3 participants