Massive slowdowns when downloading to samba share
Categories
(Core :: DOM: Core & HTML, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox82 | --- | fixed |
People
(Reporter: lightlike, Assigned: Gijs)
References
(Depends on 1 open bug)
Details
(Keywords: main-thread-io, perf:responsiveness)
Attachments
(2 files)
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:79.0) Gecko/20100101 Firefox/79.0
Steps to reproduce:
Download file to smb-share
(my case Win10 to Debian/OpenMediaVault)
Actual results:
Firefox became very unresponsive. (about 10 seconds to switch tabs)
Websides still worked fine (except for bandwidth problems. But that is on me)
Expected results:
Firefox should continue working fine while the download loads in the background.
Comment 1•4 years ago
|
||
Bugbug thinks this bug should belong to this component, but please revert this change in case of error.
Assignee | ||
Comment 2•4 years ago
|
||
I don't have access to a samba share. Can you create a performance profile using https://profiler.firefox.com/ and share it with us? That'll help pinpoint the problem.
Here you go: https://share.firefox.dev/3aJCd17
I did a profile using the Front-End Settings.
Slowdowns only started after a short while when I started the second download.
Assignee | ||
Comment 4•4 years ago
•
|
||
Thanks for the super quick turnaround, that already helps. Can you elaborate on what steps you're taking to "download file"? Do you use ctrl-s or similar to save a file that's displayed in the browser? Or click a link and "save link as..."? Or something else?
(edit: oops, submitted too early, ctd...)
I am using ctrl+s or right-mouse -> save on a direct video page.
Assignee | ||
Comment 6•4 years ago
|
||
The root cause here seems to be that https://searchfox.org/mozilla-central/rev/6cc48251bb97600fdf11a5b4c5f621bfc8606d55/dom/webbrowserpersist/nsWebBrowserPersist.cpp#915 does main thread IO, and in 8kb chunks to boot, using NSPR APIs... so if you save a big file that's going to hurt. This code is ancient (2002) and hasn't been very well-optimized... :-(
This is worse on network drives like samba, of course, but it's going to be bad everywhere tbh, and we shouldn't be doing it. By now the codebase has primitives to make this a lot easier.
Nathan, do you have suggestions on what the most modern way to do this is? I'm guessing we should be using a background IO queue task thingy, or do it on the socket thread pool or something (but we'll need to make sure tasks get executed in order...).
(Moving to Core :: DOM & HTML because that's where the webbrowserpersist code lives)
Comment 7•4 years ago
|
||
I think this gets into questions about how our networking stack works, which I am relatively ignorant about. We'd need to receive nsIRequestListener
callbacks off the main thread, and we'd need to make sure whatever code is calling that is async...but looking at the stacks in the profile, it looks like this is getting called from UI, so it...needs to be sync? Maybe we could do a better job of continuing to process events while the I/O is being done by doing the I/O somewhere else.
Sending ni? to Valentin to inquire about receiving OnDataAvailable
off the main thread somehow.
Comment 8•4 years ago
|
||
(In reply to lightlike from comment #3)
Here you go: https://share.firefox.dev/3aJCd17
In addition to the issue discussed in comment 6, this profile also shows us spending 4s checking if a file exists. This is caued by this code, and that part is already covered by bug 827010.
Comment 9•4 years ago
|
||
(In reply to Nathan Froyd [:froydnj] from comment #7)
I think this gets into questions about how our networking stack works, which I am relatively ignorant about. We'd need to receive
nsIRequestListener
callbacks off the main thread, and we'd need to make sure whatever code is calling that is async...but looking at the stacks in the profile, it looks like this is getting called from UI, so it...needs to be sync? Maybe we could do a better job of continuing to process events while the I/O is being done by doing the I/O somewhere else.Sending ni? to Valentin to inquire about receiving
OnDataAvailable
off the main thread somehow.
Yes, that's possible using nsIThreadRetargetableRequest
I think the JS issues are separate, and the bulk of the time is spent writing things to disk in nsWebBrowserPersist::OnDataAvailable
Making nsWebBrowserPersist implement nsIThreadRetargetableRequest and retarget data delivery to a background thread should fix the bulk of the delays.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 10•4 years ago
|
||
Assignee | ||
Comment 11•4 years ago
|
||
Depends on D88730
Comment 12•4 years ago
|
||
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/bc244bc15836 move writes from onDataAvailable away from the main thread, r=valentin https://hg.mozilla.org/integration/autoland/rev/577001cf12ad move flushes caused by closing the stream away from the main thread, r=valentin
Comment 13•4 years ago
|
||
Backed out for failures at test_DownloadLegacy.js.
Backout link: https://hg.mozilla.org/integration/autoland/rev/9f97919310d6e65de07de91a3a43d93d4fd18b32
Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=314606256&repo=autoland&lineNumber=5918
Assignee | ||
Comment 14•4 years ago
|
||
Hrmpf, mach try auto
failed me. I'll have to take a look tomorrow.
Updated•4 years ago
|
Comment 15•4 years ago
|
||
Pushed by gijskruitbosch@gmail.com: https://hg.mozilla.org/integration/autoland/rev/1b44712c42dc move writes from onDataAvailable away from the main thread, r=valentin https://hg.mozilla.org/integration/autoland/rev/a8b3c33aee21 move flushes caused by closing the stream away from the main thread, r=valentin
Assignee | ||
Updated•4 years ago
|
Comment 16•4 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/1b44712c42dc
https://hg.mozilla.org/mozilla-central/rev/a8b3c33aee21
Updated•2 years ago
|
Description
•