https://bugs.winehq.org/show_bug.cgi?id=41107
Bug ID: 41107 Summary: Total Commander 8.52a Final has VERY slow folder icons rendering Product: Wine Version: 1.9.16 Hardware: x86-64 OS: Linux Status: UNCONFIRMED Severity: normal Priority: P2 Component: shell32 Assignee: wine-bugs@winehq.org Reporter: katsunori.kumatani@gmail.com Distribution: ---
Created attachment 55313 --> https://bugs.winehq.org/attachment.cgi?id=55313 Terminal Output, not very useful
Using the 32-bit version of Total Commander 8.52a Final (latest non-beta), when drawing/rendering the icons for folders, it is VERY slow. Download the 32-bit version here: http://totalcmd2.s3.amazonaws.com/tcmd852ax32.exe
After installing and launching it, please do the following to reproduce:
1) Maximize the Window to have more folders on screen.
2) Near the top-left, to the right of the green "Refresh" button, click on the "Source: Only file names" button, this enables more folders visible at once to show the slow effect more.
3) Go/navigate to a folder which has around 200 folders, such as Z:\usr\share (depends on your Linux distro), it doesn't matter, just make sure it has a lot of folders to see how painfully slow it is.
This problem of course is not present in Windows.
Terminal output has been attached, however I don't think it has any useful information because [b]Total Commander version 7 exhibits almost the same terminal output, but IT IS NOT SLOW like 8.52a![/b] In fact it is almost instant when rendering folder icons, which look the exact same.
If you turn off folder icons in Preferences, it is fast, so I know that they are the culprit, 100%.
Btw, the version 9 beta does not seem to have this problem because it uses the system's folder icons (which are blue on my system) instead of internal. The internal ones are yellow.
I hope you can track down and fix this bug with this information.
https://bugs.winehq.org/show_bug.cgi?id=41107
katsunori.kumatani@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |performance CC| |katsunori.kumatani@gmail.co | |m Distribution|--- |Mint URL| |http://totalcmd2.s3.amazona | |ws.com/tcmd852ax32.exe
https://bugs.winehq.org/show_bug.cgi?id=41107
katsunori.kumatani@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Keywords| |regression Component|shell32 |-unknown Product|Wine |Wine-staging CC| |erich.e.hoover@wine-staging | |.com, michael@fds-team.de, | |sebastian@fds-team.de
--- Comment #1 from katsunori.kumatani@gmail.com --- I went through the older wine versions I used and it seems this could be a regression.
It is way faster with wine 1.7.42 or wine-staging 1.7.42, so this regression was likely introduced later (I suspect near the beginning of 1.9 or earlier).
Sorry, I didn't test the exact version where the regression happened, and I don't know how to compile wine. Only been using the packages for Ubuntu...
https://bugs.winehq.org/show_bug.cgi?id=41107
--- Comment #2 from Sebastian Lackner sebastian@fds-team.de --- Does the problem also exist in the development version of Wine? If yes, the "Wine" product is more appropriate.
https://bugs.winehq.org/show_bug.cgi?id=41107
--- Comment #3 from katsunori.kumatani@gmail.com --- Yes it does, I selected wine-staging because it's what I was using, I did not know it was supposed to be if it's a fault with it specifically.
I am really sorry for the confusion, I will change my other bug reports to normal Wine as well then.
https://bugs.winehq.org/show_bug.cgi?id=41107
katsunori.kumatani@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Component|-unknown |-unknown Product|Wine-staging |Wine
https://bugs.winehq.org/show_bug.cgi?id=41107
Wylda wylda@volny.cz changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |wylda@volny.cz
--- Comment #4 from Wylda wylda@volny.cz --- I can't confirm that. TC 8.52a 32bit and z:\usr\share\ displays 293 folders immediately. TC's window is maximized.
Tested under: * wine-1.9.16 * wine-1.9.16-161-gd6d0d96 * debian stretch 64bit
As i can't reproduce it, i can't do the regression test for you. Please follow https://wiki.winehq.org/Regression_Testing
https://bugs.winehq.org/show_bug.cgi?id=41107
katsunori.kumatani@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Component|-unknown |-unknown Product|Wine |Wine-staging
--- Comment #5 from katsunori.kumatani@gmail.com --- You are right!
I probably messed up something earlier, but now it's clear. I just tested right now normal wine-devel 1.9.16 package and it is instant (not slow).
So it appears this is indeed a problem with wine-staging specifically!
Here's some more tests I did:
* Normal wine versions are fine. * wine-staging 1.9.1 and up are slow. * wine-staging 1.7.42 is fast (like normal wine).
I'm sorry I couldn't test other versions, these are the only ones I archived myself as I used them. I could not find any old wine-staging versions anywhere, the PPA only has the latest binary version (1.9.16).
The terminal output is the same as staging though...
Anyway I've set back the version to wine-staging since it is indeed affecting only wine-staging, and it seems only fairly recently (not years ago).
https://bugs.winehq.org/show_bug.cgi?id=41107
Wylda wylda@volny.cz changed:
What |Removed |Added ---------------------------------------------------------------------------- Ever confirmed|0 |1 Status|UNCONFIRMED |NEW
--- Comment #6 from Wylda wylda@volny.cz --- Confirming. It is wine-staging related.
Vanilla wine-1.9.15 ~ instant. Wine-staging-1.9.15 ~ takes about 4 seconds (paints folders slowly)
https://bugs.winehq.org/show_bug.cgi?id=41107
--- Comment #7 from Wylda wylda@volny.cz --- Results of regression test: * Wine-staging-1.7.52 ~ fast * Wine-staging-1.7.53 ~ slow
Initially i wanted to make the regression test per patches, but there are tons of them and dependent. Hopefully this will at least narrow the scope of staging's patches.
https://bugs.winehq.org/show_bug.cgi?id=41107
winetest@luukku.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |winetest@luukku.com
--- Comment #8 from winetest@luukku.com --- (In reply to Wylda from comment #7)
Results of regression test:
- Wine-staging-1.7.52 ~ fast
- Wine-staging-1.7.53 ~ slow
Initially i wanted to make the regression test per patches, but there are tons of them and dependent. Hopefully this will at least narrow the scope of staging's patches.
I quess its possible to hunt down the patches that were marked into staging during that time. The amount shouldnt be that huge. Also it's quite possible that staging added some total commander patches. They fixed some time a lot of the known issues.
https://bugs.winehq.org/show_bug.cgi?id=41107
--- Comment #9 from Wylda wylda@volny.cz --- I was wondering, that someone could put here educated guess about a patches to save me some time.
But latter today, i could make manual bisection per re-base commits based on staging's commit history between 1.7.52 and 1.7.53 to pinpoint exact patch set. Or is there a better way in case of staging?
https://bugs.winehq.org/show_bug.cgi?id=41107
--- Comment #10 from Sebastian Lackner sebastian@fds-team.de --- (In reply to Wylda from comment #9)
I was wondering, that someone could put here educated guess about a patches to save me some time.
But latter today, i could make manual bisection per re-base commits based on staging's commit history between 1.7.52 and 1.7.53 to pinpoint exact patch set. Or is there a better way in case of staging?
My best guess would be:
https://github.com/wine-compholio/wine-staging/tree/master/patches/server-Si...
However, there is also an easy way to find the guilty patch using bisect without going back in history. Just check out wine git, then use:
/path/to/wine-staging/patches/patchinstall.sh --backend=git --force-autoconf --all
This will add all patches as separate commits with their corresponding autogenerated changes. Then just do:
git bisect start git bisect good origin git bisect bad
And then continue bisecting as usual. You might have to re-run ./configure if compilation aborts because of Makefile changes.
https://bugs.winehq.org/show_bug.cgi?id=41107
--- Comment #11 from Wylda wylda@volny.cz --- Thanks, this exactly what i wished ;) But could you help me bit more, please?
... to find the guilty patch using bisect without going back in history. Just check out wine git
Do you mean:
wine-git$ git checkout -f wine-1.9.16 staging-git$ git checkout -f v1.9.16
wine-git$ /path/to/wine-staging/patches/patchinstall.sh --backend=git --force-autoconf --all
This will add all patches as separate commits with their corresponding autogenerated changes. Then just do:
git bisect start git bisect good origin git bisect bad
And then continue bisecting as usual. You might have to re-run ./configure if compilation aborts because of Makefile changes.
Above git bisect commands should be in "wine-git" dir or "staging-git" dir?
Why plain "origin" (git bisect good origin) instead of version?
https://bugs.winehq.org/show_bug.cgi?id=41107
--- Comment #12 from Sebastian Lackner sebastian@fds-team.de --- (In reply to Wylda from comment #11)
Thanks, this exactly what i wished ;) But could you help me bit more, please?
Of course, but our discussions are not really related to the bug itself. If you need more help with bisecting, it might be better if you ask for help in the #wine-staging IRC channel or if you contact me privately.
... to find the guilty patch using bisect without going back in history. Just check out wine git
Do you mean:
wine-git$ git checkout -f wine-1.9.16 staging-git$ git checkout -f v1.9.16
wine-git$ /path/to/wine-staging/patches/patchinstall.sh --backend=git --force-autoconf --all
Yes.
This will add all patches as separate commits with their corresponding autogenerated changes. Then just do:
git bisect start git bisect good origin git bisect bad
And then continue bisecting as usual. You might have to re-run ./configure if compilation aborts because of Makefile changes.
Above git bisect commands should be in "wine-git" dir or "staging-git" dir?
In the wine-git dir.
Why plain "origin" (git bisect good origin) instead of version?
If you check out a versioned release it is indeed better to specify the same version number here. I was initially thinking about bisecting with the patches applied on top of the latest upstream commit, but it should also work like you suggested.
https://bugs.winehq.org/show_bug.cgi?id=41107
--- Comment #13 from Wylda wylda@volny.cz ---
However, there is also an easy way to find the guilty patch...
Yes, that's really great. Thanks for sharing this idea.
OK, you already knew it, but as a confirmation here is my result:
Author: Sebastian Lackner sebastian@fds-team.de Date: Mon Oct 19 00:34:01 2015 +0200
server: Do not signal thread until it is really gone.
https://bugs.winehq.org/show_bug.cgi?id=41107
--- Comment #14 from katsunori.kumatani@gmail.com --- Not sure if this will help but I stumbled on probably the same bug applied in a different situation.
If you use Winamp with the Enhancer 0.17 plugin (a really old one), just by having it open, it will slow down certain tasks in every other application on the same WINEPREFIX! For example, enumerating/going through lists in Total Commander (files and folders) gets slowed down by just having Winamp with the Enhancer plugin up (even without playing). This slows down applications in the same prefix that use Windows "trees" too (those lists that you use + sign to open subtrees etc, not sure how they are called) when attempting to go through those trees etc.
The problem goes away if I start Winamp in another WINEPREFIX (thus different wineserver) leading me to believe it's the same bug, since Winamp is affecting the wineserver which waits because of the bug.
Sorry if I sounded dumb, it's probably way over my head to understand why it does that. :)
https://bugs.winehq.org/show_bug.cgi?id=41107
--- Comment #15 from katsunori.kumatani@gmail.com --- Any news for this regression?
I know it's probably hard to fix considering that it's been up for so long after detected with it being a regression, but shouldn't the patch that caused it be reverted? Or is what the patch fixed very important? (forgive me if I ask stupid questions, I'm not familiar with wine-staging's policies)
https://bugs.winehq.org/show_bug.cgi?id=41107
--- Comment #16 from Sebastian Lackner sebastian@fds-team.de --- (In reply to katsunori.kumatani from comment #15)
Any news for this regression?
I know it's probably hard to fix considering that it's been up for so long after detected with it being a regression, but shouldn't the patch that caused it be reverted? Or is what the patch fixed very important? (forgive me if I ask stupid questions, I'm not familiar with wine-staging's policies)
Wine-Staging uses the same policy as Wine, which is that patches should not be reverted or removed unless they are shown to be wrong. Technically the patch makes Wine behave more correct and fixes bugs in other applications. In addition, TotalCommanders thread handling is clearly broken when it spawns million of threads just to enumerate the directory content. The correct way would be to use (or implement) a threadpool, which can deal with lots of small tasks.
After some discussions with Michael Müller I have updated the patch and reduced the timeout interval, which should improve performance a bit. However, of course it is still not as good as before, where waiting for thread exit was broken. In order to improve it even further it will be necessary to come up with a method which is faster, without breaking what the initial patch is supposed to fix.
https://bugs.winehq.org/show_bug.cgi?id=41107
--- Comment #17 from katsunori.kumatani@gmail.com --- I wonder how Windows handles it, then. What exactly does it wait for? I mean, it waits for thread to exit, but can't that be done asynchronously? (for example in a different working thread, or by spawning a waiting thread to wait). In Windows there is zero slowdown so obviously it's not a matter of choosing a timeout interval. If this should get fixed, then clearly a different approach is needed, right?
Just to be sure, it's the wineserver that waits for it right? Is wineserver not allowed to have multiple threads?
https://bugs.winehq.org/show_bug.cgi?id=41107
--- Comment #18 from katsunori.kumatani@gmail.com --- Hi, just tested with wine-staging 1.9.22, it's much faster now, but still noticeable on folders with a large amount of folders inside. This happens only with a lot of folder icons for some reason, any other files and their icons are instant. (i.e disabling icons makes it instant)
I know you reduced the timeout and consequently made it much faster (thank you), but I hope you don't stop thinking about this bug if you can somehow make it better, as it clearly is not supposed to work this way.
For example the other problem with the very old Winamp plugin Enhancer_017 is still there and it's the same bug (it is also faster there, but still noticeable). i.e it slows down other applications that rely on wineserver.
If you use an application with a huge "tree view" list (like Plogue Bidule's parameter list after you inserted a Spectral->Graph) and have Winamp with that plugin open (even in background/minimized and not playing), it will be very slow to select/move through the tree in Bidule (1 sec delay). Of course it's instant in normal wine. It also is instant if you put Winamp in another prefix, because it's a different wineserver.
And it's "faster" with this reduced timeout than it was previously so I'm 100% positive it's the same bug.
Sorry if I was too verbose but I just felt like I had to give some feedback with latest changes. But yeah, stupid bug is still there just less noticeable :(
https://bugs.winehq.org/show_bug.cgi?id=41107
Gabriel Ivăncescu gabrielopcode@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |gabrielopcode@gmail.com
--- Comment #19 from Gabriel Ivăncescu gabrielopcode@gmail.com --- Hi Sebastian,
I want to fix this bug properly, without having this performance problem due to polling.
Do you have some testcases or anything? I mean, I totally understand the patch is proper behavior, and I'm not doubting that. But I need some testcases, or at least a bug report, or something to reproduce, so I can test it when I attempt my fix, i.e. so I don't end up making the patch not do its intended purpose.
Honestly, a test case would also be good so I can try to upstream it to wine-devel, but a bug report should be ok too :-) I mean the patch does fix at least an app, right?
Now onto the fix: I have two ideas. The first one is to use 'waitpid' with WNOHANG in thread_signaled *if* we are polling (so it's non-NULL), in which case we don't have to wait for the result of the polling to give a proper answer. Polling would still be used (but with the original interval if you want, to reduce the overhead) to clean up after the thread as usual. We can't rely on thread_signaled to cleanup by itself (without polling) since that function may not be called early enough and then we may end up with a re-used pid/tid, which is nasty.
The second idea is to spawn a new thread instead of polling. This thread would 'waitpid' on the thread (hanging) and when done, write to a field in the thread's structure indicating it finished, and then cleanup as usual. This is more "elegant" than polling, but spawning a new thread to terminate a thread makes me think it's quite overkill.
Obviously, these are just theories for now since I can only test Total Commander's performance, as I don't know what else this current patch fixes. That's why a testcase or bug report would be very helpful.
https://bugs.winehq.org/show_bug.cgi?id=41107
--- Comment #20 from Gabriel Ivăncescu gabrielopcode@gmail.com --- Nevermind, I forgot that 'waitpid' can't be used on non-children and the thread is not a child of the wineserver... this really complicates things. Blah.
https://bugs.winehq.org/show_bug.cgi?id=41107
--- Comment #21 from Gabriel Ivăncescu gabrielopcode@gmail.com --- Created attachment 62786 --> https://bugs.winehq.org/attachment.cgi?id=62786 server: Do not signal violently terminated threads until they are really gone
Here's a fix as suggested by Sebastian: only wait if the thread was terminated violently, since that's when the risk of race condition exists (the thread is executing arbitrary code).
I sent it to wine-devel but I guess it's not going to get accepted so easily. At the very least, it should be updated in wine-staging, so this bug can be squashed.
https://bugs.winehq.org/show_bug.cgi?id=41107
Gabriel Ivăncescu gabrielopcode@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- CC| |leslie_alistair@hotmail.com
--- Comment #22 from Gabriel Ivăncescu gabrielopcode@gmail.com --- Alistair, can you update the patch in wine-staging with the one attached? (server-Signal_Thread)
At least until (if ever! :-) heh) it gets accepted upstream, so this bug can be marked fixed.
https://bugs.winehq.org/show_bug.cgi?id=41107
Gabriel Ivăncescu gabrielopcode@gmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Resolution|--- |FIXED Fixed by SHA1| |73c60b98968269bbd254b6b27e8 | |5519388badc5b Status|NEW |RESOLVED
--- Comment #23 from Gabriel Ivăncescu gabrielopcode@gmail.com --- Fixed with the updated patch in wine-staging.
https://bugs.winehq.org/show_bug.cgi?id=41107
Alistair Leslie-Hughes leslie_alistair@hotmail.com changed:
What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |CLOSED
--- Comment #24 from Alistair Leslie-Hughes leslie_alistair@hotmail.com --- Closing Fixed Staging bugs.
-
wine-bugs＠winehq.org