The attached patchset includes a change to debian/rules to set -DHAVE_inotify on linux.
This patch depends on the fix for #564931 -- using inotify causes that race to happen much more frequently, since ttyplay wakes up instantly when the file is written to. BTW, the speedup and latency reduction that I'm seeing from using inotify is truely significant. -- see shy jo
From e3f0c39e33118a4ae2dcb31edae4cb8724394ab9 Mon Sep 17 00:00:00 2001 From: Joey Hess <j...@gnu.kitenet.net> Date: Tue, 12 Jan 2010 15:16:12 -0500 Subject: [PATCH 2/3] ttyplay: add inotify support When built with -DHAVE_inotify, will use linux's inotify to speed up ttyplay -p. The difference is quite significant. The old tail emulation code checked 4 times a second for new data in the ttyrecord. So when something happened, it was seen with a lag of on average, 1/8th of a second. This lag was partcularly apparent when something was being typed, resulting in playback that was blocky and slow. With inotify, there is nearly no perceivablt difference between the terminal showing the playback and the terminal where the editing takes place. It's that fast. Also, it uses less CPU when nothing is happening. :) Note: This patch really needs to be applied on top of the patch that fixes the partial record eof race in ttyplay -p. With inotify, ttyplay is *much* more likely to see partial records, since it wakes up so fast when something is being written. --- README | 8 ++++++-- ttyplay.c | 61 +++++++++++++++++++++++++++++++++++++++++++++++-------------- 2 files changed, 53 insertions(+), 16 deletions(-) diff --git a/README b/README index b34f53f..de0d625 100644 --- a/README +++ b/README @@ -4,13 +4,17 @@ Installation: % make +or if your system is a modern Linux system with inotify: + + % make CFLAGS="-DSVR4 -DHAVE_inotify" + or if your system is SVR4 system (Solaris etc.), - % make CFLAGS=-DSVR4 + % make CFLAGS="-DSVR4" or if your system supports getpt(3), - % make CFLAGS=-DHAVE_getpt + % make CFLAGS="-DHAVE_getpt" HAVE_getpt is required if your linux system uses devfs. diff --git a/ttyplay.c b/ttyplay.c index 7ca4d83..13ba7a5 100644 --- a/ttyplay.c +++ b/ttyplay.c @@ -38,6 +38,9 @@ #include <termios.h> #include <sys/time.h> #include <string.h> +#ifdef HAVE_inotify +#include <sys/inotify.h> +#endif #include "ttyrec.h" #include "io.h" @@ -45,9 +48,10 @@ typedef double (*WaitFunc) (struct timeval prev, struct timeval cur, double speed); -typedef int (*ReadFunc) (FILE *fp, Header *h, char **buf); +typedef int (*ReadFunc) (FILE *fp, const char *filename, + Header *h, char **buf); typedef void (*WriteFunc) (char *buf, int len); -typedef void (*ProcessFunc) (FILE *fp, double speed, +typedef void (*ProcessFunc) (FILE *fp, const char *filename, double speed, ReadFunc read_func, WaitFunc wait_func); struct timeval @@ -140,7 +144,7 @@ ttynowait (struct timeval prev, struct timeval cur, double speed) } int -ttyread (FILE *fp, Header *h, char **buf) +ttyread (FILE *fp, const char *filename, Header *h, char **buf) { fpos_t pos; int can_seek=0; @@ -177,12 +181,38 @@ err: } int -ttypread (FILE *fp, Header *h, char **buf) +ttypread (FILE *fp, const char *filename, Header *h, char **buf) { +#ifdef HAVE_inotify + /* Linux inotify support. This uses less CPU and is less laggy than + * using select. + * + * The inotify setup code follows. A static inotify_fd is used + * to avoid having to set up inotify each call. This assumes that + * ttypread is always called with the same filename. + */ + static int inotify_fd=-1; + struct inotify_event event; + if (filename && inotify_fd == -1) { + inotify_fd = inotify_init(); + if (inotify_fd != -1) { + if (inotify_add_watch(inotify_fd, filename, IN_MODIFY) == -1) { + inotify_fd = -1; + } + } + } +#endif + /* * Read persistently just like tail -f. */ - while (ttyread(fp, h, buf) == 0) { + while (ttyread(fp, filename, h, buf) == 0) { +#ifdef HAVE_inotify + if (inotify_fd != -1) { + read(inotify_fd, &event, sizeof(event)); /* blocks until modified */ + continue; + } +#endif struct timeval w = {0, 250000}; select(0, NULL, NULL, NULL, &w); clearerr(fp); @@ -203,7 +233,7 @@ ttynowrite (char *buf, int len) } void -ttyplay (FILE *fp, double speed, ReadFunc read_func, +ttyplay (FILE *fp, const char *filename, double speed, ReadFunc read_func, WriteFunc write_func, WaitFunc wait_func) { int first_time = 1; @@ -216,7 +246,7 @@ ttyplay (FILE *fp, double speed, ReadFunc read_func, char *buf; Header h; - if (read_func(fp, &h, &buf) == 0) { + if (read_func(fp, filename, &h, &buf) == 0) { break; } @@ -237,20 +267,20 @@ ttyskipall (FILE *fp) /* * Skip all records. */ - ttyplay(fp, 0, ttyread, ttynowrite, ttynowait); + ttyplay(fp, 0, 0, ttyread, ttynowrite, ttynowait); } -void ttyplayback (FILE *fp, double speed, +void ttyplayback (FILE *fp, const char *filename, double speed, ReadFunc read_func, WaitFunc wait_func) { - ttyplay(fp, speed, ttyread, ttywrite, wait_func); + ttyplay(fp, filename, speed, ttyread, ttywrite, wait_func); } -void ttypeek (FILE *fp, double speed, +void ttypeek (FILE *fp, const char *filename, double speed, ReadFunc read_func, WaitFunc wait_func) { ttyskipall(fp); - ttyplay(fp, speed, ttypread, ttywrite, ttynowait); + ttyplay(fp, filename, speed, ttypread, ttywrite, ttynowait); } @@ -286,6 +316,7 @@ main (int argc, char **argv) ProcessFunc process = ttyplayback; FILE *input = NULL; struct termios old, new; + const char *filename; set_progname(argv[0]); while (1) { @@ -313,8 +344,10 @@ main (int argc, char **argv) } if (optind < argc) { - input = efopen(argv[optind], "r"); + filename=argv[optind]; + input = efopen(filename, "r"); } else { + filename=NULL; input = input_from_stdin(); } assert(input != NULL); @@ -324,7 +357,7 @@ main (int argc, char **argv) new.c_lflag &= ~(ICANON | ECHO | ECHONL); /* unbuffered, no echo */ tcsetattr(0, TCSANOW, &new); /* Make it current */ - process(input, speed, read_func, wait_func); + process(input, filename, speed, read_func, wait_func); tcsetattr(0, TCSANOW, &old); /* Return terminal state */ return 0; -- 1.6.5.7
From 934e16548bc3c268a53b8d2c19d47ec8a5c77c83 Mon Sep 17 00:00:00 2001 From: Joey Hess <j...@gnu.kitenet.net> Date: Tue, 12 Jan 2010 16:43:35 -0500 Subject: [PATCH 3/3] adapt debian/rules, adding inotify support --- debian/rules | 9 ++++++++- 1 files changed, 8 insertions(+), 1 deletions(-) diff --git a/debian/rules b/debian/rules index e821f44..ed12a31 100755 --- a/debian/rules +++ b/debian/rules @@ -2,6 +2,13 @@ PACKAGE := ttyrec +ARCH_OS = $(shell dpkg-architecture -qDEB_BUILD_ARCH_OS) +ifeq ($(ARCH_OS),linux) +CFLAGS="-O2 -Wall -DSVR4 -D_XOPEN_SOURCE=500 -DHAVE_inotify" +else +CFLAGS="-O2 -Wall -DSVR4 -D_XOPEN_SOURCE=500" +endif + configure: configure-stamp configure-stamp: dh_testdir @@ -12,7 +19,7 @@ build: configure-stamp build-stamp build-stamp: dh_testdir - $(MAKE) CFLAGS="-O2 -Wall -DSVR4 -D_XOPEN_SOURCE=500" + $(MAKE) CFLAGS=$(CFLAGS) uudecode debian/sample1.tty.uue -- 1.6.5.7
signature.asc
Description: Digital signature