-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Excessive execution time of getAllTitles() (MacOS) #2
Comments
Hi there! Unfortunately, Apple Script is slow. The execution times on my system are a little better (though tested on virtualbox), but not good at all. I managed to reduce getAllTitles() execution time, but it is not possible for other functions like getAllWindows():
Can you please try this on your system? If possible, open a number of random apps/windows.
Thank you! |
Hi again! I managed to improve performance (by some "non-intuitive" changes and workarounds). On my system, new measures are as follows:
They are high in absolute terms, but I don't think it's possible to improve them much more when using AppleScript (will keep on investigating anyway). getMenu() is specially hard to be improved. If you can test on your system what I suggested in my previous comment, as well as this new version, I would really appreciate it!!! Thanks! |
from pywinctl import *
import AppKit
import subprocess
import timeit
#def getAllTitles(app: AppKit.NSApplication = None) -> List[str]:
def getAllTitles(app: AppKit.NSApplication = None):
"""Returns a list of strings of window titles for all visible windows."""
cmd = ("""'tell application "System Events"
set winNames to {}
repeat with p in every process whose background only is false
repeat with w in every window of p
set end of winNames to name of w
end repeat
end repeat
return winNames
end tell'""")
cmd='osascript -e '+cmd #;print(cmd);1/0
ret = subprocess.check_output(cmd,
shell=True).decode(encoding="utf-8").strip().split(", ")
return ret
start_time = timeit.default_timer()
windows = getAllTitles()
duration = timeit.default_timer() - start_time
print(f'number of windows = {len(windows)}\ngetAllTitles() {duration = }')
Have you try To test your speedy code could you build the wheel (?) as I used Cheers |
Thank you!!! the wheel with new scripts and other modifications should be located at "dist" folder (version 0.0.18). If you get the time to test it, I would really appreciate it a lot! Besides, I will dig into *scpt files (I had not hear from it before). If I am not wrong, it seems to be separate, pre-compiled script files. Perhaps it makes it much more difficult to pack and disttibute the library? We'll see! Thanks for all your help. |
How to used the 0.0.18 wheel (which I have already downloaded)? |
Use pip for that:
Found and tested about scptd!
The result is not very encouraging though (dammit!):
Inside Test.scptd there is exactly the same script code. I can provide you with it if you want to test it on you own system too. Thank you so much again! |
from pywinctl import *
import AppKit
import subprocess
import sysconfig
import timeit
from pywinctl import __version__ as version
print(f'{sysconfig.get_platform()}\nPyWinctl v{version}')
#def getAllTitles(app: AppKit.NSApplication = None) -> List[str]:
def getAllTitles2(app: AppKit.NSApplication = None):
"""Returns a list of strings of window titles for all visible windows."""
cmd = ("""'tell application "System Events"
set winNames to {}
repeat with p in every process whose background only is false
repeat with w in every window of p
set end of winNames to name of w
end repeat
end repeat
return winNames
end tell'""")
cmd='osascript -e '+cmd #;print(cmd);1/0
ret = subprocess.check_output(cmd,
shell=True).decode(encoding="utf-8").strip().split(", ")
return ret
start_time = timeit.default_timer()
windows = getAllTitles2()
duration = timeit.default_timer() - start_time
print(f'windows number = {len(windows)}\n'
f'getAllTitles2() duration = {duration:.3f}')
start_time = timeit.default_timer()
windows = getAllTitles()
duration = timeit.default_timer() - start_time
print(f'windows number = {len(windows)}\n'
f'getAllTitles() duration = {duration:.3f}')
Windows number discrepancy Cheers |
Hi! First off, It seems that the improvement has worked. To be honest, I didn't expect that enormous difference (from 6.17 to 0.085!!!). Happy to see that, anyway. About windows number discrepancy, totally lost. There was a huge difference compared with v0.0.17, but not with getAllTitle2(), which was my first solution approach to improve performance. The difference between getAllTitles2() and getAllTitles() inside v0.0.18 version is basically... none! It's the same code, reordered in a different way (I realized "repeat" is really slow). This is actually the new 0.0.18 script:
Whenever you try it again, please print the list of window names, so we can identify if there is something I am not properly catching. If I am not asking too much, please also include in your measurment tests getAllWindows() and getMenu() methods to check how they behave in "real" environments. Finally, version method included. I am uploading a new version (0.0.19) which already contains this new method (version/getVersion). Thank you SO much! |
Please provide the "real" code you need to test in "real" environment ;-)
Nice, but I suggest
Short private mail follows |
Sure! First, download wheel file and install 0.0.19 version:
And then run this:
Regarding version methods, you mean version() should return "0.0.19" and getVersion() should return "PyWinCtl 0.0.19", right? I have received your mail. Thank you! I see there is an empty ('') window in the list of getAllTitles() which is not present in getAllTitles2()... Extremely weird when comparing both scripts, and it doesn't happen in my virtual machine. I will look into it. |
Found! You won't believe it. It only happens in Yosemite. The solution is as simple as THIS (note the lack of parenthesis):
Well seen!! Thank you! |
When an updated wheel exist let me know |
Found and fixed. Yesterday wasn't a good day for me at all, but this doesn't mean to waste others' time. Sorry for that. Version 0.0.20 is uploaded. Besides, my code was wrong. I was trying to avoid language problems (if I open TextEdit, the window name is "Abrir", which I guess would not work on your system). This is the rigth code to test:
Thank you for your patience, and my apologies again. |
I have to make this change: |
Thank you! Extremely weird that behavior: 6 vs. 19!?!?!?!?! When testing on both Yosemite and Catalina, this is what I get on version 0.0.20:
Do you know if 6 or 19 or other is the right number of open windows on your system when running the test? Could you please email me with the print result for both getAllWindows() and getAllTitles()? Before that, please, donwload wheel again and uninstall/reinstall it on your system. Sorry, but I am not sure if we properly synced since getWindowsWithTitle('test.txt') was already present in my previous comment. In addition to all this fixes, I just uploaded a new wheel which contains one-liner versions of all problemmatic scripts (not for getMenu(), which still I don't know how to improve it), which should be even faster! Of course, no compromise nor urgency. |
com.apple.iCal but other apps runningin MacOS Dock as Firefox, Safari, TextEdit are absent... |
In the end, one-liner scripts are harder than I thought. They are way faster, but they produce very complex data structures which were misleading me. I think I finally catched it with your extremely appreciated help!!! I am filtering for running apps only, this is why those other apps are not included in the ouput. Minimized apps/windows will be included, however. If you run this on a terminal: I'm uploading version 0.0.21 which hopefully solves this. |
|
Getting closer... HAHAHAHAHA! Can you please email me the list of titles the next time you test? I will dig into it anyway. Thanks a lot!!! |
Again, complex structures issues. I even installed PathFinder and BBEdit to test on my own system. I think (hope) it is working OK now. Version 0.0.22 uploaded!!! |
#2 (comment) |
So, is 0.0.22 ok in terms of time and windows number? |
Test:
getAllTitles() duration=6.178130242
The text was updated successfully, but these errors were encountered: