1545386 - Add support for custom features for Gecko profiler

Status: REOPENED

(Testing :: Raptor, task, P3)

Description

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

It would be nice to allow customization of the Gecko Profiler features via the test manifest or an command line argument. It can be similar to what I currently implement for the to profile threads via bug 1544686.

Comment 2

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

As filed this specific bug is for Raptor / Browsertime. Bug 1702310 is for adding a flag to mach try fuzzy but that won't help when running these tests locally.

Going to reopen for now, but I will leave it to the the Raptor owners to decide if they want such support in Raptor proper or if mach try is sufficient enough for them.

Comment 3

[Steve Fink [:sfink] [:s:]]

Bug 1702310 suffered from a bit of scope creep while I was implementing it. It does add support for local raptor runs, at least as I understand what that means: mach raptor --gecko-profile --gecko-profile-features=nostacksampling,leaf -t imgur --browsertime works.

Comment 4

[Steve Fink [:sfink] [:s:]]

Ugh, accidentally submitted this. I should say, it worked with my manual raptor.py command line, but either that's slightly different or I broke it when updating for review comments, because it's erroring out. But that's just a matter of fixing it before landing; the patch over there does implement this support.

Comment 5

[Steve Fink [:sfink] [:s:]]

(Oh, and I believe setting the options via the manifest should also work post-bug 1702310, though I didn't test that.)

Comment 6

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

I see. So lets mark as dependent for now. This bug can be left for the manifest specific use case if that is wanted. Greg, maybe you can give feedback here and if necessary dupe if not needed/wanted.

Comment 7

[Greg Mierzwinski [:sparky]]

It looks like the patch from bug 1702310 would solve those issues as well. The manifest will be able to use these new features, and the local command line options are also being updated with these.

:sfink, if you want, you could split apart your current patch so we can see these changes better. I don't mind if you leave it as a single patch though, we'll just need to make sure the patch summary mentions that these changes are being made in it.

I agree with :whimboo though, so let's leave it as a dependent bug for now in case the changes in bug 1702310 aren't enough.

Comment 8

[Steve Fink [:sfink] [:s:]]

(In reply to Greg Mierzwinski [:sparky] from comment #7)

It looks like the patch from bug 1702310 would solve those issues as well. The manifest will be able to use these new features, and the local command line options are also being updated with these.

Right, that is my expectation as well.

:sfink, if you want, you could split apart your current patch so we can see these changes better. I don't mind if you leave it as a single patch though, we'll just need to make sure the patch summary mentions that these changes are being made in it.

No no please no. I don't understand the layering of what calls what, and what config data inherits/overrides what. I've done a long series of try pushes where I thought I only had to change one set of places and gradually uncovered more and more things. Splitting it up correctly would require more architectural understanding than I possess.

I added 'raptor' to the summary already. I can mention that it supports manifest entries as well.

I agree with :whimboo though, so let's leave it as a dependent bug for now in case the changes in bug 1702310 aren't enough.

Works for me!