Skip to content

Conversation

@N-R-K
Copy link
Collaborator

@N-R-K N-R-K commented Jul 22, 2022

This pretty much reworks the entire icon system. Some notable changes:

  • The extensions are put into a statically generated hash-table instead
    of a sorted array. We use Robin-Hood insertion to reduce the max probe
    length. Currently we need to probe only 2 slots for O_EMOJI and only
    3 for O_NERD/O_ICONS.
  • I've opted not to use a perfect-hash since the perfect hashes
    generated by gperf used some
    huge lookup table. The hash function also wasn't as minimal as I'd
    like.
  • Colors are now using X-Macros. This should speed up startup since we
    don't have to search icons_ext linearly to find unique colors.
  • The hash-table generator outputs a more space optimized struct icon_pair using a char array instead of char pointer. This brings
    down the binary size from 145KiB when using O_NERD down to
    137KiB.
  • Some unnecessary duplication and indirection has been reduced by using
    the ICON_STR() macro.

N-R-K added 2 commits July 22, 2022 17:08
This pretty much reworks the entire icon system. Some notable changes:

* The extensions are put into a statically generated hash-table instead
  of a sorted array. We use Robin-Hood insertion to reduce the max probe
  length. Currently we need to probe only 2 slots for `O_EMOJI` and only
  3 for `O_NERD`/`O_ICONS`.
* I've opted not to use a perfect-hash since the perfect hashes
  generated by [`gperf`](https://www.gnu.org/software/gperf) used some
  huge lookup table. The hash function also wasn't as minimal as I'd
  like.
* Colors are now using X-Macros. This should speed up startup since we
  don't have to search `icons_ext` linearly to find unique colors.
* The hash-table generator outputs a more space optimized `struct
  icon_pair` using a char array instead of char pointer. This brings
  down the binary size from `145KiB` when using `O_NERD` down to
  `137KiB`.
* Some unnecessary duplication and indirection has been reduced by using
  the `ICON_STR()` macro.
@jarun jarun merged commit 117025c into jarun:master Jul 22, 2022
@jarun
Copy link
Owner

jarun commented Jul 22, 2022

Simply awesome!!! 👍

@jarun
Copy link
Owner

jarun commented Jul 22, 2022

@N-R-K Can you please update the ToDo list?

@N-R-K N-R-K deleted the icon_rework_squashed branch July 22, 2022 17:09
@N-R-K N-R-K mentioned this pull request Jul 22, 2022
6 tasks
@jarun
Copy link
Owner

jarun commented Jul 22, 2022

If I try to bypass make (to use musl-gcc) I get the following error:

musl-gcc -march=native -O3 -DNORL -DNOLC -DNOMOUSE -DNOBATCH -DTOURBIN_QSORT -DNOSSN -DNOUG -DNOX11 -DEMOJI -std=c11 -Wall -Wextra -Wshadow -I../netbsd-curses/libcurses -I../musl-fts -o nnn src/nnn.c -Wl,-Bsymbolic-functions -lpthread -L/opt/nnn-libs -lcurses -lterminfo -lfts -static
src/nnn.c:128:10: fatal error: icons-generated.h: No such file or directory
  128 | #include "icons-generated.h"
      |          ^~~~~~~~~~~~~~~~~~~
compilation terminated.

What should I use instead of -DEMOJI?

@jarun
Copy link
Owner

jarun commented Jul 22, 2022

The following works:

make O_EMOJI=1
musl-gcc -march=native -O3 -DNORL -DNOLC -DNOMOUSE -DNOBATCH -DTOURBIN_QSORT -DNOSSN -DNOUG -DNOX11 -DEMOJI -std=c11 -Wall -Wextra -Wshadow -I../netbsd-curses/libcurses -I../musl-fts -o nnn src/nnn.c -Wl,-Bsymbolic-functions -lpthread -L/opt/nnn-libs -lcurses -lterminfo -lfts

@N-R-K
Copy link
Collaborator Author

N-R-K commented Jul 22, 2022

If I try to bypass make (to use musl-gcc) I get the following error:

musl-gcc -march=native -O3 -DNORL -DNOLC -DNOMOUSE -DNOBATCH -DTOURBIN_QSORT -DNOSSN -DNOUG -DNOX11 -DEMOJI -std=c11 -Wall -Wextra -Wshadow -I../netbsd-curses/libcurses -I../musl-fts -o nnn src/nnn.c -Wl,-Bsymbolic-functions -lpthread -L/opt/nnn-libs -lcurses -lterminfo -lfts -static
src/nnn.c:128:10: fatal error: icons-generated.h: No such file or directory
  128 | #include "icons-generated.h"
      |          ^~~~~~~~~~~~~~~~~~~
compilation terminated.

What should I use instead of -DEMOJI?

First the "generator" needs to be built:

$ gcc -DICONS_GENERATE -DEMOJI -o src/icons-hash-gen src/icons-hash.c

Then the hash-table needs to be generated with the generator:

$ ./src/icons-hash-gen > src/icons-generated.h

Now you should be able to run the compile command manually as long as both the generator and nnn is build with the same icon set (-DEMOJI).

The makefile already takes care of this. So just doing make O_EMOJI=1 src/icons-generated.h to generate the hash-table and then manually compiling nnn with -DEMOJI would also work.

@jarun
Copy link
Owner

jarun commented Jul 22, 2022

Settled with:

CC=musl-gcc make O_EMOJI=1 src/icons-generated.h
musl-gcc -march=native -O3 -DNORL -DNOLC -DNOMOUSE -DNOBATCH -DTOURBIN_QSORT -DNOSSN -DNOUG -DNOX11 -DEMOJI -std=c11 -Wall -Wextra -Wshadow -I../netbsd-curses/libcurses -I../musl-fts -o nnn src/nnn.c -Wl,-Bsymbolic-functions -lpthread -L/opt/nnn-libs -lcurses -lterminfo -lfts -static

Thank you!

@N-R-K
Copy link
Collaborator Author

N-R-K commented Jul 22, 2022

Updated the performance section on the wiki :)

Also note that I still see some room for improvements. For example a lot of the extensions use the same icon. So instead of wasting 4 bytes (for O_NERD and O_ICONS) or 9 bytes (for O_EMOJI) per slot, we can store the unique icons in a separate array and store a single byte index into that array.

There's also the possibility of using a minimal-perfect-hash, which can reduce memory usage by shrinking the table size but might make the hash-function/lookup slower. I still have lot more to learn and research on this topic.

I'll see if I can make some time next weekend to implement and benchmark some of these ideas. If they turn out to be overall improvement, then I'll open the PR.

@jarun
Copy link
Owner

jarun commented Jul 22, 2022

Thank you! Keep it up!

@jarun
Copy link
Owner

jarun commented Jul 22, 2022

With the above options and musl-gcc, dynamically linked binary size:

-rwxrwxr-x 1 vaio vaio 116632 Jul 22 23:33 nnn*

Static:

-rwxrwxr-x 1 vaio vaio 399504 Jul 22 23:04 /usr/local/bin/nnn*

Great improvement!

@github-actions github-actions bot locked and limited conversation to collaborators Aug 22, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants