search

ckilb / pocketbooksync.koplugin

A Pocketbook app that sync the reading status between KOReader and Pocketbook's original software.
54 stars 5 forks source link

concerns about calling the plugin at every page turn #2

Open EastEriq opened 1 year ago

EastEriq commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

WFM, thanks!

Silther commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

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

liskin commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

Anyway, if people want to test what the behaviour is like without hooking into onPageUpdate, take https://github.com/ckilb/pocketbooksync.koplugin/pull/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 commented 1 year ago

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 commented 2 months ago

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. :-)