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

concerns about calling the plugin at every page turn #2

Open
EastEriq opened this issue Jul 20, 2023 · 12 comments
Open

concerns about calling the plugin at every page turn #2

EastEriq opened this issue Jul 20, 2023 · 12 comments

Comments

@EastEriq
Copy link

Maybe non-issues.

  1. accessing so often the database files in the lifetime of the ereader may damage the NAND
  2. rewriting sync.log thousands of times in the lifetime of the ereader may damage the NAND
  3. increased cpu usage and therefore battery consumption

For most use cases, it may be enough to call the plugin only when a book file is closed (or KR is closed). At least in my use, I wouldn't return to OS with KR open, AND expect to see the book percentage updated.

@ckilb
Copy link
Owner

ckilb commented Jul 21, 2023

In my use case I usually never close books or KOReader itself. I just press the Home Button, either to switch to another book or do sth else. In that case I prefer to have the progress updated.

I doubt (but don’t know if) the syncing will be able to damage the device. KOReader itself, and probably the stock software itself, is doing a lot more operations and logging.

Nevertheless you are right if you say there’s room for improvement we should definitely fill - even if it may „only“ for the sake of better battery life.

For once it might make sense to get rid of the script completely and just perform the database operations directly from KOReader.

Also it probably makes sense to not perform the sync operations if the page hasn’t changed.
Finally the SQL queries could be improved and combined (via JOINs).

@ckilb
Copy link
Owner

ckilb commented Jul 22, 2023

pushed a new version. now no logs will be written if there's no unexpected behaviour and progress updates will be done in a single database query. this should be much more performant than before.

@EastEriq
Copy link
Author

WFM, thanks!

@Silther
Copy link

Silther commented Aug 22, 2023

I don't know if it's possible, but the Pocketbook Reader syncs all it's progress with the Pocketbook Cloud after the device entered standby or was switched off.
Is it possible to sync the database right before that happens?

@ckilb
Copy link
Owner

ckilb commented Aug 22, 2023

@Silther this should not be necessary. The plugin makes KOReader to update the progress with each page turn. So if you turn the page, Pocketbook's database should already be up to date. So the cloud syncing should, too, upload the most recent progress state. Is that not working in your case?

@Silther
Copy link

Silther commented Aug 22, 2023

I just thought that would further reduce the number of views if the plugin is not started on every page change.

@liskin
Copy link
Collaborator

liskin commented Nov 6, 2023

It's possible to use onFlushSettings instead of onPageUpdate but it seems it's triggered too late so when pressing the Home button, PocketBook's homescreen shows stale data, even though I removed the UIManager:scheduleIn invocation. Can't think of another way to avoid NAND writes while preserving acceptable UX. 😞

@ckilb
Copy link
Owner

ckilb commented Nov 6, 2023

hey! i just merged @liskin's PR which uses folders & filenames instead of the book title - because that's the better approach. unfortunately for this two SQL statements will be executed, so I assume the overall process is a bit more heavy than before again.

page turns are still fast though (at least on my device) so I'm not sure if we have a real issue here. in general I think that onPageUpdate is the best event to listen on - simply because it's the most reliable. even if the device suddenly turns off or KOReader crashes - the process will be synced.

what we could definitly do is to implement some cache so we only fetch the book_id if we don't already know it.

if you have more ideas, let me know.

@liskin
Copy link
Collaborator

liskin commented Nov 6, 2023

hey! i just merged @liskin's PR which uses folders & filenames instead of the book title - because that's the better approach. unfortunately for this two SQL statements will be executed, so I assume the overall process is a bit more heavy than before again.

The sqlite DB has indices for all the 3 queries involved so I'd expect the performance/battery impact to be negligible. Not sure it's worth implementing the cache just for book_ids. Is there a way to benchmark/profile this just to be sure? (I'm a total Lua/KOReader newbie)

@liskin
Copy link
Collaborator

liskin commented Nov 6, 2023

Anyway, if people want to test what the behaviour is like without hooking into onPageUpdate, take #9 and just comment out onPageUpdate, relying on onFlushSettings instead. It kinda works but there's stale data on the PocketBook homescreen until e.g. a library is opened and closed again.

@Silther
Copy link

Silther commented Nov 6, 2023

page turns are still fast though

does koreader have to wait for the plugin to be ready or is it about the extra work for the processor?

@liskin
Copy link
Collaborator

liskin commented Sep 9, 2024

page turns are still fast though

does koreader have to wait for the plugin to be ready or is it about the extra work for the processor?

Yeah I believe it's all synchronous, so page turns being fast means the plugin is doing its job quickly. :-)

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

No branches or pull requests

4 participants