CLEANUP: fdtab: flatten the struct and merge the spec struct with the rest

The "spec" sub-struct was using 8 bytes for only 5 needed. There is no
reason to keep it as a struct, it doesn't bring any value. By flattening
it, we can merge the single byte with the next single byte, resulting in
an immediate saving of 4 bytes (20%). Interestingly, tests have shown a
steady performance gain of 0.6% after this change, which can possibly be
attributed to a more cache-line friendly struct.
This commit is contained in:
Willy Tarreau 2012-09-02 22:19:18 +02:00
parent 40ff59d820
commit 45dab73788
2 changed files with 22 additions and 24 deletions

View File

@ -55,10 +55,8 @@ enum {
struct fdtab { struct fdtab {
int (*iocb)(int fd); /* I/O handler, returns FD_WAIT_* */ int (*iocb)(int fd); /* I/O handler, returns FD_WAIT_* */
void *owner; /* the connection or listener associated with this fd, NULL if closed */ void *owner; /* the connection or listener associated with this fd, NULL if closed */
struct { /* used by pollers which support speculative polling */ unsigned int spec_p; /* speculative polling: position in spec list+1. 0=not in list. */
unsigned char e; /* read and write events status. 4 bits, may be merged into flags' lower bits */ unsigned char spec_e; /* speculative polling: read and write events status. 4 bits */
unsigned int s1; /* Position in spec list+1. 0=not in list. */
} spec;
unsigned char ev; /* event seen in return of poll() : FD_POLL_* */ unsigned char ev; /* event seen in return of poll() : FD_POLL_* */
}; };

View File

@ -105,7 +105,7 @@
* The FD array has to hold a back reference to the speculative list. This * The FD array has to hold a back reference to the speculative list. This
* reference is only valid if at least one of the directions is marked SPEC. * reference is only valid if at least one of the directions is marked SPEC.
* *
* We store the FD state in the 4 lower bits of fdtab[fd].spec.e, and save the * We store the FD state in the 4 lower bits of fdtab[fd].spec_e, and save the
* previous state upon changes in the 4 higher bits, so that changes are easy * previous state upon changes in the 4 higher bits, so that changes are easy
* to spot. * to spot.
*/ */
@ -160,10 +160,10 @@ static struct epoll_event ev;
REGPRM1 static inline void alloc_spec_entry(const int fd) REGPRM1 static inline void alloc_spec_entry(const int fd)
{ {
if (fdtab[fd].spec.s1) if (fdtab[fd].spec_p)
/* sometimes the entry already exists for the other direction */ /* sometimes the entry already exists for the other direction */
return; return;
fdtab[fd].spec.s1 = nbspec + 1; fdtab[fd].spec_p = nbspec + 1;
spec_list[nbspec] = fd; spec_list[nbspec] = fd;
nbspec++; nbspec++;
} }
@ -176,11 +176,11 @@ REGPRM1 static void release_spec_entry(int fd)
{ {
unsigned int pos; unsigned int pos;
pos = fdtab[fd].spec.s1; pos = fdtab[fd].spec_p;
if (!pos) if (!pos)
return; return;
fdtab[fd].spec.s1 = 0; fdtab[fd].spec_p = 0;
pos--; pos--;
/* we have spec_list[pos]==fd */ /* we have spec_list[pos]==fd */
@ -191,7 +191,7 @@ REGPRM1 static void release_spec_entry(int fd)
/* we replace current FD by the highest one, which may sometimes be the same */ /* we replace current FD by the highest one, which may sometimes be the same */
fd = spec_list[nbspec]; fd = spec_list[nbspec];
spec_list[pos] = fd; spec_list[pos] = fd;
fdtab[fd].spec.s1 = pos + 1; fdtab[fd].spec_p = pos + 1;
} }
/* /*
@ -207,7 +207,7 @@ REGPRM2 static int __fd_is_set(const int fd, int dir)
ABORT_NOW(); ABORT_NOW();
} }
#endif #endif
ret = ((unsigned)fdtab[fd].spec.e >> dir) & FD_EV_MASK_DIR; ret = ((unsigned)fdtab[fd].spec_e >> dir) & FD_EV_MASK_DIR;
return (ret == FD_EV_SPEC || ret == FD_EV_WAIT); return (ret == FD_EV_SPEC || ret == FD_EV_WAIT);
} }
@ -225,14 +225,14 @@ REGPRM2 static void __fd_wai(const int fd, int dir)
ABORT_NOW(); ABORT_NOW();
} }
#endif #endif
i = ((unsigned)fdtab[fd].spec.e >> dir) & FD_EV_MASK_DIR; i = ((unsigned)fdtab[fd].spec_e >> dir) & FD_EV_MASK_DIR;
if (!(i & FD_EV_IN_SL)) { if (!(i & FD_EV_IN_SL)) {
if (i == FD_EV_WAIT) if (i == FD_EV_WAIT)
return; /* already in desired state */ return; /* already in desired state */
alloc_spec_entry(fd); /* need a spec entry */ alloc_spec_entry(fd); /* need a spec entry */
} }
fdtab[fd].spec.e ^= (i ^ (unsigned int)FD_EV_IN_PL) << dir; fdtab[fd].spec_e ^= (i ^ (unsigned int)FD_EV_IN_PL) << dir;
} }
REGPRM2 static void __fd_set(const int fd, int dir) REGPRM2 static void __fd_set(const int fd, int dir)
@ -245,7 +245,7 @@ REGPRM2 static void __fd_set(const int fd, int dir)
ABORT_NOW(); ABORT_NOW();
} }
#endif #endif
i = ((unsigned)fdtab[fd].spec.e >> dir) & FD_EV_MASK_DIR; i = ((unsigned)fdtab[fd].spec_e >> dir) & FD_EV_MASK_DIR;
if (i != FD_EV_STOP) { if (i != FD_EV_STOP) {
if (unlikely(i != FD_EV_IDLE)) if (unlikely(i != FD_EV_IDLE))
@ -253,7 +253,7 @@ REGPRM2 static void __fd_set(const int fd, int dir)
// switch to SPEC state and allocate a SPEC entry. // switch to SPEC state and allocate a SPEC entry.
alloc_spec_entry(fd); alloc_spec_entry(fd);
} }
fdtab[fd].spec.e ^= (unsigned int)(FD_EV_IN_SL << dir); fdtab[fd].spec_e ^= (unsigned int)(FD_EV_IN_SL << dir);
} }
REGPRM2 static void __fd_clr(const int fd, int dir) REGPRM2 static void __fd_clr(const int fd, int dir)
@ -266,7 +266,7 @@ REGPRM2 static void __fd_clr(const int fd, int dir)
ABORT_NOW(); ABORT_NOW();
} }
#endif #endif
i = ((unsigned)fdtab[fd].spec.e >> dir) & FD_EV_MASK_DIR; i = ((unsigned)fdtab[fd].spec_e >> dir) & FD_EV_MASK_DIR;
if (i != FD_EV_SPEC) { if (i != FD_EV_SPEC) {
if (unlikely(i != FD_EV_WAIT)) if (unlikely(i != FD_EV_WAIT))
@ -278,7 +278,7 @@ REGPRM2 static void __fd_clr(const int fd, int dir)
*/ */
alloc_spec_entry(fd); alloc_spec_entry(fd);
} }
fdtab[fd].spec.e ^= (unsigned int)(FD_EV_IN_SL << dir); fdtab[fd].spec_e ^= (unsigned int)(FD_EV_IN_SL << dir);
} }
/* normally unused */ /* normally unused */
@ -295,7 +295,7 @@ REGPRM1 static void __fd_rem(int fd)
REGPRM1 static void __fd_clo(int fd) REGPRM1 static void __fd_clo(int fd)
{ {
release_spec_entry(fd); release_spec_entry(fd);
fdtab[fd].spec.e &= ~(FD_EV_MASK | FD_EV_MASK_OLD); fdtab[fd].spec_e &= ~(FD_EV_MASK | FD_EV_MASK_OLD);
} }
/* /*
@ -314,8 +314,8 @@ REGPRM2 static void _do_poll(struct poller *p, int exp)
while (likely(spec_idx > 0)) { while (likely(spec_idx > 0)) {
spec_idx--; spec_idx--;
fd = spec_list[spec_idx]; fd = spec_list[spec_idx];
en = fdtab[fd].spec.e & 15; /* new events */ en = fdtab[fd].spec_e & 15; /* new events */
eo = fdtab[fd].spec.e >> 4; /* previous events */ eo = fdtab[fd].spec_e >> 4; /* previous events */
/* If an fd with a poll bit is present here, it means that it /* If an fd with a poll bit is present here, it means that it
* has last requested a poll, or is leaving from a poll. Given * has last requested a poll, or is leaving from a poll. Given
@ -372,9 +372,9 @@ REGPRM2 static void _do_poll(struct poller *p, int exp)
epoll_ctl(epoll_fd, opcode, fd, &ev); epoll_ctl(epoll_fd, opcode, fd, &ev);
} }
fdtab[fd].spec.e = (en << 4) + en; /* save new events */ fdtab[fd].spec_e = (en << 4) + en; /* save new events */
if (!(fdtab[fd].spec.e & FD_EV_RW_SL)) { if (!(fdtab[fd].spec_e & FD_EV_RW_SL)) {
/* This fd switched to combinations of either WAIT or /* This fd switched to combinations of either WAIT or
* IDLE. It must be removed from the spec list. * IDLE. It must be removed from the spec list.
*/ */
@ -447,7 +447,7 @@ REGPRM2 static void _do_poll(struct poller *p, int exp)
while (likely(spec_idx > 0)) { while (likely(spec_idx > 0)) {
spec_idx--; spec_idx--;
fd = spec_list[spec_idx]; fd = spec_list[spec_idx];
eo = fdtab[fd].spec.e; /* save old events */ eo = fdtab[fd].spec_e; /* save old events */
/* /*
* Process the speculative events. * Process the speculative events.
@ -473,7 +473,7 @@ REGPRM2 static void _do_poll(struct poller *p, int exp)
if (!fdtab[fd].owner) if (!fdtab[fd].owner)
continue; continue;
if (!(fdtab[fd].spec.e & (FD_EV_RW_SL|FD_EV_RW_PL))) { if (!(fdtab[fd].spec_e & (FD_EV_RW_SL|FD_EV_RW_PL))) {
/* This fd switched to IDLE, it can be removed from the spec list. */ /* This fd switched to IDLE, it can be removed from the spec list. */
release_spec_entry(fd); release_spec_entry(fd);
continue; continue;