392807 - Severe performance degradation loading file listings

Status: RESOLVED FIXED

(Core :: Networking, defect)

Description

[Boris Zbarsky [:bzbarsky]]

With the checkin for bug 294800, the time required to load the file listing for my home directory went from 4 seconds to 14 seconds (cold load; for a reload it went from 2 to 8 seconds).  At the end of the load there is a fairly long freeze when the browser is completely unresponsive.

There's nothing particularly special about this directory other than having a few hundred files (most of them dotfiles).

Profile shows that we're spending about 21% of the time in layout, 50% running the DOMContentLoaded handler (this is what causes the freeze), 21% constructing frames, 5% starting image loads.  Everything else is pretty minor.

About half or the frame construction time is style resolution, for what it's worth.

At the very least, would it be possible to presort the rows in C++ in the default sort order so we don't have to execute the JS to sort them? Failing that, can we do something about said JS effectively throwing away all layout work done to that point and redoing it?  Perhaps when script is enabled we should default the body to display:none until we've sorted the rows?  That's not as good as presorting in the c++, of course, since a lot of the time is the actual DOM mutation.  But it's something.

Also, I'm seeing a good bit of relayout happening as the images go from alt text to image... I guess there's not much we can do about that.

Comment 1

[Dão Gottwald [:dao]]

(In reply to comment #0)
> At the very least, would it be possible to presort the rows in C++ in the
> default sort order so we don't have to execute the JS to sort them?

It's a stream, isn't it?
Maybe the script could move the rows as they appear in the document.
But the easiest fix would be to not automatically change the original sorting.

Comment 2

[Boris Zbarsky [:bzbarsky]]

> It's a stream, isn't it?

Probably, yes...  You've been in this code more recently than anyone else, though, so don't ask me.  ;)

> But the easiest fix would be to not automatically change the original sorting.

That might be a reasonable approach, yeah...

Comment 3

[Dão Gottwald [:dao]]

Attachment: ditch auto-sorting

Comment 4

[Dão Gottwald [:dao]]

Attachment: ditch auto-sorting

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 278402 [details] [diff] [review]
ditch auto-sorting

Looks good.  r+sr+a=bzbarsky

Comment 6

[Dão Gottwald [:dao]]

It would be good if the sorting and reordering could be sped up too. Unfortunately I don't have a concrete idea how to do this, other than using a XUL tree, which wouldn't be a trivial move at this point.

Comment 7

[Boris Zbarsky [:bzbarsky]]

One approach that might speed up reordering is to, instead of appending the rows to the table one at a time, append them all in the right order to a DocumentFragment and then append the DocumentFragment.  That _might_ reduce the amount of time spent in frame construction.

Note also that even with the sorting disabled by default (by that patch) this is still about 2x slower than it used to be.  At least in part this seems to be due to the way the icons are done: since all the URIs are different, we always miss the image cache, even in cases when we'd end up with the same icon.  As a result, we end up having to load a _lot_ of "different" images.

Comment 8

[Dão Gottwald [:dao]]

(In reply to comment #7)
> One approach that might speed up reordering is to, instead of appending the
> rows to the table one at a time, append them all in the right order to a
> DocumentFragment and then append the DocumentFragment.  That _might_ reduce the
> amount of time spent in frame construction.

That's indeed faster. Actually, it appears to be A LOT faster under certain conditions. I'll attach a second patch.

> Note also that even with the sorting disabled by default (by that patch) this
> is still about 2x slower than it used to be.  At least in part this seems to be
> due to the way the icons are done: since all the URIs are different, we always
> miss the image cache, even in cases when we'd end up with the same icon.  As a
> result, we end up having to load a _lot_ of "different" images.

Ok, 'moz-icon:.zip' then?

Comment 9

[Dão Gottwald [:dao]]

Attachment: spend less time constructing frames

Comment 10

[Dão Gottwald [:dao]]

Comment on attachment 278402 [details] [diff] [review]
ditch auto-sorting

Let's wait with this one.

Comment 11

[Dão Gottwald [:dao]]

Attachment: try to load less icons

Comment 12

[Christian :Biesinger (don't email me, ping me on IRC)]

only specifying the extension will lead to different icons in some cases, compared to specifying the entire file url...

Comment 13

[Boris Zbarsky [:bzbarsky]]

> spend less time constructing frames

This about halves the time spent on the sorting here.  So it still leaves total pageload about 3x slower than it used to be, with a multi-second complete freeze.

For the other patch, biesi's right.  Let me try what impact the patch has, though.

Comment 14

[Boris Zbarsky [:bzbarsky]]

> try to load less icons

This seems to help some with the initial load (as expected), but I'm getting a _lot_ of noise today for some reason, so hard to tell...

For the sorting, I think we should both not sort by default and do the sorting speedup in question.

Comment 15

[Dão Gottwald [:dao]]

(In reply to comment #12)
> only specifying the extension will lead to different icons in some cases,
> compared to specifying the entire file url...

Which cases would that be? I can imagine that chained extensions (.tar.gz) could make a difference, but are there more weird cases?

(In reply to comment #14)
> For the sorting, I think we should both not sort by default and do the sorting
> speedup in question.

Yes, as I wrote, I'd just wait with the first patch -- not saying it shouldn't go in at all.

Comment 16

[Christian :Biesinger (don't email me, ping me on IRC)]

on windows .exe files; on linux files without extension, perhaps also image files (not sure about those). there are probably more cases.

Comment 17

[Boris Zbarsky [:bzbarsky]]

I'm just saying we need the first patch for this to be bearable over here....

Comment 18

[Dão Gottwald [:dao]]

(In reply to comment #16)
> on windows .exe files; on linux files without extension, perhaps also image
> files (not sure about those). there are probably more cases.

You would get generic icons in those cases. That wouldn't be a regression though, as the entire addresses aren't currently used. I think as a trade-off, that's acceptable.

Comment 19

[Christian :Biesinger (don't email me, ping me on IRC)]

(In reply to comment #18)
> You would get generic icons in those cases. That wouldn't be a regression
> though, as the entire addresses aren't currently used. I think as a trade-off,
> that's acceptable.

Well yes, it wouldn't be a regression. It would still be worse than what we could do though.

Comment 20

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 278780 [details] [diff] [review]
spend less time constructing frames

Looks good.

Comment 21

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 278792 [details] [diff] [review]
try to load less icons

Let's try it.

Comment 22

[Nickolay_Ponomarev]

it's not clear to me from skimming this bug what patches need to be checked in and what patches don't. Could you clarify?

Comment 23

[Dão Gottwald [:dao]]

All patches need to be checked in. Do you want me to combine them?

Comment 24

[Nickolay_Ponomarev]

No, they applied on top of each other just fine.

Checking in nsIndexedToHTML.cpp;
/cvsroot/mozilla/netwerk/streamconv/converters/nsIndexedToHTML.cpp,v  <--  nsIndexedToHTML.cpp
new revision: 1.84; previous revision: 1.83
done

Comment 25

[[e-mail]]

(In reply to comment #6)
>It would be good if the sorting and reordering could be sped up too.
>Unfortunately I don't have a concrete idea how to do this, other than using a
>XUL tree, which wouldn't be a trivial move at this point.
We already have this. Start SeaMonkey, open Preferences, Debug, Networking, then select XUL tree-based directory listing.

Comment 26

[Dão Gottwald [:dao]]

(In reply to comment #25)
> We already have this. Start SeaMonkey, open Preferences, Debug, Networking,
> then select XUL tree-based directory listing.

That's a tree, literally; what I meant is just the tree element. The old HTML listing was more or less okay in that it was a flat table, not a tree. I don't think we want to change that.

Comment 27

[Boris Zbarsky [:bzbarsky]]

So... since this got marked fixed (instead of filing and fixing bugs on particular perf issues until this was actually fixed), we need a followup bug on the remaining regression.