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

tracing support in webrender #4860

Draft
wants to merge 1 commit into
base: 0.65
Choose a base branch
from
Draft

tracing support in webrender #4860

wants to merge 1 commit into from

Conversation

atbrakhi
Copy link
Member

@atbrakhi atbrakhi commented Oct 8, 2024

I recently started testing tracing support in WebRender. While I don't see a Servo-specific #[cfg_attr()] in WebRender, I'm not sure if we'll be able to land it upstream as it is very Servo-specific. However, it seems very useful and helps with some measurements on the Servo side.

Anyway, I'm submitting this early work as a draft PR to start a discussion around the best way forward and whether it's worth trying to land upstream or maybe downstream.

I guess I will extend this work once we have a way forward

image

cc @delan

@mrobinson
Copy link
Member

Yeah, I don't think we want to carry these kind of changes downstream as we are shooting for not having any changes on top of upstream at all.

@nical
Copy link
Contributor

nical commented Oct 8, 2024

Hi,

This is a mirror of WebRender's code. It's fine to open PRs here for discussion but they won't be merged and the actual contribution, when ready, will have to happen in the mozilla-central reprository via phabricator.

The way Firefox integrates its built-in profiler into WebRender is via the ProfilerHooks trait. This avoids having to add a dependency in WebRender itself, and if you can extend and build on top of the same infrastructure it will make it a lot easier to accept the patch.
A big part of the issue is that to accept new crate dependencies in Firefox, a Mozilla employee has to spend quite a bit of time doing a proper audit of all new/updated entries in the dependency tree. From a look at the Cargo.lock changes in this PR, I suspect you are probably going to have a difficult time getting this approach approved.

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