Ticket #480 (closed task: fixed)
Large User Filelist freeze the client
| Reported by: | rat_poison | Owned by: | quinox |
|---|---|---|---|
| Priority: | normal | Milestone: | |
| Component: | nicotine | Version: | |
| Keywords: | 1.2.13 | Cc: |
Description
1.2.13 freezes when browsing users with large (about 100k+ files on my machine)
Attachments
Change History
comment:3 Changed 2 years ago by quinox
Those tips did nothing for our performance.
After timing different parts of the BrowseGetDir? function I came across an if-statement that sucked up CPU but never resolved to True. To be honest I have no clue why it never enters the statement, nor if we really need it. Removing this block of code speeds up a filelist with 7000 dirs and 40.000 files from 35 seconds to 9 seconds, and a 100k files filelist is loaded in 50 seconds (apparently GTK has trouble with so many files at that point) whereas before it would hang N+ indefinitely.
I'm not putting it on the SVN for now since I really have no clue what I was doing. You can download the tweaked userbrowse file and replace your own inside pynicotine/gtkgui . You can use it in combination with 1.2.13 or with SVN, whichever you like.
Please test this file thoroughly and try to find a problem with it. If it seems to work for you okay to I'll change the SVN code.
comment:4 Changed 2 years ago by quinox
After some more testing I found out we did need that code for a select few shares (Windows?).
I managed to rewrite the old code so it's still as fast as when I removed it completely :) Please try r1229 - it's not perfect but it sure is a lot better I think.
Changed 2 years ago by Nick Voronin <elfy.nv@…>
-
attachment
patches.zip
added
patches to speed up share browsing.
Changed 2 years ago by Nick Voronin <elfy.nv@…>
-
attachment
patches.2.zip
added
patches to speed up share browsing.
comment:5 Changed 2 years ago by Nick Voronin <elfy.nv@…>
Sorry for double post.
What's in there? Basically it's custom implementation of gtk TreeModel?, which allows to skip appending whole tree to gtk widget, which is VERY slow, at least on win32, and can take tens of minutes on slow machines.
comment:6 Changed 2 years ago by quinox
neat, you made it yourself I take it?
I'll test it out, and if it works good I'll add it to the SVN (under GPLv3, if you agree) in a few days :)
comment:7 Changed 2 years ago by Nick Voronin <elfy.nv@…>
uglytree.py had a sample from pygtk as a template for custom treemodel, but I think there is nothing left from it save for comment lines describing what handlers are supposed to do. :) Everything else is mine. I hope it will fit.
comment:8 Changed 2 years ago by quinox
Committed your work with r1247. Times it took to load a locally stored filelist:
Original: Duration debianfart: 14.033 Duration ghostop: 4.897 Duration Gmork: 2.736 Duration joeyfalduto: 0.045 Duration koolmeesje: 2.138 Duration kostochka: 1.712 Duration ludo973: 4.544 Duration MissQuianim: 0.053 Duration murdercitydevil: 111.272 Duration @n@uHiR@m: 9.565 Duration pantstx: 11.345 Duration spinX: 9.455 Duration Unsolved Mystery: 4.150 Duration uyrou: 0.638 New: Duration debianfart: 2.453 Duration ghostop: 0.438 Duration Gmork: 1.803 Duration joeyfalduto: 0.045 Duration koolmeesje: 2.035 Duration kostochka: 1.153 Duration ludo973: 2.617 Duration MissQuianim: 0.048 Duration murdercitydevil: 7.261 Duration @n@uHiR@m: 2.700 Duration pantstx: 1.931 Duration spinX: 2.378 Duration Unsolved Mystery: 2.161 Duration uyrou: 1.395
Your code beats the crap out of the GTK one :D
The only modifications I've done: changing a catch-all into a catch-TypeError? + added support for filelists from the harddrive that were still stored in a dictionary.
Keeping ticket open for a bit so people can complain if it doesn't work (so far I've not ran into problems)
comment:9 Changed 2 years ago by Nick Voronin <elfy.nv@…>
Actually, I believe exception is supposed to be IndexError?, happening in GetChild?. It's just that on_get_iter is used with path tuples formed manually, to check if they are indeed valid paths. This exception happens when assigning empty model to the treeview, when code tries to set selection on first (non-existent in empty list) directory.
comment:10 Changed 2 years ago by anonymous
so you're saying the return None should be changed to raise IndexError() inside the GetChild function, and then that one can be caught inside on_get_iter()?
comment:11 Changed 2 years ago by Nick Voronin <elfy.nv@…>
IndexError? is raised in GetChild? on access self.tree2[level][number] given incorrect node as an input. This exception was then caught in on_get_iter to return None to the caller as indication of non-existent path. It's all a bit messy about where it's better to convert exception into a meaningful return value, but initially it worked this way. If changed it should still somehow deal with path (of valid type) addressing non-existing node.
If I understand the meaning of TypeError? exception, I think it may pretty well go uncaught, since it indicates actual error in some code, not some expected try-fail situation.
comment:12 Changed 2 years ago by quinox
I haven't run into any IndexErrors yet. The TypeError comes directly from the code:
Inside GetChild? you sometimes return None:
if self.tree2[level][number][0] == 0:
return None
In on_get_iter you force this output into two variables:
level, number = self.GetChild(node)
A None cannot be split that way resulting in a TypeError. As you said this happens with an empty model. It can be solved by adding an extra check (which eliminated the need for the whole catch block):
def on_get_iter(self, path):
'''returns the node corresponding to the given path.'''
# print "on_get_iter(self, %s):" % (path,)
node = None
for i in path:
returnvalue = self.GetChild(node)
if returnvalue:
level, number = self.GetChild(node)
node = (level, number + i)
return node
While try-catch are cheap with Python the code with check (and without the try-catch) makes it more readable.
Actually I'll change the SVN code to this cleaner version - it works okay with the first 25 filelists I've tried.
comment:13 Changed 2 years ago by Nick Voronin <elfy.nv@…>
Ah. I see now. You are right. Just add 'else: return None' there, so the cycle won't iterate further.
comment:14 Changed 2 years ago by quinox
- Status changed from new to closed
- Resolution set to fixed
I've heard no complaints so far (except for the OSX version which segfaults now, but AFAICS that is python's fault, not ours). Thanks again Nick

the biggest problem seems to be BrowseGetDirs? from userbrowse.py