Re: [hackers] [dmenu] inputw: improve correctness and startup performance || NRK

From: NRK <nrk_AT_disroot.org>
Date: Fri, 29 Apr 2022 08:53:38 +0600

On Fri, Mar 25, 2022 at 10:57:42PM +0100, git_AT_suckless.org wrote:
> commit 77526f756e23e362081ac807521f901f2e5cd5e6
> Author: NRK <nrk_AT_disroot.org>
> AuthorDate: Thu Mar 24 00:37:55 2022 +0600
> Commit: Hiltjo Posthuma <hiltjo_AT_codemadness.org>
> CommitDate: Fri Mar 25 22:49:07 2022 +0100
>
> inputw: improve correctness and startup performance
>
> a massive amount of time inside readstdin() is spent trying to get the
> max input width and then put it into inputw, only for it to get clamped
> down to mw/3 inside setup().
>
> it makes more sense to calculate inputw inside setup() once we have mw
> available. similar to the last patch, i see noticeable startup
> performance improvement:
>
> before -> after
> 160ms -> 60ms
>
> additionally this will take fallback fonts into account compared to the
> previous version, so it's not only more performant but also more correct.
>
> diff --git a/dmenu.c b/dmenu.c
> index cde394b..d989d39 100644
> --- a/dmenu.c
> +++ b/dmenu.c
> _AT_@ -547,8 +547,7 @@ static void
> readstdin(void)
> {
> char buf[sizeof text], *p;
> - size_t i, imax = 0, size = 0;
> - unsigned int tmpmax = 0;
> + size_t i, size = 0;
>
> /* read each line from stdin and add it to the item list */
> for (i = 0; fgets(buf, sizeof buf, stdin); i++) {
> _AT_@ -560,15 +559,9 @@ readstdin(void)
> if (!(items[i].text = strdup(buf)))
> die("cannot strdup %u bytes:", strlen(buf) + 1);
> items[i].out = 0;
> - drw_font_getexts(drw->fonts, buf, strlen(buf), &tmpmax, NULL);
> - if (tmpmax > inputw) {
> - inputw = tmpmax;
> - imax = i;
> - }
> }
> if (items)
> items[i].text = NULL;
> - inputw = items ? TEXTW(items[imax].text) : 0;
> lines = MIN(lines, i);
> }
>
> _AT_@ -614,12 +607,13 @@ static void
> setup(void)
> {
> int x, y, i, j;
> - unsigned int du;
> + unsigned int du, tmp;
> XSetWindowAttributes swa;
> XIM xim;
> Window w, dw, *dws;
> XWindowAttributes wa;
> XClassHint ch = {"dmenu", "dmenu"};
> + struct item *item;
> #ifdef XINERAMA
> XineramaScreenInfo *info;
> Window pw;
> _AT_@ -677,7 +671,12 @@ setup(void)
> mw = wa.width;
> }
> promptw = (prompt && *prompt) ? TEXTW(prompt) - lrpad / 4 : 0;
> - inputw = MIN(inputw, mw/3);
> + for (item = items; item && item->text; ++item) {
> + if ((tmp = textw_clamp(item->text, mw/3)) > inputw) {
> + if ((inputw = tmp) == mw/3)
> + break;
> + }
> + }
> match();
>
> /* create menu window */
>

Hi,

While this patch improves startup speed on larger strings, it caused a
startup performance regression when dealing with lots of small strings
*with* many missing glyphs[0].

Because this patch correctly takes fallback font into account, it'll
inevitably end up making lots of `XftFontMatch` calls, which are deadly
slow.

For example, trying to load this [1] list of emojis on my machine now
takes slightly more than half a second. Whereas before it took about
16ms.

The following are a couple different solutions I can think of:

1. (Incorrectly) stop taking fallback font into account.

2. (Incorrectly) assume `more bytes == wider string`, which is not
correct thanks to unicode.

3. Try to get the width of the unicode code-point. I've attached a quick
and dirty patch using `utf8proc_charwidth()` from libutf8-proc. The
patch was just to confirm my hypothesis and not to be taken seriously.

I'm not too well versed on unicode so I cannot tell how difficult
rolling such a function ourself would be. But some quick searches seems
to indicate it's not going to be trivial at all, specially if we take
"grapheme clusters" into account.

So this option is probably getting ruled out.

4. The final option; just remove the dynamic input bar width entirely.
If you're unsure what I'm talking about, try the following:

        head -c 65536 < /dev/urandom | base64 | fold -w 1 | dmenu

You will notice that the width of the input bar is rather small, which
allows for more options to fit into the screen.

Now try replacing `-w 1` with `-w 30`, you will notice the width got
bigger. Now try `-w 200`, the width gets bigger, but will get clamped
down to 1/3rd of the monitor width.

My suggestion here is to just have a consistent input bar width which
can be configured via config.h and cli arg. So for example:

        static float input_bar_percent = 0.24f;

This would make the input bar width always 24% of the monitor width.
I've attached a patch for this as well. It's simpler and gives a more
static/predicable ui.

[0]: https://github.com/bakkeby/dmenu-flexipatch/issues/11
[1]: https://github.com/LukeSmithxyz/voidrice/blob/master/.local/share/larbs/emoji

- NRK

Received on Fri Apr 29 2022 - 04:53:38 CEST

This archive was generated by hypermail 2.3.0 : Fri Apr 29 2022 - 05:00:32 CEST