Closed Bug 1733335 Opened 2 years ago Closed 2 years ago

Convert the old marker related code in Rust to use new profiler rust marker API

Categories

(Core :: Gecko Profiler, task, P3)

task

Tracking

()

RESOLVED FIXED
95 Branch
Tracking Status
firefox94 --- wontfix
firefox95 --- fixed

People

(Reporter: canova, Assigned: canova)

References

Details

Attachments

(5 files)

We have some hacks around rust code to add some markers for a while. Since Bug 1654413, we have a canonical marker API for this instead. We should remove the old code and replace it with our new API instead.

Clone will make the ProfilerTime easier to use. And Add/Sub implementations are
helpful when you need to subtract/add some duration before adding a marker.
There is a similar code in the Webrender marker code, and this will allow them
to use the new API instead of some custom code.

Attachment #9243707 - Attachment description: Bug 1733335 - Add Add, Sub and Clone implementations to ProfilerTime r?emilio!,gerald! → Bug 1733335 - Add {add,subtract}_microseconds and Clone implementations to ProfilerTime r?emilio!
Pushed by canaltinova@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/b919d9cfb8e5
Add {add,subtract}_microseconds and Clone implementations to ProfilerTime r=emilio
https://hg.mozilla.org/integration/autoland/rev/cf3b6b6cc6c6
Add tracing marker type for the Rust side r=gerald
https://hg.mozilla.org/integration/autoland/rev/41c62acfb5a6
Convert the webrender profiler code to use the new API r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/45a3ae02f837
Change the ProfilerHooks trait to use str instead of CStr r=mattwoodrow
Backout by mlaza@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/abe797b3571a
Backed out 4 changesets for causing build bustages. CLOSED TREE

Backed out 4 changesets (Bug 1733335) for causing build bustages.
Backout link
Push with failures - wrench
Failure Log

Flags: needinfo?(canaltinova)

I delayed landing this for one day because this was touching the same places with Bug 1690619. It's merged to central now. I will land it after fixing the issues and rebasing on top of the central.

Flags: needinfo?(canaltinova)
Pushed by canaltinova@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/4cd1efb42295
Add {add,subtract}_microseconds and Clone implementations to ProfilerTime r=emilio
https://hg.mozilla.org/integration/autoland/rev/c44e1bb6d9ff
Add tracing marker type for the Rust side r=gerald
https://hg.mozilla.org/integration/autoland/rev/066819ce0e86
Convert the webrender profiler code to use the new API r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/8c14ac28d042
Change the ProfilerHooks trait to use str instead of CStr r=mattwoodrow

Backed out for causing mulitple dt failures.

Backout link

Push with failures

Failure log #1

Failure log #2

Failure log #3

Flags: needinfo?(canaltinova)

This was causing asan and dt failures because bindgen was computing the size of
MarkerSchema as 122 bytes instead of 144 bytes. This was causing a heap buffer
overflow. After making the std::vector an opaque type, it computes the size
properly as 144 bytes.

Depends on D127114

There was a problem with the MarkerSchema and bindgen wasn't computing its size because it has an std::vector in it. After making the std::vector an opaque type in the Rust side, the problem is resolved. This new patch should fix this.

Flags: needinfo?(canaltinova)
Pushed by canaltinova@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/be8872815105
Add {add,subtract}_microseconds and Clone implementations to ProfilerTime r=emilio
https://hg.mozilla.org/integration/autoland/rev/93abc883209f
Add tracing marker type for the Rust side r=gerald
https://hg.mozilla.org/integration/autoland/rev/299a058089e7
Convert the webrender profiler code to use the new API r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/8f6e6f2c2cbf
Change the ProfilerHooks trait to use str instead of CStr r=mattwoodrow
https://hg.mozilla.org/integration/autoland/rev/5640485486b0
Make std::vector an opaque type in the profiler Rust API r=emilio
You need to log in before you can comment on or make changes to this bug.