[Merged] Power Saver feature

cyperghost

@pjft said in Testers needed :: Power Saver features :: PR #172:

@Hex I agree. That's the wise approach to take.

Is it rather about:

Don't trust a binary compiled by someone else
Don't do working on a fix that may never be needed because the branch isn't merged into main release?
Annother codex or good practice I don't know about it?

@Hex
I think fix is easy ... but needs some work in looking @pjfts routines.
Anyway, I hope that my posting can be treated as alpha test.

ah.. and thanks for helping in using gitcommands

pjft

@cyperghost No, no, don't get this wrong :)

It's just that:
a) he won't have the source code to exactly debug it, by just using the binary -- we trust you that it crashes, but without the exact code, it's a bit of guesswork on his end as he doesn't know what code to look at exactly.
b) given that my current branch (and his branch) are both currently being developed, it may be that any time invested at this stage will be rendered obsolete - either because the bug may disappear, or because it may resurface while we keep developing.

As such, after one of them is merged, it's easier for everyone to have a stable code-base to use for actual runtime debugging. The best he could attempt to do right now would only be to try to replicate it on his current code base, seeing if deleting an entry from the list would trigger the same error. That way he would be able to narrow it down to the code to remove an entry from the list.

But if that doesn't work, he's kind of flying blind :)

Thanks for testing!

Hex

@cyperghost #3 definitely. You see features should inherently be isolated. Convoluted features should either be worked in a single PR or sequentially . This maintains the stability of the system. Simultaneous testing two PRs is very unwise.

Consider you are cooking two dishes. What do you think is better, making them one after another or at the same time? The prior is easier if you are making elaborate dishes that need great attention and timing. The later is easier if you are making scrambled eggs and toast because they are not convoluted; as in they dont require the same resources. Got it now?

cyperghost

@Hex @pjft
Sounds like you both are really know what you are doing. Thank you for clearing out :)

Hex

@pjft said in Testers needed :: Power Saver features :: PR #172:

@cyperghost No, no, don't get this wrong :)

It's just that:
a) he won't have the source code to exactly debug it, by just using the binary -- we trust you that it crashes, but without the exact code, it's a bit of guesswork on his end as he doesn't know what code to look at exactly.

@pjft I can easily reproduce the code that he is working with. I know which commands were used to get a working copy. Problem is if that doesnt exhibit the bug.

b) given that my current branch (and his branch) are both currently being developed, it may be that any time invested at this stage will be rendered obsolete - either because the bug may disappear, or because it may resurface while we keep developing.

@cyperghost This is the main reason and my post above elaborates on this idea with an example ;)

As such, after one of them is merged, it's easier for everyone to have a stable code-base to use for actual runtime debugging. The best he could attempt to do right now would only be to try to replicate it on his current code base, seeing if deleting an entry from the list would trigger the same error. That way he would be able to narrow it down to the code to remove an entry from the list.

But if that doesn't work, he's kind of flying blind :)

Thanks for testing!

Also I am completely unaware on what they are developing and what is the expected functionality. So for me everything is unexplored territory.

If i am able to find a stable solution how would you recommend me going back to my code to implement it? I might not have the interface with his code to apply the fix.

Hex

BUG : PS should be disabled while Scrapper is running

Hex

Guys I thought of a solution. Instead of having a switch how about having options(List)

PowerSaver Mode :-

None ~~~~~~No power Saving/Full performance
Minimal ~~~Power savings kicks in after Description has scrolled once, same time-out for Systems view)
Full ~~~~~~~PS kicks in after Game name has scrolled, description may not scoll
MAX ~~~~~~Experimental; Works well with Instant transition and no carousel animation

All of these will work fluidly with all Screensavers and with Video Previews.

This is the only way I can think of that will allow proper PowerSavings settings and allow user to decide till which level is the compromise acceptable.

Zigurana

@hex
I would ask, what is the difference?
Not the difference in text elements scrolling or whatnot, but what is the difference in power saved when going from min to max?
How much battery - life do users gain?

I have the feeling that the bulk of the improvements are already achieved when usin PS=min, and now you are trying to squeeze out some more improvements that have little impact in day-to-day use.
80-20 pareto, perfect being the enemy of the good, and all that.

Despite all our work on ES, remember that users will spend most of their time (and power) while running actual emulators, not just the front-end.

Hex

@Zigurana Minimal is the same as None if you are browsing the gamelist (with or without VideoP). This is because PS is not triggered till 10-12 seconds after last key pressed as description is scrolling. If someone is not interested in description then Full saves ~10 seconds of unnecessary processing per event.

How much battery life is saved, who knows. Havent profiled yet. Remember that some builds use Pi zero and lesser the processing lesser the heat. For those who are on Pi3 Min might be ideal. For those looking to squeeze every drop, MAX might be an alternative :P

MAX is total bonkers. Its branded experimental and might not be implemented.
This is to know your opinion before i implement it.

ES without PS heats pi more than Emulators. You see Emulators are meant for performance and likely so take more CPU usage. ES is an interface and its power consumption should be as low as possible dont you agree?

Zigurana

@hex You are of course correct in all your points. I wonder not if it matters, but how much, this needs to be considered as well, especially if one solution is more restrictive than the other.

For instance if the effective gain of max vs min is only 3% (because in the long run, these initial 12 sec do not matter) I would not introduce an additional user option. On the other hand, if this brings a significant improvement, albeit at the cost of some penalty, then a user option would be the way to go.

Now, we might never find out hard data on ES vs emulator usage, and therefore the precise savings that this will realize. But at least we can try to come up with a very rough approximation:

Assume about 50% usage of ES, of which the user would scroll through games about 50% of the time, and 50% is spend idling.
With PS=min we are impacting 25% of the power usage, while PS = max impacts all ES usage (50%).
Is it fair to assume that the idling will be mostly condensed in long stretches, because someone is leaving their machine running while having fun outside in the sunshine? Maybe, maybe not.
The power savings are significant, a factor of 10, I believe?

Using PS = min, we arrive at a power reduction of 22.5% (averaged over time), while PS = max gives a whopping 45%!

If this seems sensible, I would say implement PS = min as the default implementation and allow the user to enable PowerSaving (which is PS = Max) if they are willing to play the price in functionality. So no PS = NO, as PS after 12 second is basically painless, right? So the UI becomes a simple boolean, where we can also communicate the consequences of using PS in the extreme sense (no scrolling text, no videos, etc).

Hex

@Zigurana There is not much difference between Boolean and list. That much complexity is fine with me. List allows us to add further options later if necessary. Full allows PS for people who dont care about Description. I use Full all the time. My theme does not even have Description. Why should I have to be restricted to using Minimal Mode?

Remember all video stuff works well with PS. Omx does not even need PS to be disabled. So it makes sense. I would like to default PS to Minimal and allow users to decide if they want more power savings or not. It doesnt matter how much Mins you can squeeze out of this. I am against not having option to not care for description. Just as many users read description, many users dont. I dont want to punish them for that.

pjft

@hex said in Testers needed :: Power Saver features :: PR #172:

My theme does not even have Description.

Should that not be optimizable then in itself, regardless of PS being minimal or max? Same for empty descriptions, or descriptions that fit the container?

Hex

@pjft Well PS trigger is timer based so even if description is not present PS will wait full 12 secs after last event for (Missing) description to scroll. Currently PS and Scrolling is sped up for testing in my branch. Check out the stock for how long the scrolling takes.

If i have to code PS to factor in so many things as Video playing or not, Screensaver working or not, description scrollin or not, It will become too complex to code, manage and make changes to.

Description is scrollable text view and has fixed timeouts for start scrolling and repeat from start. It has no way of knowing how much time it actually scrolls. That depends on the size of text. So if description is too long then scrolling is also time consuming. 10 secs is lower bound to this.

TMNTturtlguy

All of this development is great, and I think the power saving feature is a huge benefit to some, but all of this development including the theme changes/default views, the power saving and others are really complicating a system that used to be very user friendly and easy to use. My opinion is we should keep the power saver options to in or off, or we should make it an optional package. Is there a way that the power saver features are all installed and default set to off or minimum since minimum is probably preferred, and the have the menu be an optional intall? This way basic users don't have all these menus and options. I guarantee there are more users of retropie than there are forum members and a majority of users will not read or research how the tools work and these features may lead to confusion and possibly non-working setups. Again, the features are awesome, but for the users sometimes less is more.

Hex

@TMNTturtlguy There are currently many options that wont make sense to general users. I can add a warning for users who change this option with all details about how each option works, just like Video Screensaver does. I dont think there is a way to make it optional package like you say. I know where you are coming from. May users who have made smaller handhelds with 3.5inch screen cannot even read the text of description. By default i can have Ps default to Min mode. I can although see many use cases for ~~Max~~ Full mode too. I have 3 handhelds that I use and heating is a major issue to tackle. That was the origin of PS. Hence I would like to have the ~~Max~~ Full option. Since many want the description to scroll I will support Min and make it default. Basic users dont mess with options they dont understand. If they do they can surely revert it back. Added with warning msg I think that is sufficient.

pjft

If we believe we will iterate and flesh this out into a more "accurate" solution in the long run, I'd rather start with a binary setting and have "off" be the default (which is the current situation), and "on" be whatever power saving setting you're comfortable with at the moment, and improve it over time.

Prefacing this with "I don't know a lot about how it's implemented", my comment earlier, was not as much thinking about adding edge cases, but thinking about PS as a system-wide thing rather than being centrally controlled.

I apologize for my potential over-simplification, but the way I'm understanding what has happened to solve this problem in terms of implementation is that there is a single key point of energy consumption, which is the animation loop.

So far, you've addressed it by making the render loop be interrupt driven, and added a timeout to it. Is that somewhat correct?

I believe that the render loop being interrupt driven is a key development - no pun intended - and that should be the cornerstone of this.

What I was going for was, rather than have a timer - and by consequence different "min", "max" settings corresponding to different - but arbitrary - timeout values, would effectively attempt to address this in the actual rendering pipeline.

The way I understand this is that GuiComponents have an "update" and a "render" method. Also, the only elements that are animated at the moment are ScrollableContainers, the TextListComponent (not sure if by virtue of the previous one, or in itself) and VideoComponent.

We could consider changing "render" to return a bool value, which by default will be "false" if the element does not need an animation.

And for those 3, you'd just need to update the "render" method to return "true" if being animated or not, and "false" if doesn't need to be animated (meaning:

content of ScrollableContainer needs to scroll;
TextListComponent's currently selected element needs to scroll;
VideoComponent is using VLC.

That way, the cycle would be (pseudo-code):

start:
    bool needsAnimation = render();
    while (needsAnimation)
        // this will return false if all of the rendered components return false;
        needsAnimation = render();

    wait(input);
    updateGuiComponents();
    go to start;

Once again, blatant simplification, and terrible pseudo code.

But if our intention would/could be to head in that direction, then I don't see why we will ever need more than two options - with the ending up being your current PS min, but without the current effects on the user experience. And as such, we can launch for now with just "Off" and "On".

Or maybe it wouldn't even need to be an option in the future - it'd just be optimized by default. :)

My 2 - very likely wrong and overly simplistic - cents. :)

Hex

@pjft What you have mentioned is actually my long term plan. It will take a long time to refactor and test everything. I am interested in implementing this asap while what you mention will be taken up later. I do not have an idea of how rendering currently works so I will have to get familiar with that first.

Another problem with your idea is that SDL has to wait for events, so if we dont render when render is not needed we end up with a busy wait loop which is even worse for power.

Currently stock code looks like this

While(Event.get())
   if Event != null 
        Handle event
   Render()

What you are proposing would work like this (over simplifying)

While(Event.get())
   if Event != null 
        Handle event
   if (Render.needed())
       Render()

Problem would be if both conditions are false it will just be a while loop, which is bad. Having an empty while loop puts the cpu in overdrive.

Another problem with your psudocode is events will not be triggered if rendering is running. ie when you press right on carousel it takes 1 secs to move one system and no event will be consumed till that animation is over. Event.get() needs to be called as fast as possible even while animating.

It is very complicated to redesign rendering to be optimized for PS. And that is time i dont have presently. Hence baby steps.

pjft

@hex great! Then start with just on and off. There will be no need to complicate things if you head in that direction.

As for the code, as I mention, it was pseudo code.

The way to handle the loop when both are false is to sleep until (time - time at start of the loop) is larger than whatever max frame rate we want to enforce. That way we ensure we only run the loop, if both are false, a max of 60 times per second, assuming we want to keep it at 60 fps max. It's a standard frame rate loop controller. Hopefully that's not terrible in terms of performance.

But great to hear that that is where it's headed. Take your time and launch something now. Then iterate.

Hex

@pjft said in Testers needed :: Power Saver features :: PR #172:

@hex great! Then start with just on and off. There will be no need to complicate things if you head in that direction.

Its not that complicated to implement. I shall be submitting final updates soon. Was just interested in checking if others had any better suggestions to the names: None/Min/Full

I know that keeping defaulted to Min will be sufficient for majority of users. Optionally None is also worth using if you are connected to wall socket and have good cooling. Full is for portable setups and there is a whole community for it : sudomod.com

As for the code, as I mention, it was pseudo code.

Yes I know. Your psudo code had nested while loops which wouldnt work and hence i pointed it out. Nothing more than that.

TMNTturtlguy

@hex I have another thought which might not be popular, but I am not sure that these options need to be available in the main build. I don't know if a third ES build is worth the effort to keep updated but it seems to me that havin a RPI1 a RPI2/3 and a RPI-handheld might make sense. This build could be created efficiently to work for handhelds and you could maximize it to the fullest PS settings.