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

Suspend function #2239

Closed
oldhasbeen opened this issue Sep 6, 2016 · 165 comments
Closed

Suspend function #2239

oldhasbeen opened this issue Sep 6, 2016 · 165 comments

Comments

@oldhasbeen
Copy link

  • KOReader version: several including the latest stable and nightly
  • Device: Aura H2O

Issue Device seems to sleep but it is draining power like in page display or menu modes

Steps to reproduce

I have measured the battery currents of my Aura H2O and made this observations:

Off state: approx 40 microamps

KSM 08 idle: 13 mA average

Koreader idle book open: 13mA average
Koreader in true sleep mode: 1mA average

Nickel Menu idle: 2 mA avg
Nickel sleep: 1mA avg

maximal working current 320 mA

Coolreader and other apps in the vlasovsoft package
have also 1mA sleep current

True Sleep (suspend)is a three stage process:

  1. full working current [300 mA] for 5-6 seconds
  2. then 13mA for approx 10 seconds
  3. drop to 1mA and remains (or should) there until wake up is initiated (open sleep cover or push power button briefly).

With open sleep cover and pushing the power button briefly Coolreader goes to true sleep. Closing now the sleep cover will wake the device briefly followed by the suspend chain. Coolreader will always true suspend when the sleep cover is closed. (I have not tested Nickel).

The latest releases of Koreader do go into true sleep when sleep cover is closed. They will also go into true sleep when power button is briefly pressed.
With open sleep cover and pressing power button true sleep is initiated. Closing the sleep cover will wake the device briefly (full 300 mA current) it will proceed to stage 2 (13 mA) and will remain there. The screen shows the book cover and the device appears to sleep but it is not. The current drain is the same as with a displayed page in reading mode.
Disconnecting a charging cable with closed sleep cover will also result in a false sleep state and 13 mA drain.

I have measured with Fluke 87 (average mode) direct in the positive battery lead.

The device activity can be non-intrusive observed with an AM radio: Place the ereader within a few centimeters to the ferrite antenna and select a long or medium wave range. Select an unoccupied frequency and listen to the speaker noise. Move the ereader around to get the feel what to look at.
Stage 1 and 2 are not easy to discriminate but stage 3 is significantly quieter. The jump to stage 3 occurs after about 16 seconds from triggering the suspension chain and is easily to recognize.

I hope this helps to improve the sleep function.

@Markismus
Copy link
Member

So basically you're saying that the suspend function works, but that it would be nice if we could get an idle state that only consumes 2mA?

@KenMaltby
Copy link

Hmm... That would suggest that Nickel's "Menu Idle"/background processing = 2mA drain. Some of that would be the touchscreen monitoring. The 15 or 16 seconds is interesting as well, there are deliberate delays applied to KOReader's current suspend function. I would assume that the need to save position and other data enters into the delay.

@oldhasbeen
Copy link
Author

@Markismus ,
to get an idle state to 2mA would of course be nice but that is not my priority (it would predictably add some reading time).
The big issue is that the suspend function is not reliable. I tried to illustrate in my post that there is a true sleep (suspend) state with little current drain -1mA - that gives more than a month standby time with the inbuilt 1500mAh battery). And there are at least two easily reproducible situations where there appears that a true sleep is present but in fact it is in idle state (13mA) and the battery is drained after 5 days - without any reading. There is no way to tell from the seemingly sleeping device whether there is true or false sleep other than external monitoring. It may truly sleep or it may not. There must be a way because the sleep initiation in the coolreader package (and I assume Nickel's too) is reliable.

@KenMaltby
I don't know why Nickel needs less idle current. I can also not tell if it is Koreader alone or KSM 08 that needs more idle processing (can we shut off background KSM when Koreader runs to tell the difference?). But I can personally happily live with that because of the huge benefits Koreader and KSM give (and it applies anyway only to the wake state).
I interpret stage 1 (300mA) - about 5-6 seconds as data and housekeeping work and stage 2 (13 mA) - about 10 seconds as waiting idle until true sleep (1 mA).
My issue is that in Koreader situations do exist resulting in a state of idle power consumption (max 5 days until battery is flat without reading) that appears to the unsuspecting user as true sleep - but is in fact the idle state.

@KenMaltby
Copy link

KenMaltby commented Sep 7, 2016

I am not seeing anything that KSM has running in the background, when it calls koreader.sh, unless one of the other utilities like WiFi or USB services were started by the user, as well. (Plugging in the USB cable, might also.) The 13mA idle you show for KSM suggests that more than the 2mA [Correction there is only 1mA above true sleep] Nickel needs (to monitor the touchscreen and whatever else) is involved in the KSM idle, as you made the measurement.

It still is a mystery, but you have certainly provided some data I haven't seen before.

@houqp
Copy link
Member

houqp commented Sep 7, 2016

Thanks @oldhasbeen for the data! They are very useful. I have never thought of do this kind of checking before. This is something we should have done better from the beginning.

I think the high idle consumption rate has something to do with KSM. IIRC, it actually runs in the background if that's what you use to start KOReader. I need to double check on this one though.

The bigger issue, as you pointed out, is not putting the device in the sleep properly in some of the scenarios. We should definitely fix this. Without actually testing myself, my initial guess would be event triggered by closing sleep cover interrupts the sleep and wakes up the kernel. But KOReader never puts the system into sleep after by calling suspend script again because it thinks the device is already suspended due to pressed power button.

@houqp houqp added this to the next stable release milestone Sep 7, 2016
@houqp houqp added the bug label Sep 7, 2016
@houqp
Copy link
Member

houqp commented Sep 7, 2016

Tagged this issue to stable release for further investigation.

@oldhasbeen
Copy link
Author

@houqp
It appears that your reasoning mirrors mine. I would be more than happy to test any suspend script prior to release.

@AlanSP1
Copy link

AlanSP1 commented Sep 7, 2016

@oldhasbeen thank you for your great analyses of power consumption for kobo reader, it is really helpful in my opinion and you even went step up from the one on forum.

With open sleep cover and pressing power button true sleep is initiated. Closing the sleep cover will wake the device briefly (full 300 mA current) it will proceed to stage 2 (13 mA) and will remain there. The screen shows the book cover and the device appears to sleep but it is not. The current drain is the same as with a displayed page in reading mode.

This also shows something I felt happening (and never was sure if it really is so), that for some reason sleep cover is causing battery drain. That's why I was asking for ability to disable sleep cover: #1733

I really hope that koreader will achieve power efficiency as nickel, as this is only thing in my opinion where nickel has some advantage. Of course, koreader is much superior in any other way that charging battery more frequently is something I gladly do.

Just to mention, it looks that in some scenarios there could be even worse battery drain, something I mentioned here: http://www.mobileread.com/forums/showpost.php?p=3379558&postcount=42 In that instance, over less than 24 hours battery dropped from 100% to 72% and there was loss of 3 minutes (clock was late 3 minutes), suggesting that something impacted whatever takes care of clock accuracy. But, this is rare. This with sleep cover, this happens more often.

@houqp
Copy link
Member

houqp commented Sep 11, 2016

@oldhasbeen could you help test #2245 to see if it makes a difference?

@AlanSP1 if you know how to reproduce the clock skew issue, please do let us know, I will take a look.

@oldhasbeen
Copy link
Author

@houqp
I have tried but could not start koreader. I am a hardware person not a coder and may have done things wrong. I have copied both files into notepad ++ and saved as lua. have then substituted both files in the last stable release. koreader did briefly blink but did not start. Can you give me the file as lua (not script) or an idea what I may have done wrong?

@houqp
Copy link
Member

houqp commented Sep 11, 2016

Copy content fromt he web and saving files with notepad++ might introduce weird encoding issues. It's better to download them directly and copy them to the device :)

links:
https://raw.githubusercontent.com/koreader/koreader/houqp-master/frontend/ui/uimanager.lua
https://raw.githubusercontent.com/koreader/koreader/houqp-master/frontend/device/generic/device.lua

Also make sure you don't have ignore_power_sleepcover set to true in the reader setting since it will not put device back to suspend when device is waken up by a cover close action. I will fix that part later.

@AlanSP1
Copy link

AlanSP1 commented Sep 11, 2016

About clock, that is something I noticed when there was also battery drain. I didn't do anything special. And this is rare.

After that I started using again CoolReader suspend script and even tried powering off device completely. Strange thing is, when I power off device there's also loss of battery and clock is late about same amount. This is pretty confusing. Power off script I use points to KSM's own power off script. So, this probably isn't koreader related.

@oldhasbeen
Copy link
Author

@houqp
thanks for the advice. I have directly copied into the lua files and it seems to work now. A quick and dirty test with AM radio indicates that true sleep is there where it wasn't before. Looks encouraging. I will do some real world measurements in the next day or two and will report back. Again thank you.

@houqp
Copy link
Member

houqp commented Sep 11, 2016

That's awesome! Thanks again for the help! What radio device are you using for the test? I would like to get my aura one first before opening up my hd :P

@oldhasbeen
Copy link
Author

@houqp
unfortunately things are not all singing and dancing. I have done some detailed tests now. Installed is latest stable and proposed changes. System is rebooted. Used is AM reception tool to monitor system activity.

  1. cover cycle test (book is opened and cover is closed then opened then closed ....). The opening and closing is done in such slow sequence that all data and housekeeping work has enough time to complete. True sleep worked for 9 cycles (time between cover trigger and true sleep is approx. 16 seconds) - then went corrupt - again false sleep. No matter what I did then (short press power or open and close cover) the true sleep was gone. After reboot true sleep works again.
  2. Short press power button cycle test (cover is left open and is not touched - open book is toggled between sleep and wake with short pressed power button. True sleep occurred after approx. 13 seconds (shorter than cover triggered sleep). I have not seen corruption.
  3. Closed sleep cover (true sleep state) and short press power button. Device wakes and stays awake peeking under cover shows that book is open. Short press button again results in false sleep. Once there is false sleep rebooting looks like the only way to get true sleep again working.

When false sleep is present the screen shows book cover and no touch actions. The Infrared touch system is however active as well and touches (with nonconductive pointer) gives responses on AM monitor.

I have done all this with coolreader but have not found a way to goof it's sleep function. Works every time no matter what I throw at it. Closed cover results always in true sleep and short pressing power then will wake the system briefly but is will fall back into true sleep. Closed sleep cover overrides everything else.

About AM monitoring:
It needs to be understood that we do not analyse data content but total system activity. This is done by picking up the electromagnetic emission that is radiated by the ereader. The emission spectrum is very very broad and strong enough to be picked up by probably any type of AM receiver. I have tried several low cost radios and use myself an almost antique multiband receiver (Grundig Satellite 2000). I have not tried any PLL, SDR or digitally processing tuners and I would advise against because of artificially created signals with those systems (noise and birdies). A straightforward analog super will do. If it has a signal meter - even better to observe changes in emitted power (true sleep makes the needle drop very nicely).
Important is:

  1. radio must have a ferrite antenna (we pick up the magnetic component of the radiation)
  2. radio must be in AM reception mode (FM will not do). Best results are in the long wave range (150 - 450 kHz) but medium wave is good to - it seems here best around 800 - 900 kHz).
  3. a frequency must be found not occupied by broadcasting but only hissing background noise. Since AM transmission is very far reaching during night time it may be that due to occupied bands only daytime tests can be done.
  4. keep away from anything that has significant EM emissions. The long and medium wave bands are unfortunately very polluted nowadays. Computers, switchmode power supplies, TVs, mobile phones and who knows what else can make it necessary to find a quiet place - maybe even outdoors. A battery operated radio is needed then.

I myself place the ereader on top of my radio in a fixed position to have some signal when the reader is awake. The change to true sleep is clearly discernible and a stopwatch helps with timing. Data processing and house keeping can be observed as well.

@AlanSP1
Copy link

AlanSP1 commented Sep 12, 2016

Just to comment that CoolReaders suspend script can have problems (some users report it, I also had some), but they seem to be less pronounced, far less pronounced.

@oldhasbeen
Copy link
Author

@houqp
correction: when true sleep is lost no system reboot is required - exiting and restarting koreader will do.
I am now absolutely positive that IR system is active in false sleep (without causing action). Have dug out my SSB adapter which give the option to switch off the automatic gain control in my AM receiver. The three activity states are now easily distinguishable. Have set the gain control so that full processing power gives a reading 5 on the field strength meter, idle is 3 and true sleep 0.5. Zero ambiguity and everything in real-time.

@dagrim1
Copy link

dagrim1 commented Sep 13, 2016

I also wanted to add to this discussion, good stuff...

But I also notice drain in Koreader without having KSM installed. I now installed Koreader on my Aura One using the Filemonitor method and also then I notice draining. (Last night it went from 97% to 46% over ~7 hours in sleep mode. Pressed the power button from within Koreader to put it to sleep after which it showed the book cover (as expected). Woke it up this morning and battery was down by ~50%,

I also noticed that from within Koreader it doesn't seem to go to sleep automatically... But perhaps that's some setting I missed somewhere.

Already put on the patches mentioned in this thread btw...

@AlanSP1
Copy link

AlanSP1 commented Sep 13, 2016

Koreader at the moment AFAIK don't have automatic sleep. If you left it on, it should stay on indefinitely (i.e. till battery runs out). There were discussion about this feature, but I think it still isn't implemented. But, I can be wrong here.

@houqp
Copy link
Member

houqp commented Sep 14, 2016

unfortunately things are not all singing and dancing. I have done some detailed tests now. Installed is latest stable and proposed changes.

My previous change only fixes the issue you mentioned earlier, i.e. after putting the device into suppend through power button, closing the cover will wake up the device and never puts it back to true sleep. Could you help verify that this is indeed fixed?

  1. cover cycle test (book is opened and cover is closed then opened then closed ....). The opening and closing is done in such slow sequence that all data and housekeeping work has enough time to complete. True sleep worked for 9 cycles (time between cover trigger and true sleep is approx. 16 seconds) - then went corrupt - again false sleep. No matter what I did then (short press power or open and close cover) the true sleep was gone. After reboot true sleep works again.

This is interesting. I can't come up with a good explanation on why this is happening yet. I just ordered a radio receiver online so I will reproduce it this weekend.

  1. Short press power button cycle test (cover is left open and is not touched - open book is toggled between sleep and wake with short pressed power button. True sleep occurred after approx. 13 seconds (shorter than cover triggered sleep). I have not seen corruption.

Thanks for confirming this. That means our suspend code is working properly but something weird happens when that code path is triggered by the sleep cover.

  1. Closed sleep cover (true sleep state) and short press power button. Device wakes and stays awake peeking under cover shows that book is open. Short press button again results in false sleep. Once there is false sleep rebooting looks like the only way to get true sleep again working.

Good catch. This should be fixed by my latest changes, again, you can download them from:
https://raw.githubusercontent.com/koreader/koreader/houqp-master/frontend/ui/uimanager.lua
https://raw.githubusercontent.com/koreader/koreader/houqp-master/frontend/device/generic/device.lua

@houqp
Copy link
Member

houqp commented Sep 14, 2016

Koreader at the moment AFAIK don't have automatic sleep. If you left it on, it should stay on indefinitely (i.e. till battery runs out). There were discussion about this feature, but I think it still isn't implemented. But, I can be wrong here.

@Hzj-jie already implemented the autosuspend feature for kobo. By default, the timeout is set to 3600 (1 hour). You can change it by tweaking the reader setting auto_suspend_timeout_seconds.

@houqp
Copy link
Member

houqp commented Sep 14, 2016

But I also notice drain in Koreader without having KSM installed. I now installed Koreader on my Aura One using the Filemonitor method and also then I notice draining. (Last night it went from 97% to 46% over ~7 hours in sleep mode.

I am not sure how well koreader works with fmon since, IIRC, nickel is running in the background. koreader does not communicate with nickel during suspend. So maybe that causes the battery drain.

@dagrim1
Copy link

dagrim1 commented Sep 14, 2016

@houqp

Thanks for the feedback! Just wanted to add something else wrt the drain...

I found this thread: https://www.tapatalk.com/topic/270306-50705

So one of the things there suggested the 10s delay might not be enough to finish necessary processes before going into suspend and thus failing. As a result I modified the following line in koreader\frontend\device\generic\device.lua

UIManager:scheduleIn(10, self.suspend)

to

UIManager:scheduleIn(15, self.suspend)

Thinking along the lines of 'higher resolution so perhaps needing more time for drawing the cover, etc'. 15 was just a 'really should be enough if this is the issue'.

An no idea if it's a coincidence but after a 1 hour suspend it now didn't loose a single percentage of battery. Also checking the log I now see:

[09/13/16 @ 14:45:21] Kobo Suspend: Going to sleep . . .
[09/13/16 @ 14:45:21] Kobo Suspend: Asked the kernel to put subsystems to sleep
[09/13/16 @ 14:45:23] Kobo Suspend: Waited for 2s because of reasons...
[09/13/16 @ 14:45:23] Kobo Suspend: Synced FS
[09/13/16 @ 14:45:23] Kobo Suspend: Asking for a suspend to RAM . . .
[09/13/16 @ 15:45:08] Kobo Suspend: ZzZ ZzZ ZzZ? (0)
[09/13/16 @ 15:45:08] Kobo Suspend: Woke up!

Before there was a
sh: write error: Operation not permitted

In between the Asking for a Suspend to RAM... and Zzzz ZzZ ZzZ? (0) lines.

I now have no drain in Koreader (in combination with filemonitor) anymore it seems. Also after a reboot all is still good.

No clue if this did the trick, but might be worth a try as well... gonna keep an eye on it (and perhaps start from scratch to really identify what did the trick (if any trick has really been done ;) )

I will check the timeout setting as well wrt auto-suspend. Things are getting better and better ;)

Edit
Changed the timeout and indeed it goes to sleep, also again about a single percentage lost in over an hour of sleep in Koreader. So nice!

@AlanSP1
Copy link

AlanSP1 commented Sep 14, 2016

By default, the timeout is set to 3600 (1 hour). You can change it by tweaking the reader setting auto_suspend_timeout_seconds.

Where this setting is stored? Defaults.lua?

@dagrim1
Copy link

dagrim1 commented Sep 14, 2016

@AlanSP1 : It's in the Koreader folder once you started (and closed?) Koreader for the first time.

Btw, today I did a reinstall of the kobo firmware and reinstalled Koreader with KSM08. Everything seems to work great INCLUDING sleep!

I think the 15s tweak mentioned above indeed does the trick:

koreader\frontend\device\generic\device.lua

UIManager:scheduleIn(10, self.suspend)

to

UIManager:scheduleIn(15, self.suspend)

Both after autosleep after 4 mins and manual sleep I don't have battery drain.

@oldhasbeen
Copy link
Author

@houqp
I look forward to compare AM activity observations. I am obsessed with the power consumption issue because I am looking to incorporate a solar panel in the sleep cover and forget about external charging.
I have noticed the 1 hr autosleep delay and have looked in vain for the delay script in the various setting files (which file is it in?). I would set it to 20 minutes. That is power wise about the break even time for the Aura H2O between idle power consumption and shutoff/restart.
From the perspective on my solar powered aim it would be good to have a timer which will switch the device off after say 2 or 3 days in sleep mode.
It has happened now several times that only cycling with the power button (no other functions were used - no navigation or page changes or sleep cover etc.) corrupted the true sleep state. Something continues to corrupt and requires to exit and restart koreader to get that function again. Just to make this clear: the true sleep as such does exist in koreader and is power wise on par with that of nickel - it is the unsuspecting loss of it that really bothers.

This observation may be a red herring (?): when true sleep works there is a brief burst of activity about 1 second before switching to true sleep. When true sleep is lost there is no burst but idling continues.

@dagrim1
Copy link

dagrim1 commented Sep 15, 2016

@AlanSP1 @oldhasbeen :

The autosleep timeout can be set in the settings.reader.lua file (in the koreader folder, koreader has first to be started (and closed?) in order for it to be created).

There you can add a line to set timeout in seconds, such as below (where the time is set to 240seconds, meaning 4 minutes). Take note that the lines, except the last one, are seperated by a comma:

["auto_suspend_timeout_seconds"] = 240

@houqp
Copy link
Member

houqp commented Jan 15, 2017

It adds a 9th footer item in Status Bar submenu (mem_usage in the code), that will display M:124 207, Resident size and Virtual Size : Resident size should stay below 400 to be fine.
Handy to monitor if actions or features eat memory.
@houqp: should I PR that (may be with only the Resident size) ? or is that too technical for endusers ?

I think this is too technical for other users :) What I prefer is to have all these system info (including disk usage) shown in a modal that can be triggered from the menu. Or even better, have koreader automatically monitor the memory usage in background and trigger an alert to user when it meets a threshold.

@oldhasbeen
Copy link
Author

@poire-z
I have added the memory usage monitor. This is at this stage exactly the tool I have wished for. Thank You!!! If this works out as hoped it will restore the needed confidence in koreader for me.

@oldhasbeen
Copy link
Author

@houqp
after almost one week in continuous operation (about 1000 page turns and about 100 suspends and plenty of dictionary lookups) the suspend has not corrupted but the dictionary lookup is now lost again. I am now also pretty sold on the memory leak trail. It seems that the suspend function was more sensitive to corruption and the next obvious in line seems to be the dictionary lookup. I have added the memory usage monitor and keep observing.
Do you feel that the dictionary lookup failure - which seems now explainable and repeatable - should be opened in a new issue or is that obsolete now? For me not any more a pressing problem.
My guess is that the false sleep problem with the associated battery drain has come to an end. I refrain however to close this thread yet until I have full confidence and that may take some time.
For now I want to express my gratitude for your help and work.

@oldhasbeen
Copy link
Author

Has anyone knowledge how other readers (nickel, kindle etc.) handle memory and memory leaks?

@KenMaltby
Copy link

You could check for "pkill -15"s, wonder how suspend would clear the blitbuffers memory and still have the image displayed. I don't think the e-Ink display refreshes that way, but I don't know how it would work.

@houqp
Copy link
Member

houqp commented Jan 16, 2017

Do you feel that the dictionary lookup failure - which seems now explainable and repeatable - should be opened in a new issue or is that obsolete now? For me not any more a pressing problem.
My guess is that the false sleep problem with the associated battery drain has come to an end. I refrain however to close this thread yet until I have full confidence and that may take some time.
For now I want to express my gratitude for your help and work.

It's indeed a wild ride (more than 100+ technical comments) ;) We couldn't have gone this far without help from everyone especially your detailed hardware level analysis, @dagrim1's 15s discovery and @poire-z's insight. I still can't explain why 15s is the magic number though, maybe it's kernel related.

Please don't close this issue yet since we still have couple more actionable items:

  1. remove all os.execute calls in suspend and resume code path
  2. implement logic to put koreader back to suspend when it was wakenup without user interaction.

Has anyone knowledge how other readers (nickel, kindle etc.) handle memory and memory leaks?

As I mentioned in my previous comment, they simply launch one process for each opened book. Once we move to this model, it will "fix" most of our memory leak issue too. This model also has the extra benefit of isolating crashes from books to the main koreader process which will make koreader able to reopen a book if a crash is detected instead of going back to KSM.

@oldhasbeen
Copy link
Author

oldhasbeen commented Jan 23, 2017

I am now convinced that the memory leak is the root cause. After starting koreader the resident/virtual memory size was 19/69, creeping up eventually to 355/521 where the dictionary lookup stopped to work. At 443/617 I tried to suspend and koreader crashed. Restarting restored full memory and all functions worked ok. There was never any false sleep - that seems a thing of the past.
Until the memory leaks are fixed is the "Poirot Crash O Meter" (thanks poire-z) a good tool to keep things functional. I will from now on restart koreader whenever the resident memory goes past 300.
I wonder how many other bugs are in fact not really bugs but results from shrunk working memory size.

@poire-z
Copy link
Contributor

poire-z commented Jan 23, 2017

Memory leaks related to images should be fixed in current nightly (>=813), so you may wish to upgrade to see how that goes, and tell us if it leaks less quickly (there are still some leaks when switching documents).

@j4w0r
Copy link

j4w0r commented Jan 23, 2017

@poire-z In versions before v.813 on my Aura H2O resident size was increasing by 12MB each time after "suspend (show book cover) - wake up" action. After v.813 in that scenario it is inreasing by 2MB (some small leak was left). All in all, thank you all for determination narrowing suspend/memory problems.

@poire-z
Copy link
Contributor

poire-z commented Jan 24, 2017

@j4w0r: i just checked that (cover image - i usually use files), and i may see the same increase you do (may, as i'm not sure, the increase is small, there may be some increase when turning pages, and sometimes there's a decrease).
But I may have seen less of that if I choose "Screensaver / autostrech cover image" in the 3rd menu icon.
Can you confirm you were not using autostretch ? And if not, test how it goes with autostretch enabled ?
That may give me a hint, although i don't see yet what's not beeing freed in this case.

@AlanSP1
Copy link

AlanSP1 commented Jan 24, 2017

If you want to follow koreader process memory usage, at least to check that it's way below the limit, you may drop this file in place of frontend/apps/reader/modules/readerfooter.lua
readerfooter.lua.txt
It adds a 9th footer item in Status Bar submenu (mem_usage in the code), that will display M:124 207, Resident size and Virtual Size : Resident size should stay below 400 to be fine.
Handy to monitor if actions or features eat memory.
@houqp: should I PR that (may be with only the Resident size) ? or is that too technical for endusers ?


@poire-z I'd like to have this added as an additional option, that users can choose to use or not. It is very useful now, but can be handy in the future also.

@oldhasbeen
Copy link
Author

@poire-z
I agree with Alan SP1. The Crash o Meter is a resource monitor in the same way as a battery gauge is and may have helped immensely with the false sleep corruption issue.
Without book covers as screensaver I see less than 1MB per suspend cycle leaked away. 1-2 MB with book cover - stretching does not seem to change much. Dictionary lookup eats noticeable. I use this often - can be there a memory release implemented?

@poire-z
Copy link
Contributor

poire-z commented Jan 24, 2017

Just checked with the emulator (no suspend, but calling screensaver.show() & close()), that there is a 0.2M memory leak after each, whether cover image is stretched or not, and even when it is displaying text. Image display eats 2-4M, but this seems released or reused. Numbers may be bigger on our devices, dunnoy why, arm stuff ?
Coud you check if you still notice the 1-2 Mb leak if you chose to "Screensaver/Exclude this book's cover from screensaver" (you should get then an InfoMessage with "Sleeping" text if you haven't set up screensaver images from filesystem). If it still leaks, that would mean image display is at least fine.

For these other leaks (dict, suspend), well, I don't know, it's less obvious than what i saw with blitbuffers.
For dict lookup, i also see some memory increase. I see there is some JSON/lpeg (C module) involved for the stardict output decoding. I don't know much about this. @houqp may say if he trusts lpeg with memory leaks.

BUT : we may all watch this after a fresh start, and notice memory usage grows after most actions. May be there's some delay for garbage collection/memory re-use to happen. Just saw some graph at #1060 which may indicates that it needs some time to reach some state where there is enough holes in the already allocated memory to be reused.
I don't really know how memory allocation is done, but i may imagine stuff like dict asks for 4M of RAM, releases it, some other stuff asks some smalls chunks, that are taken from these 4M, and later, when dict asks again for 4M, as the original 4M has been fragmented and some bits already used, it will allocate a new 4M block, and that may go till there are enough holes so that small requests for memory can be taken from these holes, and the last 4M block re used without the need to allocate some new memory ?

About the memory usage footer item, i'll wait for @houqp 's ok.
I also prefer having the option to have it in the footer than in modal launched from a menu, as you then get memory usage in real time. But there is one sad side effect: it distracts me from reading, as I keep watching it and wonder why and what does leak and test, and try to find a pattern, and look at code, and rewatch it... :)

@poire-z
Copy link
Contributor

poire-z commented Jan 24, 2017

I see there is some JSON/lpeg (C module) involved for the stardict output decoding

For reference: just tried dict lookup with this pure LUA json decode in place of the lpeg based one, with no noticable difference in the (small) memory increase.

@oldhasbeen
Copy link
Author

oldhasbeen commented Jan 24, 2017

@poire-z
have tested: 10 suspend cycles (wait with am radio until true sleep is present) - check memory usage after each wake up.
setting "exclude this book cover from screensaver" gives resident/virtual size readings:
initially 34/110 - after 5th cycle 35/110 after 9th cycle 35/111.
setting book cover to "autostretch":
initially 35/111 - after 1st cycle 37/112 - 2nd 39/115 - 3rd 40/117 - 4th 42/118 - and until 10th no change but still 42/118 after the 10th wake up.
I have refreshed the readout after each wake up by first toggling the footer (is set to display all at once) and then change page forward and back.
About being distracted by footer info: You pay the price for being knowledgeable focussed. As designer I can hardly enjoy anything without looking on ways how to improve this and that. My wife is social worker and cannot walk in peace through town without seeing a myriad of problems.
For myself is the presence of the Crash o Meter an assuring piece of information. The former unsettling feeling of a looming crash out of the blue was far worse than the extra number that can be toggled of if too obnoxious.
Again thanks for your dedication and help.

@poire-z
Copy link
Contributor

poire-z commented Jan 24, 2017

Thank you for the tests and numbers.
For completness, could you do the same tests, same conditions, with autostretch off ?
I'll tend to think that if after a few steps, numbers stay stable, it means there's no obvious memory leak with this action.
If we ever put memory usage in the footer, would we agree the resident size (first number) is enough, and the virtual size (2nd number) is of little interest ? (I personally don't know what to make of it and how it could be correlated to the 1st: addition, factor...).

@AlanSP1
Copy link

AlanSP1 commented Jan 25, 2017

Just want to comment on too much info in footer is distraction.

I think that if we have option to turn on/off individual items in footer, we can control our "footer irritation level" to our liking. Some would have problems with many items in footer, some would like to keep eye, but we all can control that to our liking, thus having more options we can control to our liking is always better than having less options.

About virtual memory usage, I'm not sure if this is important number, it depends on system. If anything, we can split this in two options, one for resident memory, one for virtual.

@oldhasbeen
Copy link
Author

oldhasbeen commented Jan 25, 2017

@poire-z
autostretch off numbers are (same test procedure immediately done after the others):
42/118 without change during and after 10 suspend cycles.
I do not know about the absolute importance of the virtual size (2nd number) either. I could imagine however that both numbers are relevant. My dictionary lookup failed when 355/521 was reached. My RAM is 512. I would guess that with a smaller RAM the apparent trouble will set in at lower resident memory number and a higher one with a larger RAM.. If that was the case the second number may even be the more significant one.

@oldhasbeen
Copy link
Author

update: I have never seen a false sleep again since the suspend script is in lua. I am constantly in koreader and have never turned the device off. No crash or dictionary lookup failures either since knowing how much memory is allocated and restart koreader before memory becomes too tight.
With my usage pattern I had now only twice to restart. Memory leakage is still present but at least one magnitude less than in the past.

@houqp
Copy link
Member

houqp commented Mar 3, 2017

I was concerned with difficulty of removing memory info from footer in the future when memleak issue is fixed. Now that I have a pending patch for configuration updater module, this kind of configuration migration can be handled much cleaner. So I have no objection adding memory info into footer since it does help improve usability at this stage.

@oldhasbeen good to know! I will do the final test for my unexpected wakeup patch then move on to look into the memory leak issue.

@Hzj-jie
Copy link
Contributor

Hzj-jie commented Mar 27, 2017

With batterystats plugin, I still see my Kobo Aura HD wakes up randomly. Indeed, I almost have not used it during last week, but the awake time is ~4500 minutes, I.e. half of the week. I have set the auto suspension time to 15 minutes, and auto restart KOReader after crashes. But it's not likely KOReader dies once per 15 minutes.

@NiLuJe
Copy link
Member

NiLuJe commented Jul 8, 2018

I think the suspend part of this has been taken care of by now, right? ;).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests