Closed Bug 1171488 Opened 7 years ago Closed 7 years ago

Attach system data to profiler recording


(DevTools :: Performance Tools (Profiler/Timeline), defect)

41 Branch
Not set


(firefox41 affected, firefox43 fixed)

Firefox 43
Tracking Status
firefox41 --- affected
firefox43 --- fixed


(Reporter: jsantell, Assigned: jsantell)




(1 file, 3 obsolete files)

Ccing some Framework peeps here too.

I want platform/version information (build dates/numbers for nightlies) in recordings so if someone sends us a recording profile with some weirdness, we can understand where it came from. Maybe some other info from `about:buildconfig`

* Gecko version
* Platform (b2g, fennec, firefox, etc.)
* Revision/Date of build for non-release
* OS

Bonus points for system information like RAM.

I'm guessing this should be a part of the root actor, and while we probably want to stick with feature detection vs platform detection, this helps in debugging, and will pave the way for platform specific features (like offering different tools for b2g)
A timestamp/date of recording would be useful as well
This information is already available from the device actor. You just need to send it a getDescription request to get all of that and more.
Oh, perfect; thanks Panos!
Depends on: 1172180
Good to get this in sooner than later for compat reasons.
Assignee: nobody → jsantell
Original try looks good, again with more platforms incase there is some scenario where tests fail because system info is not available.
Attachment #8653107 - Flags: review?(vporof) → review+
The SDK uses some XPCOM components that do not exist/work on linux according to try; this wraps them in a catch.
Attachment #8653107 - Attachment is obsolete: true
Attachment #8654651 - Flags: review?(dtownsend)
Comment on attachment 8654651 [details] [diff] [review]

Review of attachment 8654651 [details] [diff] [review]:

::: addon-sdk/source/lib/node/os.js
@@ +16,5 @@
>  const endianness = ((new Uint32Array((new Uint8Array([1,2,3,4])).buffer))[0] === 0x04030201) ? 'LE' : 'BE';
> +XPCOMUtils.defineLazyGetter(this, "oscpu", () => {
> +  try {
> +    return Cc[";1?name=http"].getService(Ci.nsIHttpProtocolHandler).oscpu;

I'd be surprised if this was throwing unless it was happening in an xpcshell test run. I tested on my linux machine and it returns "Linux x86_64" just fine. If you really need to return null from here then please update the use of oscpu later in the code to handle that case somewhat sanely.

@@ +25,5 @@
> +
> +XPCOMUtils.defineLazyGetter(this, "hostname", () => {
> +  try {
> +    // On some platforms (Linux according to try), this service does not exist and fails.
> +    return Cc[";1"].getService(Ci.nsIDNSService).myHostName;

Likewise this is working on my linux box but I can believe that it fails in certain configurations. Looking at node's implementation it seems that hostname will always return a string of some kind so rather than falling back to null I suggest either "" or "localhost"
Attachment #8654651 - Flags: review?(dtownsend) → review-
This'll keep it consistent then atleast. Must be some configurations result in these components not being available, atleast on try.
Attachment #8654651 - Attachment is obsolete: true
Attachment #8655113 - Flags: review?(dtownsend)
Comment on attachment 8655113 [details] [diff] [review]

Review of attachment 8655113 [details] [diff] [review]:

r=me for the os.js change
Attachment #8655113 - Flags: review?(dtownsend) → review+
s/non-e10s/e10s on that last one, sorry
Looks like accessing profile location fails in child process? Easy work around
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Experimentally backed out in on suspicion of being the cause of bug 1203804
Blocks: 1203804
Resolution: FIXED → ---
As the hyperactive pulsebot says, it wasn't at fault, relanded.
No longer blocks: 1203804
hey jordan, i wanted to reland this on fx-team but it cause conflicts in

renamed 1171488 -> 1171488-system-info-in-recordings.patch
applying 1171488-system-info-in-recordings.patch
patching file toolkit/devtools/server/tests/browser/browser_perf-recording-actor-01.js
Hunk #1 FAILED at 43
1 out of 1 hunks FAILED -- saving rejects to file toolkit/devtools/server/tests/browser/browser_perf-recording-actor-01.js.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and refresh 1171488-system-info-in-recordings.patch

so you might want to take a look before relanding
Flags: needinfo?(jsantell)
Rebased, testing on try once more for good measure
Attachment #8655113 - Attachment is obsolete: true
Flags: needinfo?(jsantell)
Attachment #8660048 - Flags: review+
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.