1552845 - Provide an API for saving profiles via nsIProfiler

Status: RESOLVED FIXED

(WebExtensions :: Experiments, enhancement, P1)

Description

[Henrik Skupin [:whimboo][⌚️UTC+1]]

With bug 1326572 we got an API for the nsIProfiler. It's used in the Raptor webextension and works great, beside the fact that for huge profiles it is barely usable. Here I speak from data of ~100MB and larger.

Having to request the profile via getProfileAsArrayBuffer() first, and then transfer it via a socket connection to a client outside of Firefox, which can actually store the data to disk is a very long task.

As such would it be possible to offer an API for dumpProfileToFileAsync(), which would let the profiler directly write the profile to disk? If allowing a path as argument is too risky, saving it in TEMP might work?

Who would have to accept that proposal? And once done how does the implementation process look like?

Doug, maybe you can give some advises and hints?

Comment 1

[Alex Thayer [:alexical] (she/her)]

I can only comment on the implementation, which I think would be straightforward. However, I have been away from this for a while. aswan might be more help here WRT approval. Andrew, as this is an internal API controlled by the pref extensions.geckoProfiler.acceptedExtensionIds, do you know who would need to approve / give guidance on the security of this API accepting a path to write a profile to?

Comment 2

[Andrew Swan [:aswan]]

Since we're the only users of this api, we don't need to go through any formal process. Adding mconca just so he's aware though.

One challenge is that extensions current don't have direct filesystem access, so there's not really a sane way to generate a useful path. Bringing up a file picker or writing to the default downloads directory would be simple enough if either of those works...

Comment 3

[Henrik Skupin [:whimboo][⌚️UTC+1]]

For automation a default path which gets selected by the profiler sounds fine. Could someone please add some links to where the existing code of the extension API is located? I only found the schema so far, but no actual implementation.

I happily would try to get this implemented.

Comment 4

[Luca Greco [:rpl] [:luca] [:lgreco]]

(In reply to Henrik Skupin (:whimboo) [⌚️UTC+2] from comment #3)

For automation a default path which gets selected by the profiler sounds fine. Could someone please add some links to where the existing code of the extension API is located? I only found the schema so far, but no actual implementation.

I happily would try to get this implemented.

Sure, the API implementation is here:

• https://searchfox.org/mozilla-central/source/toolkit/components/extensions/parent/ext-geckoProfiler.js

and here are the xpcshell tests:

• https://searchfox.org/mozilla-central/search?q=&path=toolkit%2Fcomponents%2Fextensions%2Ftest%2Fxpcshell%2Ftest_ext_geckoProfiler_

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+1]]

(In reply to Andrew Swan [:aswan] from comment #2)

One challenge is that extensions current don't have direct filesystem access, so there's not really a sane way to generate a useful path. Bringing up a file picker or writing to the default downloads directory would be simple enough if either of those works...

The extension itself won't need file system access. It only triggers the dumpProfileToFileAsync() method, and Firefox itself will write the data to the specified location.

In regards of a sane default location I would propose the following:

1. Any exported profile should end-up in the Firefox user profile, as best below a profiler sub directory.
2. The extension method requires to pass-in a file name, which leafName can then be used to construct the final file path.
3. The extension method uses await internally, and returns a Promise which resolves to the final file path.

With option 2) any consumer of the API could implement there own logic for requesting a file name from the user if necessary. Also we make sure that users will never be able to store anything on an arbitrary disk location.

I hope that the above sounds fine to you. There is already code running perfectly fine for me locally.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=77d7ef0e3c8c2ed24b8989e8b353d8a3de02b241

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Bug 1552845 - Provide an API for saving profiles via nsIProfiler. r=aswan

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

By using OS.Path.join() for relative paths I was able to fix the failures locally. Here another try build:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=b3209fe2343677abc9baacb527efce773d57d274

Comment 9

[Henrik Skupin [:whimboo][⌚️UTC+1]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=ba90e8e39c557e4cf92e2c01e5df518c3735bcbb

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: temporary commit until Bug 1552845 has been fixed.

Comment 11

[Philipp Kewisch [:Fallen] [:📆][:🧩]]

Hey Kris, do you still have cycles to review this? Maybe zombie can take over if you need to focus on other things?

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1da1824d64a6
Provide an API for saving profiles via nsIProfiler. r=aswan

Comment 13

[Alexandru Michis [:malexandru]]

https://hg.mozilla.org/mozilla-central/rev/1da1824d64a6

Comment 14

[Madalin Cotetiu]

Can you please provide some steps so we can manually verify this issue? If no manual testing is needed can you please mark it as "qe-verify- "

Comment 15

[Andrew Swan [:aswan]]

This has good automated test coverage, no manual verification needed.