Skip to content
This repository has been archived by the owner on Dec 1, 2021. It is now read-only.

Image reloading is relatively slow #162

Open
rien333 opened this issue Aug 5, 2019 · 7 comments
Open

Image reloading is relatively slow #162

rien333 opened this issue Aug 5, 2019 · 7 comments
Labels
Feature New feature request
Milestone

Comments

@rien333
Copy link
Contributor

rien333 commented Aug 5, 2019

When compared to sxiv, which uses inotify to monitor file changes, imv reloads images relatively slow. Apparently it polls the current file path every second or so. I believe inotify was swapped out because of #67, making imv less linux specific.

Nothing wrong with cross-platform support, but the current implementation is bit eh compared to the possibilites of inotify. First of, the method that uses stat is more resource intensive. This might be partly why sxiv shows 0% cpu usage (to be expected from a simple image viewer), while imv can show up to 1% on my machine. Nothing too bad, but given that I always have imv running, a simpler file change monitoring solution is definitely something I would benefit from. And secondly, the current method is obviously considerably slower (sxiv is pretty much instantaneous on file changes).

Would it be possible to optionally configure imv to be build with inotify for systems that have it available? That way, cross-platform support can be retained, and linux users can gain a bit of speed and freed resources. Shouldn't be extremely hard given that inotify was apparently once supported and sxiv also has code for it. Looking at #67, using the best option available was once intended I think?

@rien333 rien333 changed the title inotify reload code is relatively slow Image reloading is relatively slow Aug 5, 2019
@rien333
Copy link
Contributor Author

rien333 commented Aug 5, 2019

If I checkout d0b041f (the last commit before inotify was dropped) , reloading is instantaneous. CPU usage is still a little high though (but maybe a tiny bit better?), so maybe inotify is not related to that.

@rien333
Copy link
Contributor Author

rien333 commented Aug 6, 2019

Alternatively, some image viewers supports quick reloading by handling some kill signal. This would be fine for my use case (an album art viewer that reloads quickly on song change). I could even submit a PR for this, I already have a sort of hacky solution.

@eXeC64
Copy link
Owner

eXeC64 commented Aug 7, 2019

Using inotify inside of #ifdef __linux__ sounds reasonable enough to me. I'd rather opt-in to platform specific features where supported than use signals as a hack, so long as the current approach remains as a fallback for BSDs etc.

This is something to go into the v4 branch I've been casually working on recently, which is a fairly big refactor of imv.

@eXeC64 eXeC64 added this to the v4.0.0 milestone Aug 12, 2019
@eXeC64
Copy link
Owner

eXeC64 commented Aug 19, 2019

All the major work for v4 is done, so I'd like to start doing the final clean up to prepare it for release. Therefore I'm afraid I'm pushing this improvement to v4.1.

@eXeC64 eXeC64 modified the milestones: v4.0.0, 4.1.0 Aug 19, 2019
@eXeC64 eXeC64 added the Feature New feature request label Aug 24, 2019
@rien333
Copy link
Contributor Author

rien333 commented Nov 23, 2019

I am working on this, btw, in case anyone was planning to do this. Almost done, but will probably need some reviewing.

@eXeC64
Copy link
Owner

eXeC64 commented Nov 23, 2019

Sounds good. I started on this, but did very little. I'll review when you open a PR

@rien333
Copy link
Contributor Author

rien333 commented May 8, 2021

@eXeC64 this may need a milestone update 😊, hopefully that will attract the attention of somemone actually able to finish this PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Feature New feature request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants