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

Status: RESOLVED FIXED

(Core :: Gecko Profiler, task, P3)

Description

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

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.

Comment 1

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1733335 - Add {add,subtract}_microseconds and Clone implementations to ProfilerTime r?emilio!

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.

Comment 2

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1733335 - Add tracing marker type for the Rust side r?gerald

Depends on D127111

Comment 3

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1733335 - Convert the webrender profiler code to use the new API r?mattwoodrow

Depends on D127112

Comment 4

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1733335 - Change the ProfilerHooks trait to use str instead of CStr r?mattwoodrow

Depends on D127113

Comment 5

[Pulsebot]

Pushed by [e-mail]:
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

Comment 6

[Pulsebot]

Backout by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/abe797b3571a
Backed out 4 changesets for causing build bustages. CLOSED TREE

Comment 7

[Marian-Vasile Laza]

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

Comment 8

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

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.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
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

Comment 10

[Sandor Molnar[:smolnar]]

Backed out for causing mulitple dt failures.

Backout link

Push with failures

Failure log #1

Failure log #2

Failure log #3

Comment 11

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

Attachment: Bug 1733335 - Make std::vector an opaque type in the profiler Rust API r?emilio

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

Comment 12

[Nazım Can Altınova [:canova][:canaltinova on phabricator]]

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.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
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

Comment 14

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/be8872815105
https://hg.mozilla.org/mozilla-central/rev/93abc883209f
https://hg.mozilla.org/mozilla-central/rev/299a058089e7
https://hg.mozilla.org/mozilla-central/rev/8f6e6f2c2cbf
https://hg.mozilla.org/mozilla-central/rev/5640485486b0