search

davatorium / rofi

Rofi: A window switcher, application launcher and dmenu replacement
https://davatorium.github.io/rofi/
Other
13.28k stars 612 forks source link

Warning/error on "Enter" with window switcher: Unhandled keypress in global keyhandler #347

Closed blueyed closed 8 years ago

blueyed commented 8 years ago

Version

Git master, bisected to afc056ecc50bb6f082356605ee6e82121a383d48.

Configuration

% xrdb -q | grep -i rofi
rofi.bg-active: #002b36
rofi.bg-urgent: #002b36
rofi.key-run:   Alt-F2
rofi.key-window:    Alt-F1

Steps to reproduce

Rofi is launched in daemon mode.
listening to the following keys:
    * window on Alt-F1
    * run on Alt-F2
    * ssh on <unspecified>
Warning: Unhandled keypress in global keyhandler, keycode = 36 mask = 0
DaveDavenport commented 8 years ago

I will take a look at it. But in our move to glib (already merged, causing this bug) and xcb we will remove the daemon mode. Given it duplicates functionality of other small tools (or WM).

DaveDavenport commented 8 years ago

Should be fixed.

blueyed commented 8 years ago

we will remove the daemon mode

Hmm.. isn't it better to use a running daemon, instead of starting the program anew on every invocation?

DaveDavenport commented 8 years ago

no.. rofi isn't that heavy that startup time is an issue. also in daemon mode, not a lot keeps running. But this is functionality that rofi duplicates with window manager/keybinding tools.. Lets keep things KISS..

blueyed commented 8 years ago

that heavy

sure, but it adds up. You could still using something like gpg-agent that gets started automatically.

It's not about the bindings, but the startup for request serving. (I am coming from looking at strace output from it, but not recently)

carnager commented 8 years ago

how does it add up. it's instant without daemon mode. not noticable at all. also: how often do you use rofi. even as a heavy user surely not more than every couple of minutes average.

DaveDavenport commented 8 years ago

If you give me clear numbers showing that it is useful to keep daemon mode, I will keep it. (in terms of performance)

But the benefits should out way the costs. It is legacy code remaining from the simpleswitcher area that I personally never used (as I think it is unneeded to handle global keybindings in the app itself if your Window manager can do this better), moving to XCB it seems to be the right time to remove this (as it avoid having to re-implement and test a reasonable amount of code).

Regrettable github does not have a voting system on issues :(

DaveDavenport commented 8 years ago

A quick test.. 50 ms from cold start to show of window and 41 ms from release keyboard in daemon mode.

Will retest on low end arm hardware later.

blueyed commented 8 years ago

The load is interesting in this context, too - a delay caused by keyboard signal processing is better than loading files and computing state.

But I can see the point that removing it is easier to maintain.

blueyed commented 8 years ago

The issue itself is fixed, thanks!

DaveDavenport commented 8 years ago

a normal operating system will keep rofi and dependencies cached. (cairo, xcb, pango should be 'live' if you have anything using this running and a lot of things use this). Rofi if you use it more then once a day should also be life and cached in memory.

On the other hand, rofi will (even in daemon mode) rescan executables and more when opening view so that file-io, computing state and more won't really change. So the only thing that is kept is the connection to the X server, thread pool (not the actual threads) and configuration. This takes around 6ms to start.

Unsure what you mean by keyboard signal processing. but loading files and computing state will happen in either case.

As said before, if there is a good reason I will keep /reimplement it no problem. So far I haven't seen it.

blueyed commented 8 years ago

I would imagine that the window lister is pretty cheap on disk io?! And I've thought there was some internal window re-usage (or has that been removed?).

(just look at what strace gives you for a startup; while cached, it might be good to skip. That's the systray app style?!)

DaveDavenport commented 8 years ago

owh sarcasm?! This is the way to have a productive discussion?! < /sarcasm >

Old simpleswitcher created a fork on each caught global keypress, then created the view and after handled the fork was destroyed. I don't think there was any window re-use there. Also later versions destroyed it when you where done. The reason a window is kept around for a bit is that re-drawing (when switching between modi and so) causes less visual 'glitching´ when destroying and re-creating a window. If creating a window has so much impact on performance, please show me numbers.

I can look at strace output (done many many times) but how does this give me any performance information or user experience? I already tried to explain that most data accessed by rofi (that you see in strace) will be life in memory anyway. If you are so memory constraint this isn't, you do not want to keep this occupied by running rofi 24/7 anyway!

I tried to get some numbers about startup time, can you get something I can actually quantify? If not having a daemon mode really affects the user-experience I will keep it (I have been saying this over and over, but so far I have not seen a good reason) but show me something more then 'look at strace output'.

What is the 'systray app style' where is this described? I personally HATE application that do not close when closed but minimize to 'systray', there are only a few cases where it is useful imho (e.g. something that keeps on working for you and requires interaction like a music player or your wireless network monitor)

blueyed commented 8 years ago

I've not meant to be sarcastic, sorry. With "systray app style" I've meant only programs having a daemon in general.

I do not want to argue further, just wanted to let this being considered.

Daemon mode would only make sense with window caching.

The reason a window is kept around for a bit is that re-drawing (when switching between modi and so) causes less visual 'glitching´ when destroying and re-creating a window. If creating a window has so much impact on performance, please show me numbers.

I think visual glitches are an indicator for a performance impact. (My point is also that while something is fast, it does not mean to be efficient, and uses more power/battery for example).

DaveDavenport commented 8 years ago

I think visual glitches are an indicator for a performance impact.

No, you seem to miss the point why the window is kept around and making assumptions. The window is kept around because the user hasn't closed it yet, it just switched modi (so it needs a redraw). The reason why there is extra code to 'keep it around' is more a side-effect of how simpleswitcher was written. It has little todo with 'performance'. Again in daemon mode the window has never been kept around and there is no point doing this.

(My point is also that while something is fast, it does not mean to be efficient, and uses more power/battery for example).

Again, this is an assumption without any proof. Let try to reason about this: What takes power on a modern pc are wake ups; The CPU being pulled out of deep sleep state. What moderns PC do is race to idle: when they wake up, try to do all the work as fast as possible and go back into deep sleep.

So the question is what is more expensive the extra startup time or having an extra application wake up on keyboard events. Doing a strace on rofi will show that if you bind Alt+F2 to run dialog in daemon mode, rofi will wake up on every Alt key press. Here I must admit I do not know the details, if current implementation in kernel/X11 groups these events to reduce extra wakeups? This could be investigated.

But I don't see how the tiny amount of 'cpu time' consumed by rofi , either in daemon mode or fresh start every time will have any noticeable impact on battery life. A quick check with perf (running on the full system) shows that the time spend by rofi in 10 minutes and calling it 10 times (this is very high use) only shows ( with less then the displayed accuracy amount of the cpu time) for the rofi_view_update callback in rofi. E.g. this is independent of what modi rofi is ran.

Again please proof me wrong.

blueyed commented 8 years ago

or having an extra application wake up on keyboard events

I think it's fine to remove the global keyboard handling: so the daemon would only get woken up when the client gets run (which connects to the daemon, which would reuse (most of) the window). But if there's no reason for caching the window across runs, then this is also pointless, of course. (that would mean to save on the rofi_view_update callback, wouldn't it?)

DaveDavenport commented 8 years ago

rofi_view_update is basically the call that makes cairo repaint. This involves rendering text etc.. this is the logical thing to take most cpu. No caching the window has nothing todo with the cost of rofi_view_update overhead. Try to understand that the caching we keep talking about is NOTHING more then keeping a XWindow around. On the whole scheme of things this is completely irrelevant, if you want to check this do a profile on rofi using perf or callgrind.

so the daemon would only get woken up when the client gets run (which connects to the daemon, which would reuse (most of) the window).

Please don't get to focused on this caching of the XWindow. First check if it is relevant at all.

blueyed commented 8 years ago

Thanks for explaining and sorry for the noise then.

The main issue arises from my setup scanning executables for the "run" mode, which is enabled, but not selected initially: it loads the information not on display, but startup. And I've imagined this could be cached for longer (apart from making it more lazy).

I will switch to no-daemon mode now myself.

btw: I've just noticed ~/.cache/rofi-3.runcache, too.

blueyed commented 8 years ago

btw: I had to use pkill -HUP rofi also to make it pick up changes to xrdb. Not necessary in non-daemon mode.

DaveDavenport commented 8 years ago

you can also add configuration to ~/.config/rofi/config then you do not need to xrdb anymore. The runcache is history.

blueyed commented 8 years ago

I use Xresources/xrdb for different color variances (solarized light/dark):

#if defined(SOLARIZED_LIGHT) || defined(SOLARIZED_DARK)
#ifdef SOLARIZED_DARK
#include ".dotfiles/lib/base16/base16-solarized.dark.256.xresources"
rofi.bg-active:         #002b36
rofi.bg-urgent:         #002b36
#endif

#ifdef SOLARIZED_LIGHT
#include ".dotfiles/lib/base16/base16-solarized.light.256.xresources"
rofi.bg-active:         #fdf6e3
rofi.bg-urgent:         #fdf6e3
#endif

Therefore it's good to go through xrdb.

github-actions[bot] commented 4 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.