Keypirinha / Keypirinha

A fast keystroke launcher for Windows
http://keypirinha.com
1.05k stars 21 forks source link

Realtime monitoring in the Apps package #115

Closed koteq closed 7 years ago

koteq commented 8 years ago

It would be great if there were an option to enable realtime monitoring for changes in the Apps package. Every time I install new apps I need to manually trigger Refresh Catalog action otherwise I won't be able to find newly created shortcuts.

The only launcher which known to me to have those feature is Wox and it utilizes ReadDirectoryChangesW function for that.

polyvertex commented 8 years ago

Keypirinha makes use of ReadDirectoryChangesW already to detect configuration changes in a very optimized way, so it has been considered to insert a similar feature in Keypirinha plugins API already but it eventually has been rejected.

Problem is that intensive use of ReadDirectoryChangesW can lead to high CPU usage and this API can even miss events from the file system in case it is under heavy usage. In the context of the Apps package for example, this would become a nightmare for the user without them even noticing, just because the extra_paths setting is not configured properly (i.e. too much files/apps referenced).

However, although not being a priority, a very limited similar feature may be offered in the future to allow the detection of newly installed (or uninstalled) applications by monitoring the content of the Start Menu for example, which is lightweight enough in most configs.

polyvertex commented 7 years ago

To add onto that, somehow I'm guilty of not wanting to implement half of a solution...

The ReadDirectoryChangesW API, depending on how it is used, is the most optimized way to get notified about changes in the file system from the user-land (i.e. over os-level). Problem is that it still is not fast enough. A faster way is to read file system's journal directly, which is how the OS actually does it. Unfortunately that requires Keypirinha to run in elevated privileges, which means some users would not be able to use it anymore (at work for example, where security policies are usually stronger). I cannot afford to go both way at the same time, so I'll eventually have to choose:

  1. stay in user-land and use ReadDirectoryChangesW as much as possible (less features for the future; no faster indexing"; full file indexing like Everything is not possible/not realistic)
  2. or go nuts and try a bold move by imposing user to run in elevated privileges; which means Keypirinha looses a bit more of its "portability" feature
sschuberth commented 7 years ago

Just as a thought, wouldn't it be possible to combine these two approaches by e.g. using the Everything API if it's installed, and falling back to ReadDirectoryChangesW otherwise?

polyvertex commented 7 years ago

@sschuberth The Everything API only exposes its search feature. No notification mechanism is provided. I contacted Everything's author in Feb 2016 with that reason in mind but never got any answer from him.

So I have to roll my own file system monitor for Keypirinha unfortunately. This is quite a time-demanding task and a thing on its own. Not trivial, subject to many iterations to be perfected. See the history of Everything and you'll get a glimpse.

koteq commented 7 years ago

I don't think that requesting elevated permissions and implementing Everything's features does a good architectural decision.

You said, that Everything doesn't have a api to notify about changes that happen. But maybe it will be enough just to query Everything for all the files in paths that is needed by the App package in realtime. Everything keeps all data in some kind of database so it shouldn't take too long to query the data every time when Keypirinha is brought foreground.

polyvertex commented 7 years ago

@koteq I think you're mixing things a bit. The goal here is to get notified, not to poll. And polling Everything for that matter, instead of just reading info from the file system, is a nonsense because you are just moving the problem out of KP's scope without solving it: in order to be fully operative, Everything does require elevated privileges too, so it remains the same from user's stand point (in addition to being absolutely awful conceptually-wise and I'm not even sure that transferring so much data via its IPC would not involve some difficulties at some point).

Reading the file system and building KP's own internal database is not the hard part. What is non-trivial however is to keep this database synchronized with the current state of the file system. Think path merging, symlinks, joint points, I/O errors, gaps in FS journal, ...

koteq commented 7 years ago

Well, back to ReadDirectoryChangesW then. Maybe it's a good idea to stick with it and let users configure it. For example you might create an option to allow users decide what paths they want to monitor for changes and just leave a notice that it may be cpu expensive in some edge cases.

Also you mentioned that it may be not as fast as direct fs journal reading. But the question is: do the users actually need such a fast updates? Maybe it's ok to be somewhat slow. Or maybe those slowdowns could be hidden from users somehow, maybe it's possible to do fs readings in background and constantly update current search result?

polyvertex commented 7 years ago

... it's possible to do fs readings in background and constantly update current search result?

On the home page of KP's website, it is written Battery friendly :) If possible, I would prefer to keep it that way.

Also you mentioned that it may be not as fast as direct fs journal reading. But the question is: do the users actually need such a fast updates? ...

It is not about having a maximum overkill of a feature, more about having something that "Just Works" (tm). The problem with ReadDirectoryChangesW is not that it is slow. It is fast enough actually when used properly and sparingly. But it has its limits and can become a bottleneck in some cases due to the way it passes data for example. Also, did I mention that the ReadDirectoryChangesW option would require elevated privileges too, depending on user's config?

Anyway, thanks for participating. It's just that all of these options take time to implement, no matter how good they are.

Vanav commented 7 years ago

If you need elevated privileges, you can move notification code to service. I prefer functionality over portability. Is reading journal less CPU intensive than ReadDirectoryChangesW notification?

polyvertex commented 7 years ago

@Vanav Elevated privileges are still required in order to install a service so it doesn't solve our problem. The benefit of a service in that regard is that UAC requires the user to validate access rights only at install time. Which implies no portable mode in this case.

Is reading journal less CPU intensive than ReadDirectoryChangesW notification?

Please (re)read my previous comment.

koteq commented 7 years ago

@polyvertex maybe you generalize it way too much. Maybe some ad-hoc solution will be better here.

… a … feature may be offered in the future to allow the detection of newly installed (or uninstalled) applications by monitoring the content of the Start Menu for example, which is lightweight enough in most configs.

polyvertex commented 7 years ago

@koteq I was just answering to be polite and for the sake of not leaving users in the dark regarding the current lack of this feature and why a generic solution hasn't been implemented yet in KP (since it popped up again from the chat room).

I have a cristal clear view on the problem and plan hasn't changed since my first comment since I just can't afford it ATM. So the very limited similar feature I was referring to is still the candidate.

koteq commented 7 years ago

@polyvertex oh, you should have mention it earlier so I wouldn't bother your time.

polyvertex commented 7 years ago

As of v2.10, the Apps package refreshes it's Catalog when the content of user's Desktop or Start Menu has changed.