Package: dhex
Version: 0.68-2
Severity: important
Tags: upstream patch

Hello,

I have been looking for a useful console-based hex editor and dhex is a good
candidate, except that it uses a lot of CPU when idling and waiting for the
user's next key press. This happens with an input file of any size
(including
an empty file). The CPU usage ranges from 30% on an i5 to 50% on an AMD
E-350.

I have run dhex through a debugger and looked at the source and the problem
appears to be entirely due to its input processing. The function getkey() in
dhex-0.68/input.c consists of a busy-loop waiting for input, delaying for a
maximum of one microsecond (through usleep(1)).

However, the program still seems to function correctly when asynchronous
input
is disabled. I have tested editing, searching, go to position, etc. This is
assuming ncurses is in cbreak() mode which mine appears to be by default (a
call to cbreak() could also be included). Here is a patch that disables
asynchronous input:

diff --git a/dhex-0.68-orig/main.c b/dhex-0.68/main.c
index e6f014c..21cef37 100644
--- a/dhex-0.68-orig/main.c
+++ b/dhex-0.68/main.c
@@ -525,7 +525,6 @@ int main(int argc,char** argv)
                output->win=initscr();
                pairsinit(output);
                noecho();
-               nodelay(output->win,1);
                if (keyboardsetupreq) keyboardsetup(output,configfile);
                readbuf(buf1,0);
                firstpos1=cursorpos1;

Of course the getkey() input routine could be much simpler if it knows that
getkey() will block, and I recommend that it be rewritten (by someone who
knows
more about ncurses than I). It seems like the routine is trying to handle
multi-character extended keys (arrow keys, function keys) by receiving
blocks
of characters at once and comparing them with a list. This should be
equivalent
to blocking on each getch() call and searching through the list after each
character (which is what my patch will induce, since getch() never returns
ERR
in a blocking context).

If that patch seems too risky, at the very least, the usleep(1) call
(input.c
line 72) could be changed to usleep(1000) to sleep for a millisecond
instead of
a microsecond. This still uses CPU but normally less than 5%. The program
still
functions without apparent latency (the maximum latency introduced would be
1
ms), and it will never miss any input.

diff --git a/dhex-0.68-orig/input.c b/dhex-0.68/input.c
index 34e2e93..8ead185 100644
--- a/dhex-0.68-orig/input.c
+++ b/dhex-0.68/input.c
@@ -69,7 +69,7 @@ tInt16 getkey(tKeyTab* pKeyTab,tBool inputfield)
// =1 this is an inputfield. w
                                lastch=ch;
                                done=1; // a key was pressed
                        } else {
-                               usleep(1);
+                               usleep(1000);
                                donecnt=donecnt-partial;
                                done=!donecnt;
                        }

In this second case, the initial donecnt value should perhaps be
initialized to
a smaller value (for less async getch() calls since there is a bigger delay
between each one). It is curious however that donecnt is never reset to a
nonzero value once it hits zero; is that another bug?

All in all, I recommend that one or the other of my patches be adopted, and
perhaps someone else take a look at the getkey() function. It seems in need
of
review. Cheers.

~ dwk



-- System Information:
Debian Release: jessie/sid
  APT prefers testing
  APT policy: (500, 'testing'), (1, 'experimental')
Architecture: amd64 (x86_64)

Kernel: Linux 3.2.0-4-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_CA.UTF-8, LC_CTYPE=en_CA.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash

Versions of packages dhex depends on:
ii  libc6        2.17-3
ii  libncurses5  5.9+20130608-1
ii  libtinfo5    5.9+20130608-1

dhex recommends no packages.

dhex suggests no packages.

-- no debconf information

Reply via email to