-
Notifications
You must be signed in to change notification settings - Fork 20
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
Addon is slow on Raspberry Pi #169
Comments
I've noticed that the more recent versions seem to be slower. I can't tell when it changed, and I haven't benchmarked it. Running on a Vero 4k, should be quite powerfull compared to a rpi. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
No, v1.7.1 should work again now that the urllib3 change that broke our addon is fixed again. |
This comment has been minimized.
This comment has been minimized.
Alright, I have been profiling our code, and focussing on the A-Z listing. My findings:
Everything else was either part of the above, or negligible. Which brings me back to my feeling that the problem lies outside of our addon (either building up the python call, or processing the result). Although other addons do not seem to be impacted by this (as their menu listings are quite fast on the same system). If you like to experiment, you have to make the following modification to addon.py: import cProfile
if __name__ == '__main__':
cProfile.run('router(sys.argv[2][1:])', '/tmp/profiling_stats') And then you can read out the profiling information using: #!/usr/bin/python
import pstats
p = pstats.Stats('/tmp/profiling_stats')
p.sort_stats('name').print_stats()
p.sort_stats('cumulative').print_stats(20) PS My metrics above are based on a heavily dumbed down version of our code with everything possible disabled while still having a functional addon. |
And I just tested version 1.7.0 (verbatim) which suffers from the same problem. It's not just the A-Z listing, or the Live TV menu, even the main menu is slow when the addon is restart from scratch. Unlike some other addons (like TED Talks) are fast, but some addons (Retrospect, FOSDEM) seem to suffer from this as well. (Subsequent runs are fast, but the original menu is dead slow). I blame Kodi, and I bet I remember the speed from Kodi 17 or some external factor (SD card speed?) |
Based on syscall tracing I can see this:
Process 864 was started around 00:31:46, but the actual request only happened (in this case) at 00:31:51. So we spend 5 seconds doing nothing. Tracing this process revealed what I had seen before, python is looking every library in a myriad of (wrong) places before finding it in the right place. (it first tries our addon paths, and every Kodi module path, before looking into its own path). In itself this is the correct behaviour, but it happens for every internal library. 6 seconds in total for that run. |
I investigated this issue also. I found out that external python modules, for instance Please report your findings on menu loading times with these changes. |
@mediaminister I am interested in this too, but I expected such reorganization be part of one where we also include the new routing module and change the interface (this WILL break existing favourites and what has been watched). So I was planning something like this for v2.0, and first finish the features on my TODO list. (2 items left: Channels section and EPG information in Live streams) The things I wanted to reorganize for v2.0:
The aim would be that for specific menu's or functionality we only call the essential stuff (a bit like what you're doing by moving imports into function calls, but I had hoped we didn't needed that). |
I just tested your new code, and I do see vast improvements to render the main menu and the live TV menu. But the A-Z menu, or Categories menu are as slow as before. I also expected based on this that the TV Guide menu (at least the 2 first levels) would be fast, given that it doesn't require any lookups, but it was also slow. As is playing any content. Personally, I think this requires fixing at the Kodi level. Potentially this is a Python 2 vs Python 3 thing (and may be why my Windows system did not have any issues). It should work the same as on Windows. |
Try this older Jarvis requests packages: It speeds up every menu for me, not instant, but everything within 2 seconds. Relevant information:
Importing requests takes more than 4 seconds on Raspberry Pi :-( kodi03:~ # export PYTHONPATH=/storage/.kodi/addons/script.module.chardet/lib:/storage/.kodi/addons/script.module.requests/lib:/storage/.kodi/addons/script.module.urllib3
/lib:/storage/.kodi/addons/script.module.certifi/lib:/storage/.kodi/addons/script.module.idna/lib
kodi03:~ # time python -c 'import requests'
real 0m 4.33s
user 0m 4.17s
sys 0m 0.11s When profiling, I find this:
|
I wrote #173 which replaces requests with urllib2 and this works fine on Python 2 if you don't need proxy support. Speeds up menus drastically on Raspberry Pi. Please test. |
I think it's indeed a good idea to reconsider which python libraries we use. |
Well, I still think we need to fix this issue properly on the Raspberry Pi because requests is a much better library for the things we are doing (cookie-handling, proxy support, etc.) and we are not the only ones affected. Almost every module is affected. |
It looks like I have the same issue on my raspberry pi3 with libreelec. v1.7 was the fastest one I've run.
I'll try that one this weekend. I don't know if it is related, but when using Yatse as remote control it says there are no episodes. I'll open a bugreport if it's not fixed by the attached zip file. |
Downgrading script.module.urllib3 to v1.24.1 fixes it. The problem was introduced with the update to v1.25 a month ago. The same update that forced us to release a new version of our addon because it broke URL encoding. Install this and rejoice: |
So now that we found the root-cause, let's use what we learned and take it with us for the future:
So reorganizing our code could really benefit us on Raspberry Pi, even if we stick with the requests library (switching is not that easy wrt. py2/py3). So a lot of the code-changes from #173 are still worthwhile ! Getting rid of BeautifulSoup may be worthwhile, although we only use it for generating categories listing and playing live streams these days. It may still be useful if there are mitigating tactics to have a more light-weight requests (by disabling certain parts/packages ?). |
It's possible to migrate to urllib2: https://github.com/mediaminister/plugin.video.vrt.nu/tree/urllib2 |
@mediaminister Tough call, I prefer requests because the code is cleaner/easier to maintain. I would also like to see if this an issue on Python 3. On the other hand, from what I have seen, Requests/urllib3 is a moving target and there is no guarantee that a future update does not break our addon (i.e. had 2 issues in the past month). To be honest, I am disappointed to learn how easily things can break/deteriorate/regress in the Kodi world. Things get updated/pushed with little regard on impact on end-users. Maybe I can agree on your implementation if we keep the Python 2 code properly separated (so we can get rid of it when no longer needed). Let me first move the fancy bits from my PR into the main codebase. cc @pietje666 |
My urllib2 branch succeeded al python2/3 tests: https://github.com/mediaminister/plugin.video.vrt.nu/runs/117783255 |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@mediaminister Does it include proxy-support ? Because that's a requirement IMO. |
Not yet, but I'll add this in my pull request. I'll first test my branch on a Windows Python3 test-build to be sure it really works. |
And after this, I guess we can do another release (v1.9.0). We can't expect our users to downgrade urllib3. And the master branch is again packed with features too. |
Before releasing, we should check what's wrong with dateutil and Libreelec Kodi 17.6. |
@mediaminister Can you open a new issue ? We can get rid of dateutil if needed, but it won't look pretty :-( And I prefer not to mess with timezones myself (especially since it also needs to work on Windows) |
This problem does not only occur with vrt nu but with several addons using urllib3. For example also youtubr, trakt, vpn manager, ... |
@peno64 Thanks, I wish we had found this one sooner. Projects should stop closing issues quickly like that, and keep them open for others to find and interact. (Also dissing other projects doesn't help the users of your library...) Given the feedback of the stakeholders involved, not relying on urllib3 and requests seems in our best interest, how sad or inconvenient that may be :-/ |
Well done everybody, we are now requests and urllib3 clean. |
They’re moving targets, but they’re also used by thousands of code pieces around the world and are very conscious about breaking backwards compatibility (without bumping their MAJOR version at least). If you really did encounter issues with code that broke due to a non-MAJOR version update, have you reported this to upstream?
Are you referring the ones I closed? If you’re unable to search closed GitHub issues, that’s on you. As evidenced by this comment I’m leaving here, it is obviously still possible to interact with closed issues. At the same time, there’s absolutely no reason to leave an issue open once it’s been determined that it’s out‐of‐scope for the issue tracker it’s been opened in, especially when there’s also be referred to more appropriate venues for continuing discussion as well as actually making a fix. Anyway, FWIW, |
The first issue was unclear if it was a bug or not, so we modified our code and released a new version to our users. (The addon failed to work completely) Upstream fixed it with v1.25.2. The second issue is a performance regression. The report was closed with the words: Alright, we'll mark this as a no-fix then.. At this point in time the performance regression was felt by millions of Kodi users on Raspberry Pi for a week.
I wasn't, at that point in time it was a requests issue for me, you clarified it was a urllib3 issue instead.
I can search closed issues, but I didn't find the one I needed. So I guess that's on me. (If even I can't find it, tough luck for others, but maybe I really am the problem. And no lessons to learn here...)
I don't blame anyone in particular here, I am just looking at the overall results of decisions at many levels and the impact it has on what we are delivering as a whole. And that picture does not look good if the aim is to provide a reliable experience. If you understand Dutch (or it translates well?), I give a much more detailed explanation on why I think using urllib3 or requests is probably not a wise choice for the future of our addon here: https://bitbucket.org/basrieter/xbmc-online-tv/issues/1159/plugin-traag-op-raspberry-pi#comment-51989436 And it's not about reporting and fixing when an issue occurs (reactively). If we want to offer a reliable user experience, we have to look for the best options to avoid breakage/regressions (proactively). And that's what we have been doing for the addon for a few months now to great result (not just related to urllib3, but lots of other resilience improvements). |
With v1.9.0 being released in the official Kodi repositories and with v1.10.0 being prepared with even more improvements, I think we can finally unpin this issue. No users currently should be affected. |
Describe the bug
I am not sure if this is related to the addon itself, but the addon is quite slow on LibreELEC on a RPi3 compared to my Windows laptop.
The A-Z listing (and many other menus in fact) take up to 7-8 seconds, whereas they are almost instant (sub 1 sec) on Windows. And this effect does not seem related to the size of the menu (number of items), every menu seems to take
5-6at least 4 seconds.This requires profiling to see what is exactly going on.
Additional context
The text was updated successfully, but these errors were encountered: