Skip to content
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

Improve performance for python addons using xbmcgui.ListItem #11651

Merged
merged 2 commits into from Feb 12, 2017

Conversation

@popcornmix
Copy link
Member

commented Feb 10, 2017

Description

Improve performance for python addons using xbmcgui.ListItem

Motivation and Context

See: http://trac.kodi.tv/ticket/17304#comment:28

How Has This Been Tested?

In last two Milhouse nightly builds.
Myself and trac ticket reported have measured significant performance increases

Types of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the Code guidelines of this project
  • My change requires a change to the documentation, either Doxygen or wiki
  • I have updated the documentation accordingly
  • I have read the CONTRIBUTING document
  • I have added tests to cover my change
  • All new and existing tests passed
@popcornmix

This comment has been minimized.

Copy link
Member Author

commented Feb 10, 2017

Ping @FernetMenta
#9748 introduced the performance regression.

I suspect a better solution to this issue will be found for Leia, but this is a low risk fix that could be backported to Krypton.

@@ -296,6 +296,7 @@ CApplication::CApplication(void)
, m_fallbackLanguageLoaded(false)
, m_WaitingExternalCalls(0)
, m_ProcessedExternalCalls(0)
, m_ProcessedExternalDecay(0)

This comment has been minimized.

Copy link
@FernetMenta

FernetMenta Feb 10, 2017

Member

please use c++11 style for init of members

This comment has been minimized.

Copy link
@popcornmix

popcornmix Feb 10, 2017

Author Member

I've switched them all. Is that okay?

This comment has been minimized.

Copy link
@FernetMenta

FernetMenta Feb 10, 2017

Member

I meant doing init by assignment in the header. that is much better than having to keep the right order in the constructor.

This comment has been minimized.

Copy link
@popcornmix

popcornmix Feb 10, 2017

Author Member

Ah like this?

This comment has been minimized.

Copy link
@FernetMenta

FernetMenta Feb 10, 2017

Member

yes, thanks

@popcornmix popcornmix force-pushed the popcornmix:openwindow branch 2 times, most recently from e5259ee to 2e4cedf Feb 10, 2017
@popcornmix popcornmix force-pushed the popcornmix:openwindow branch from 2e4cedf to 8445d39 Feb 10, 2017
@DaveTBlake

This comment has been minimized.

Copy link
Member

commented Feb 11, 2017

Do you think the Python regression could also be the cause of music playback stuttering when returning back to the visualisation screen? User reports it clearly here http://forum.kodi.tv/showthread.php?tid=306568. Happens with visualisation, but not genreal lib navigation.

I going to set a build going for him test

@FernetMenta

This comment has been minimized.

Copy link
Member

commented Feb 11, 2017

Completely different issue. Most likely related to the visualization. Note that what you call python regression would show effect in the opposite direction, means cure stuttering in a/v playback. It's the outside world like python and addons that disturbs the core.

@FernetMenta

This comment has been minimized.

Copy link
Member

commented Feb 11, 2017

jenkins build and merge

@FernetMenta

This comment has been minimized.

Copy link
Member

commented Feb 12, 2017

jenkins is out of order

@FernetMenta FernetMenta merged commit 5e1bc59 into xbmc:master Feb 12, 2017
1 of 3 checks passed
1 of 3 checks passed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
jenkins4kodi Yeah yeah I'll get to it when i have some time
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
@popcornmix popcornmix deleted the popcornmix:openwindow branch Feb 12, 2017
@basrieter

This comment has been minimized.

Copy link
Contributor

commented Feb 12, 2017

Will this also be merged into the Krypton branch and be in 17.1 ?

@popcornmix

This comment has been minimized.

Copy link
Member Author

commented Feb 12, 2017

Hopefully. Keep an eye on #11660

@basrieter

This comment has been minimized.

Copy link
Contributor

commented Feb 12, 2017

I guess I should wake up and should have looked better at the text

popcornmix referenced this pull request 6 hours ago
Open
Improve performance for python addons using xbmcgui.ListItem #11660

Sorry for that.

@posti85

This comment has been minimized.

Copy link

commented Feb 12, 2017

We all hope is merged soon xD it's known our kodi addons are running very slow in our Raspberries. Thank popcornmix for the code and basrieter for the research!

@MartijnKaijser MartijnKaijser added this to the L 18.0-alpha1 milestone Feb 14, 2017
@MartijnKaijser

This comment has been minimized.

Copy link
Member

commented Feb 14, 2017

@popcornmix could you make a backport PR?

edit:
nevermind. already there

@popcornmix

This comment has been minimized.

Copy link
Member Author

commented Feb 15, 2017

As this has only just been merged then I assume your issue has nothing to do with this PR.
(this PR actually does nothing different when video is playing).

Open a trac ticket if you have found a regression on Krypton compared to Jarvis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.