Rearrange packet list code
authorAlan T. DeKok <aland@freeradius.org>
Sat, 26 Nov 2011 15:08:52 +0000 (16:08 +0100)
committerAlan T. DeKok <aland@freeradius.org>
Sat, 26 Nov 2011 15:12:11 +0000 (16:12 +0100)
yank no longer returns a pointer.  No one was using it, so that
work was unnecessary.

Re-arrange the code in fr_packet_cmp() so that fewer comparisons
are necessary to disambiiguate packets.

Remove workaround for bug #35 in packet_entry_cmp().  It is
no longer necessary.

The result is a somewhat faster on in-memory performance tests.
But the callgrind output stil lshows large blocks of time
spent handling the packet lists.  Those could be optimized
some more.

src/include/packet.h
src/lib/packet.c

index 315a4ee..74e2a33 100644 (file)
@@ -51,8 +51,8 @@ RADIUS_PACKET **fr_packet_list_find(fr_packet_list_t *pl,
                                      RADIUS_PACKET *request);
 RADIUS_PACKET **fr_packet_list_find_byreply(fr_packet_list_t *pl,
                                              RADIUS_PACKET *reply);
-RADIUS_PACKET **fr_packet_list_yank(fr_packet_list_t *pl,
-                                     RADIUS_PACKET *request);
+void fr_packet_list_yank(fr_packet_list_t *pl,
+                        RADIUS_PACKET *request);
 int fr_packet_list_num_elements(fr_packet_list_t *pl);
 int fr_packet_list_id_alloc(fr_packet_list_t *pl, int proto,
                            RADIUS_PACKET *request, void **pctx);
index 1065173..ad68ce2 100644 (file)
@@ -138,21 +138,22 @@ int fr_packet_cmp(const RADIUS_PACKET *a, const RADIUS_PACKET *b)
 {
        int rcode;
 
-       if (a->sockfd < b->sockfd) return -1;
-       if (a->sockfd > b->sockfd) return +1;
+       rcode = a->id - b->id;
+       if (rcode != 0) return rcode;
 
-       if (a->id < b->id) return -1;
-       if (a->id > b->id) return +1;
+       rcode = (int) a->src_port - (int) b->src_port;
+       if (rcode != 0) return rcode;
 
-       if (a->src_port < b->src_port) return -1;
-       if (a->src_port > b->src_port) return +1;
+       rcode = (int) a->dst_port - (int) b->dst_port;
+       if (rcode != 0) return rcode;
 
-       if (a->dst_port < b->dst_port) return -1;
-       if (a->dst_port > b->dst_port) return +1;
+       rcode = a->sockfd - b->sockfd;
+       if (rcode != 0) return rcode;
 
-       rcode = fr_ipaddr_cmp(&a->dst_ipaddr, &b->dst_ipaddr);
+       rcode = fr_ipaddr_cmp(&a->src_ipaddr, &b->src_ipaddr);
        if (rcode != 0) return rcode;
-       return fr_ipaddr_cmp(&a->src_ipaddr, &b->src_ipaddr);
+
+       return fr_ipaddr_cmp(&a->dst_ipaddr, &b->dst_ipaddr);
 }
 
 int fr_inaddr_any(fr_ipaddr_t *ipaddr)
@@ -519,8 +520,6 @@ static int packet_entry_cmp(const void *one, const void *two)
        const RADIUS_PACKET * const *a = one;
        const RADIUS_PACKET * const *b = two;
 
-       if (!a || !*a || !b || !*b) return -1; /* work-around for bug #35 */
-
        return fr_packet_cmp(*a, *b);
 }
 
@@ -622,19 +621,16 @@ RADIUS_PACKET **fr_packet_list_find_byreply(fr_packet_list_t *pl,
 }
 
 
-RADIUS_PACKET **fr_packet_list_yank(fr_packet_list_t *pl,
-                                     RADIUS_PACKET *request)
+void fr_packet_list_yank(fr_packet_list_t *pl, RADIUS_PACKET *request)
 {
-       RADIUS_PACKET **packet_p;
-
-       if (!pl || !request) return NULL;
+       rbnode_t *node;
 
-       packet_p = rbtree_finddata(pl->tree, &request);
-       if (!packet_p) return NULL;
+       if (!pl || !request) return;
 
-       if (!rbtree_deletebydata(pl->tree, packet_p)) return NULL;
+       node = rbtree_find(pl->tree, &request);
+       if (!node) return;
 
-       return packet_p;
+       rbtree_delete(pl->tree, node);
 }
 
 int fr_packet_list_num_elements(fr_packet_list_t *pl)