1624993 - Expose an API for adding profiler markers from embedders like Fenix

Status: RESOLVED FIXED

(GeckoView :: General, enhancement, P2)

Description

[Markus Stange [:mstange]]

When profiling GeckoView, it would be nice to see some information from embedders like Fenix. For example, when certain stages of startup are reached.

What's the best way to integrate embedder instrumentation into Gecko profiles?

GeckoView could expose an API that calls profiler_add_marker and friends from C++, or Services.profiler.AddMarker from JavaScript.

Comment 1

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

This looks definitely worth to implement. I would like to work on it but would also love some android/java feedback on this. There are 2 options that come to my mind for us to add a marker API.

1. Have a marker API that holds the markers inside the java thread. And collect them with JNI calls at the capture time(similar to profiler java samples).
2. Have a marker API that directly sends the markers to C++ side with JNI whenever we add a marker, without having a storage for markers in the java thread.

The second option can be more complicated because we don't have a "java thread" in the profiler until the capture time. And I'm not sure how expensive the JNI calls are. It can bring some overhead and skew the profile data.

I'm also assuming the only way to send the data to C++ side is JNI. But please correct me if there is a better way.

Comment 2

[James Willcox (:snorp) ([e-mail]) (he/him)]

(In reply to Nazım Can Altınova [:canova][:canaltinova on phabricator] from comment #1)

This looks definitely worth to implement. I would like to work on it but would also love some android/java feedback on this. There are 2 options that come to my mind for us to add a marker API.

1. Have a marker API that holds the markers inside the java thread. And collect them with JNI calls at the capture time([similar to profiler java samples][0]).
2. Have a marker API that directly sends the markers to C++ side with JNI whenever we add a marker, without having a storage for markers in the java thread.

I think we might have one more option, which is to use the stuff in mozglue. We don't have to wait for Gecko to be up in that case, so we can call that any time. I think those functions also chain to the right ones once Gecko is running, but not sure?

The second option can be more complicated because we don't have a "java thread" in the profiler until the [capture time][1]. And I'm not sure how expensive the JNI calls are. It can bring some overhead and skew the profile data.

JNI calls are very fast, but not having any reference to the thread seems like a major problem.

It seems like option 1) may be the easiest to implement today, but we should be able to change that later if we wish, since these details shouldn't be exposed in the public API.

Speaking of the public API, I think it would make sense to put these methods in a ProfilerController (or better name) class hanging off of GeckoRuntime. We have several such examples already, e.g WebExtensionController.

Comment 3

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

(In reply to James Willcox (:snorp) ([e-mail]) (he/him) from comment #2)

I think we might have one more option, which is to use the stuff in mozglue. We don't have to wait for Gecko to be up in that case, so we can call that any time. I think those functions also chain to the right ones once Gecko is running, but not sure?

We can add it to the base profiler (the one in mozglue) as well, but we can't add it only to the base profiler. Because we stop the base profiler once the gecko profiler starts and we don't use it except the startup. But we can definitely add the markers to both base profiler and gecko profiler once we have an implementation. It's mostly copy/paste of the code, since they are mostly duplicated.

JNI calls are very fast, but not having any reference to the thread seems like a major problem.

Great, good to hear!

It seems like option 1) may be the easiest to implement today, but we should be able to change that later if we wish, since these details shouldn't be exposed in the public API.

Yeah, I think that option is better for now too.

Speaking of the public API, I think it would make sense to put these methods in a ProfilerController (or better name) class hanging off of GeckoRuntime. We have several such examples already, e.g WebExtensionController.

Cool. I will take a look at it.

Thanks a lot for the information James! Will try something soon and see if the first option is usable.

Comment 4

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

Attachment: Bug 1624993 - Add PutObjects method inside the ProfilerBuffer class as well. r?gerald

Comment 5

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

Attachment: Bug 1624993 - Add a marker API inside the non-public Android Java sampler. r?julienw!,#geckoview-reviewers

Depends on D79122

Comment 6

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

Attachment: Bug 1624993 - Transfer all the Android markers to platform side while capturing the profile r?gerald!,julienw

Depends on D79123

Comment 7

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

Attachment: Bug 1624993 - Expose a GeckoView API for Gecko Profiler markers r?#geckoview-reviewers!,julienw

Depends on D79124

Comment 8

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

Attachment: Bug 1624993 - Add StoreMarker function to unify the marker insertion points. r?gerald

Depends on D79122

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4ab0751c4913
Add PutObjects method inside the ProfilerBuffer class as well. r=gerald
https://hg.mozilla.org/integration/autoland/rev/ecd086541b97
Add StoreMarker function to unify the marker insertion points. r=gerald
https://hg.mozilla.org/integration/autoland/rev/b6b19313bfb5
Add a marker API inside the non-public Android Java sampler. r=geckoview-reviewers,julienw,esawin
https://hg.mozilla.org/integration/autoland/rev/7556d4f93b29
Transfer all the Android markers to platform side while capturing the profile r=gerald
https://hg.mozilla.org/integration/autoland/rev/cac6157a808b
Expose a GeckoView API for Gecko Profiler markers r=geckoview-reviewers,agi,snorp

Comment 10

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/4ab0751c4913
https://hg.mozilla.org/mozilla-central/rev/ecd086541b97
https://hg.mozilla.org/mozilla-central/rev/b6b19313bfb5
https://hg.mozilla.org/mozilla-central/rev/7556d4f93b29
https://hg.mozilla.org/mozilla-central/rev/cac6157a808b

Comment 11

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

Android-components PR: https://github.com/mozilla-mobile/android-components/pull/7510