Display upload progress while importing profile from Firefox/"add-on"

[squelart]

When capturing a profile from Firefox, the profiler.firefox.com page shows a bit of animation and "Importing the profile directly from Firefox...". If that takes more than 30s, another message says "We were unable to connect to the Gecko profiler add-on within thirty seconds. [...] Still waiting..." (Side-issue: we should remove "add-on".)

It would be great to have more information during this process, to inform the user whether nothing at all is happening, or the upload is indeed very slow. A user suffering from Firefox using 100% CPU said they waited for more than ~5 minutes before giving up.

So I think we should at least display a count of bytes uploaded, to show whether data is coming through.

Other improvements:

• Show the current data rate.
• If the file size is known, also show a percentage.
• Show the estimated remaining time.
• Coordinate more with the Firefox profiler (this would require Firefox backend changes as well), to better know what's happening before the data starts streaming. E.g.: Request received, gathering main process data, gathered M/N subprocess profiles so far, zipping, and then uploading (with file size sent before the actual file).
• If "gathered M/N subprocess profiles" doesn't seem to reach M==N after a short while, offer the option to upload the profile as-is. (bug 1648324 will attempt to add an automatic timeout on the backend side.)

┆Issue is synchronized with this Jira Task

[julienw]

Indeed the loading UI isn't very good when it takes a lot of time.

The significant challenge is that we have some steps that are very monolithic and can't be changed easily. Here are the steps:

1. get the gecko profiler promise: https://github.com/firefox-devtools/profiler/blob/9b30de5fcce2ae9cbdbc77b62116006d4d76d974/src/actions/receive-profile.js#L908-L920
2. get the raw data: https://github.com/firefox-devtools/profiler/blob/9b30de5fcce2ae9cbdbc77b62116006d4d76d974/src/actions/receive-profile.js#L802-L803
3. possibly decompress the profile: https://github.com/firefox-devtools/profiler/blob/9b30de5fcce2ae9cbdbc77b62116006d4d76d974/src/actions/receive-profile.js#L785
4. decode the buffer as a string, and parse the raw data into a json: https://github.com/firefox-devtools/profiler/blob/9b30de5fcce2ae9cbdbc77b62116006d4d76d974/src/actions/receive-profile.js#L790-L791
5. upgrade the gecko profile: https://github.com/firefox-devtools/profiler/blob/9b30de5fcce2ae9cbdbc77b62116006d4d76d974/src/profile-logic/process-profile.js#L1190
6. process it to our processed format: (same function as above, but below)

Steps 5 and 6 aren't monolithic and we could imagine to have a progress bar for these. Steps 1-4 are more monolithic though, but we can imagine updating a state between each of them, to display the current step to the user.

In the case of the user as described above, they were stuck at step 1 already, but we don't have more clue about what happened.

[julienw]

The loading screen is displayed from https://github.com/firefox-devtools/profiler/blob/9b30de5fcce2ae9cbdbc77b62116006d4d76d974/src/components/app/ProfileRootMessage.js.

Maybe some contributor could look at this and add states to display them in this component? We could have a state that look like this: { loadingStep: ENUM_VALUE, progress?: <number between 0 and 100> }. Not all loadingStep would have progress. All possible enum values would be defined in a type.

[yozaam]

So instead of a progress bar we need to show 1/6 -> 2/6 -> 3/6 -> 4/6 then it will done? so with the await statement i would send an event to update the loadingStep?

I am very new and would need some hand holding, so if you could guide me I would love to be work on this bug, thank you very much

Step 4 doesn't have an await so how will we show it or are we showing the step that is currently running?

[julienw]

Thanks for your interest, this is very appreciated.

This is how we could do it:

1. Create a new action to trigger the changes. The action would be { action: "CHANGE_LOAD_PROGRESS", loadingStep: XXX, progress: XXX }. This could be done in actions/app. Also types/actions will need to be changed. The new enum for loadingStep will need to be in types/state because we'll use it there too.
2. Create a new state to hold this. Creating a new state is:

• add it to the type in types/state
• create a reducer, probably in reducers/app. The reducer will react to the action above.

3. Dispatch the new action in all places where it makes sense.
4. Then I think the component app/ProfileLoaderAnimation.js would display this information. It would need some knowledge of all possible cases, maybe we can come up with a clever Flow type to make sure of this.
5. Of course, some new tests will need to assert this. This shouldn't be too hard to change existing tests, but maybe we'll need some new ones.

To be honest I think this bug may have too many (small) changes for a newcomer, especially if you don't have experience with react and redux. Maybe you can look at our other issues that are labeled as "good first issue" which are more suitable as a first issue (obviously :) ).

[yozaam]

I'll try my best understanding it, if i cannot by 8th July could you point me elsewhere?

[julienw]

Sure I can assign this task to you for now!

And this is our list of unassigned good first issues.

[yozaam]

Thanks ! , is there a similar feature for another animation ( or another event so i can refer) ? also what is a flow type?

[yozaam]

What will happen to 'WAITING_FOR_PROFILE_FROM_FILE' should i add my action over here? https://github.com/firefox-devtools/profiler/blob/7a159b8435968f92405ff6eb4ce92cffcd2ac5bd/src/types/actions.js#L313-L319

Or am i to make a new type like how we did here https://github.com/firefox-devtools/profiler/blob/7a159b8435968f92405ff6eb4ce92cffcd2ac5bd/src/types/actions.js#L406-L431

my actions/app will get a new function:

export function changeLoadProgress(loadingStep,progress): Action{
  return { 
    type: 'CHANGE_LOAD_PROGRESS',
    loadingStep,
    progress,
  };
}

Also is it okay to comment questions like these on the issue or should i do it someplace else?

[julienw]

Thanks for coming on Matrix, I think this is the best way to discuss. Also whenever you want you can push your code to a pull request so that we can have context about your questions. Note that git push will run all local tests by default, but you can add --no-verify to push without checking locally.

Also we wrote some instructions about working in our project in our contributing documentation.

[yozaam]

This PR gives errors with yarn flow, I was trying to solve it but I wanted to know if this is the way

[yozaam]

I think I did it :D no errors in yarn flow and i've made LoadingStep LoadingState LoadingStateAction

Now i need to work on :

• [ ] reducer,
• [ ] dispatch
• [ ] and then display the information
• [ ] test

[yozaam]

https://github.com/firefox-devtools/profiler/blob/04d81d51ed394827bff9c22e540993abeff1db5e/src/reducers/app.js#L34

https://github.com/firefox-devtools/profiler/blob/04d81d51ed394827bff9c22e540993abeff1db5e/src/reducers/app.js#L62

do i need to make any change to these or ignore it and have a separate reducer ?

[yozaam]

I am very sorry @julienw I cannot do it 😞 , could you point me to another project perhaps having vanilla js, I looked at treeherder

[julienw]

Thanks for reporting though!

[miggs125]

I would like to work in this :)

[informatorius]

Any news on the issue? Is see the same problem on Firefox 85

[julienw]

Any news on the issue? Is see the same problem on Firefox 85

Can you please give some more information?

• frequency that you're seeing this problem
• is it in some situations especially? Which configuration have you used for the profiler?
• would you say you're using a slow computer?

This would help us priorizing this issue.

Thanks!

[informatorius]

I saw it one time. I created a profiler snapshot and later wanted to open it. Firefox was not able to open it. Then I found this bug report.

I used profiler settings Firefox Front-End.

It is a fast computer.

That is the link it saved in my Firefox for the captured profiler. https://profiler.firefox.com/from-addon/calltree/?globalTrackOrder=7-8-0-1-2-3-4-5-6&hiddenGlobalTracks=1-2-3-4-5&hiddenLocalTracksByPid=7764-1-2-3~2156-0&localTrackOrderByPid=7764-0-1-2-3~&thread=0&v=5

It opened fast the first time opened after capture. But later opening this link does not show any profile data but only "Importing the profile directly from Firefox..."

[julienw]

Thanks for the new information. Now I see what's happening, and this is unrelated to this bug. Let me explain:

• when you capture a profile, this opens the profiler UI, with /from-addon/ in the URL as you can see. This is completely local to your computer, and nothing is saved yet. (in the future we'll likely save it in a local DB, but this work hasn't been started yet). If you reload this URL you'll have nothing but this loading URL doing nothing. We should definitely improve that too.
• If you want to save it, you have 2 solutions:
  1. Save it locally. You can do it by clicking the "Upload" button at the top, then use the "download" button (now that I explain this to you, I realize this isn't obvious).
  2. Upload to a server. You can do it by clicking the "Upload" button at the top, then use the "upload" button. Then you'll have a permalink you can reload and share with other people.

Hope this makes sense!

[informatorius]

Yes now I see. To save it locally I have to press the Upload button and then the Download button.

But still if I use only the link from the generated profile then it will do nothing because after browser restart data is lost when I do not save it before. And then the profiler page is shown with "Importing the profile directly from Firefox..." though it does not import anything because it cannot find it anymore. So I expect it tells me: "Nothing found! Did you save your results before?"

[julienw]

So I expect it tells me: "Nothing found! Did you save your results before?"

I agree, we should do that. But this is a different bug that this one. Thanks!

[squelart]

FYI: In the back-end task https://bugzilla.mozilla.org/show_bug.cgi?id=1673513, I'm adding a progress value when gathering profiles from child processes, and it could later be extended to more of the JSON-generating code. For now it's only used by the parent process to know it should still wait for slow child processes to send their profile.

Eventually it would be great if we could also show that progress on the transfer screen, even before the actual upload starts.

This would probably need even more work, including opening the new tab immediately after the user clicks "Capture Profile" or presses ctrl+shift+2 (so that part would be more responsive and informative), so it's just something to think about, whether this could be done as part of this issue here, or in a separate one later on...

[julienw]

This would probably need even more work, including opening the new tab immediately after the user clicks "Capture Profile" or presses ctrl+shift+2 (so that part would be more responsive and informative), so it's just something to think about, whether this could be done as part of this issue here, or in a separate one later on...

This is the code responsible for opening a tab from the popup: https://searchfox.org/mozilla-central/rev/c3cbf7e56630d12443459a6bd7938358c231ce3b/devtools/client/performance-new/popup/background.jsm.js#318-346

I suppose that instead of awaiting the result of capturing the profile, we could instead register the promise and await later, when the page is requesting the profile.

This is the code responsible when capturing from devtools: https://searchfox.org/mozilla-central/rev/c3cbf7e56630d12443459a6bd7938358c231ce3b/devtools/client/performance-new/initializer.js#141-162

I suppose there's some more work for this case, because we need to also change the devtools protocol.

Lastly, another solution would be to report this progress in the Firefox UI rather than in the web page. That could simplify the process a bit by reducing the "information forwarding" code needed.