profiler icon indicating copy to clipboard operation
profiler copied to clipboard

Load a profile from URL doesn't work for perf script profiles

Open copy opened this issue 3 years ago • 8 comments

Profiles generated using perf script can be opened from the file selection dialog, but loading the same file by URL fails with:

The profile could not be downloaded and decoded. This does not look like a supported file type. 
JSON parsing error: SyntaxError: JSON.parse: unexpected character at line 1 column 1 of the JSON data

copy avatar Dec 20 '21 17:12 copy

Interesting!

Strangely I don't see an obvious difference in the code paths for these 2 cases, so if you can provide a URL where you see the problem so that I can debug, that would be wonderful. Thanks!

(Details: initially I thought this could be because we don't process gzipped payload in the case of loading from a URL, but we actually do it in a different place, so that's not that).

julienw avatar Dec 21 '21 10:12 julienw

Sure. Here is the script.perf file (generated by perf script -i perf.data -F +pid > script.perf): https://copy.sh/script.perf

And here is the profiler.firefox.com url that produces the above error: https://profiler.firefox.com/from-url/https%3A%2F%2Fcopy.sh%2Fscript.perf/calltree/?v=6

copy avatar Dec 21 '21 14:12 copy

Oh I see! When getting a profile from an URL we enforce that it must be a JSON in: https://github.com/firefox-devtools/profiler/blob/2214df8a1b10f434476e963ae40bdb9e15f67b0e/src/actions/receive-profile.js#L1282

And only after that we pass it to the function that may decide if it's a perf profile (unserializeProfileOfArbitraryFormat).

We should probably simplify this. Especially move the gzip handling to https://github.com/firefox-devtools/profiler/blob/2214df8a1b10f434476e963ae40bdb9e15f67b0e/src/profile-logic/process-profile.js#L1529, remove the call to JSON.stringify after fetching, and call unserializeProfileOfArbitraryFormat earlier.

julienw avatar Dec 21 '21 14:12 julienw

Hello. I want to work on this issue if it is still open?

barisconur avatar May 18 '22 03:05 barisconur

@barisconur Yes, this issue is still present.

copy avatar May 18 '22 09:05 copy

Hello. I want to work on this issue if it is still open?

Hey @barisconur, this issue is not trivial (our profile loading mechanism is quite complex -- possibly too complex), but if you want to give it a try that would be nice!

julienw avatar May 20 '22 15:05 julienw

For reference, this minimal patch fixes the issue (but will break other formats):

diff --git a/src/actions/receive-profile.js b/src/actions/receive-profile.js
index 744ccfb0..da8259b9 100644
--- a/src/actions/receive-profile.js
+++ b/src/actions/receive-profile.js
@@ -1224,11 +1224,7 @@ async function _extractProfileOrZipFromResponse(
       // and try to process it as a profile.
       return {
         responseType: 'PROFILE',
-        profile: await _extractJsonFromResponse(
-          response,
-          reportError,
-          contentType
-        ),
+        profile: await response.text()
       };
     default:
       throw assertExhaustiveCheck(contentType);

So now I can write a shell script that runs perf and automatically opens the profile in a local instance of this UI:

#!/bin/bash
set -euo pipefail
perf record -o /tmp/perf.data "$@"
perf script -i /tmp/perf.data -F +pid > ~/www/profiler/dist/test.perf
cd ~/www/profiler/dist
pidof nginx || nginx
xdg-open http://localhost:9000/from-url/%2Ftest.perf

With nginx configured as follows:

    server {
        listen 127.0.0.1:9000;
        server_name  localhost;
        location / {
            root   /home/fabian/www/profiler/dist;
            index  index.html;
            add_header Cache-Control no-cache;
            if_modified_since off;
            expires off;
            etag off;
        }
        error_page  404  =200 /index.html;
    }

Pretty cool!

copy avatar Jul 13 '22 14:07 copy

You can also give this a try: https://github.com/mstange/fxprof-perf-convert It's not finished yet but it should work for most use cases.

mstange avatar Jul 13 '22 14:07 mstange