Re: [exprimental patch] wakeup instead of polling for dns

Johannes Hofmann <[email protected]>
Newsgroups gmane.comp.web.dillo.devel
Message-ID <[email protected]>
On Sun, Feb 12, 2012 at 08:44:04PM +0100, Johannes Hofmann wrote:
> Hi Jorge,
> 
> On Fri, Feb 10, 2012 at 01:40:10PM -0300, Jorge Arellano Cid wrote:
> > Hi Johannes,
> > 
> > On Mon, Feb 06, 2012 at 09:45:44PM +0100, Johannes Hofmann wrote:
> > > Hi,
> > > 
> > > attached experimental patch replaces the polling for DNS answers
> > > with the wakeup mechanism fltk provides.
> > > What do you think?
> > 
> >   Well dns resolving is delicate, but not sacred ;)
> > 
> >   AFAIS  the main advantage would be a near 0.1 second speedup on
> > average  when  displaying  non dns-cached pages, (0.2 best case).
> > Which is significative.
> > 
> >   Please let me know if there's another advantage I miss.
> 
> Well, I don't like the polling :-)
> There might be an increased power consumption related to the
> 5 times/s timer, but I'm not sure whether it really matters.
> 
> > 
> >   The  cost  is using an FLTK mechanism (awake) which we don't in
> > the rest of the code.
> 
> Yes, plus we need to switch on locking for all fltk just for the dns
> thing. That's the major drawback I think.
> 
> > 
> >   It looks clean. It should be tested on different platforms/OS.
> > 
> >   BTW,  dns.c has code for a blocking server (not threaded). Does
> > this  awake() call work OK when called from the same main thread?
> > Is  this by design or by chance? Maybe it would be safer to avoid
> > calling awake() in this case.
> 
> I didn't test it yet. I still want to try other approaches that
> would avoid switching on the fltk threading stuff with the
> Fl::lock() call.

... and here comes the patch.

_______________________________________________
Dillo-dev mailing list
[email protected]
http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev
dns_wakeup2.diff (text/plain, 3.8 KB)
diff -r 35e4c15029a0 src/dns.c
--- a/src/dns.c	Thu Feb 02 20:35:28 2012 +0100
+++ b/src/dns.c	Tue Feb 14 21:34:52 2012 +0100
@@ -24,11 +24,13 @@
 #endif
 
 
+#include <assert.h>
 #include <netdb.h>
 #include <sys/types.h>
 #include <sys/socket.h>
 #include <arpa/inet.h>
 #include <netinet/in.h>
+#include <fcntl.h>
 #include <errno.h>
 #include <unistd.h>
 #include <stdlib.h>
@@ -77,7 +79,7 @@
 /*
  * Forward declarations
  */
-static void Dns_timeout_client(void *data);
+static void Dns_timeout_client(int fd, void *data);
 
 /*
  * Local Data
@@ -88,6 +90,7 @@
 static int dns_cache_size, dns_cache_size_max;
 static GDnsQueue *dns_queue;
 static int dns_queue_size, dns_queue_size_max;
+static int dns_notify_pipe[2];
 
 
 /* ----------------------------------------------------------------------
@@ -169,7 +172,7 @@
  */
 void a_Dns_init(void)
 {
-   int i;
+   int res, i;
 
 #ifdef D_DNS_THREADED
    MSG("dillo_dns_init: Here we go! (threaded)\n");
@@ -187,6 +190,11 @@
 
    num_servers = D_DNS_MAX_SERVERS;
 
+   res = pipe(dns_notify_pipe);
+   assert(res == 0);
+   fcntl(dns_notify_pipe[0], F_SETFL, O_NONBLOCK);
+   a_Timeout_add_fd(dns_notify_pipe[0], Dns_timeout_client, NULL);
+
    /* Initialize servers data */
    for (i = 0; i < num_servers; ++i) {
       dns_server[i].channel = i;
@@ -325,6 +333,8 @@
    dns_server[channel].addr_list = hosts;
    dns_server[channel].ip_ready = TRUE;
 
+   write(dns_notify_pipe[1], "A", 1);
+
    return NULL;                 /* (avoids a compiler warning) */
 }
 
@@ -345,10 +355,6 @@
    dFree(dns_server[channel].hostname);
    dns_server[channel].hostname = dStrdup(hostname);
 
-   /* Let's set a timeout client to poll the server channel (5 times/sec) */
-   a_Timeout_add(0.2,Dns_timeout_client,
-                 INT2VOIDP(dns_server[channel].channel));
-
 #ifdef D_DNS_THREADED
    /* set the thread attribute to the detached state */
    if (!thrATTRInitialized) {
@@ -457,27 +463,26 @@
 }
 
 /*
- * This is a timeout function that
- * reads the DNS results and resumes the stopped jobs.
+ * This function is called on the main thread and
+ * reads the DNS results.
  */
-static void Dns_timeout_client(void *data)
+static void Dns_timeout_client(int fd, void *data)
 {
-   int channel = VOIDP2INT(data);
-   DnsServer *srv = &dns_server[channel];
+   int i;
+   char buf[16];
 
-   if (srv->ip_ready) {
-      if (srv->addr_list != NULL) {
+   while (read(dns_notify_pipe[0], buf, sizeof(buf)) > 0);
+
+   for (i = 0; i < num_servers; ++i) {
+      DnsServer *srv = &dns_server[i];
+
+      if (srv->ip_ready && srv->addr_list != NULL) {
          /* DNS succeeded, let's cache it */
          Dns_cache_add(srv->hostname, srv->addr_list);
+         Dns_serve_channel(i);
       }
-      Dns_serve_channel(channel);
-      Dns_assign_channels();
-      a_Timeout_remove(); /* Done! */
-
-   } else {
-      /* IP not already resolved, keep on trying... */
-      a_Timeout_repeat(0.2, Dns_timeout_client, data);
    }
+   Dns_assign_channels();
 }
 
 
diff -r 35e4c15029a0 src/timeout.cc
--- a/src/timeout.cc	Thu Feb 02 20:35:28 2012 +0100
+++ b/src/timeout.cc	Tue Feb 14 21:34:52 2012 +0100
@@ -41,3 +41,7 @@
    /* in FLTK, timeouts run one time by default */
 }
 
+void a_Timeout_add_fd(int fd, TimeoutFdCb_t cb, void *cbdata)
+{
+   Fl::add_fd(fd, FL_READ, cb, cbdata);
+}
diff -r 35e4c15029a0 src/timeout.hh
--- a/src/timeout.hh	Thu Feb 02 20:35:28 2012 +0100
+++ b/src/timeout.hh	Tue Feb 14 21:34:52 2012 +0100
@@ -6,10 +6,12 @@
 #endif /* __cplusplus */
 
 typedef void (*TimeoutCb_t)(void *data);
+typedef void (*TimeoutFdCb_t)(int fd, void *data);
 
 void a_Timeout_add(float t, TimeoutCb_t cb, void *cbdata);
 void a_Timeout_repeat(float t, TimeoutCb_t cb, void *cbdata);
 void a_Timeout_remove();
+void a_Timeout_add_fd(int fd, TimeoutFdCb_t cb, void *cbdata);
 
 
 #ifdef __cplusplus
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.