From ac65538a48bd4697f10ad9f0ab197898da389aa0 Mon Sep 17 00:00:00 2001 From: matt335672 <30179339+matt335672@users.noreply.github.com> Date: Mon, 22 May 2023 11:07:38 +0100 Subject: [PATCH] Re-implement fifo code Following informal option testing, a more performant fifo implementation has been chosen which makes fewer, larger allocations, but which does not have bad edge-case performance Clearing the contents of a fifo is a common operation which generally involves freeing memory. Support has been added to the fifo interface for doing this. --- common/fifo.c | 334 +++++++++++++++++++++++++++++++------------------- common/fifo.h | 89 +++++++++++--- 2 files changed, 275 insertions(+), 148 deletions(-) diff --git a/common/fifo.c b/common/fifo.c index 7acf85a7..c6a2025e 100644 --- a/common/fifo.c +++ b/common/fifo.c @@ -1,7 +1,7 @@ /** * xrdp: A Remote Desktop Protocol server. * - * Copyright (C) Laxmikant Rashinkar 2004-2014 + * Copyright (C) Matt Burt 2023 * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -14,174 +14,250 @@ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. - * - * FIFO implementation to store pointer to data struct */ +/** + * @file common/fifo.c + * @brief Fifo for storing generic pointers + * + * Defines an unbounded FIFO-queue for void * pointers + * + * The stored pointers are called 'items' below. + * + * Items are stored in groups called 'chunks'. Chunks are linked together + * in a chain:- + * + * +-------------+ +--------+ +--------+ +--------+ + * | first_chunk |--->| next |--->| next |--->| NULL |<-+ + * | last_chunk |-+ +--------+ +--------+ +--------+ | + * | . . . | | | item.0 | | item.0 | | item.0 | | + * +-------------+ | | ... | | ... | | ... | | + * | | item.n | | item.n | | item.n | | + * | +--------+ +--------+ +--------+ | + * | | + * +------------------------------------------+ + * + * This allows items to be added to the FIFO by allocating blocks + * as each one fills up. + * + * The code to read from the FIFO de-allocates blocks as each one is + * consumed. + * + * There is always at least one chunk in the FIFO. + */ + + #if defined(HAVE_CONFIG_H) #include #endif +#include + #include "fifo.h" -#include "os_calls.h" -/** - * Create new fifo data struct - * - * @return pointer to new FIFO or NULL if system out of memory - *****************************************************************************/ +#define ITEMS_PER_CHUNK 31 -FIFO * -fifo_create(void) +struct chunk { - return (FIFO *) g_malloc(sizeof(FIFO), 1); + struct chunk *next; + void *items[ITEMS_PER_CHUNK]; +}; + +struct fifo +{ + struct chunk *first_chunk; + struct chunk *last_chunk; + /** Next address to write in 'last_chunk' */ + unsigned short writer; + /** Next address to read in 'first_chunk' */ + unsigned short reader; + /** Item destructor function, or NULL */ + fifo_item_destructor item_destructor; +}; + +/*****************************************************************************/ + +struct fifo * +fifo_create(fifo_item_destructor item_destructor) +{ + struct fifo *result = NULL; + struct chunk *cptr = (struct chunk *)malloc(sizeof(struct chunk)); + if (cptr != NULL) + { + /* 'next' pointer in last block is always NULL */ + cptr->next = NULL; + result = (struct fifo *)malloc(sizeof(struct fifo)); + if (result == NULL) + { + free(cptr); + } + else + { + result->first_chunk = cptr; + result->last_chunk = cptr; + result->writer = 0; + result->reader = 0; + result->item_destructor = item_destructor; + } + } + return result; } +/*****************************************************************************/ /** - * Delete specified FIFO - *****************************************************************************/ + * Internal function to call the destructor function on all items in the fifo + * + * @param self fifo. Can't be NULL + * @param closure Additional argument to destructor function + */ +static void +call_item_destructor(struct fifo *self, void *closure) +{ + if (self->item_destructor != NULL) + { + struct chunk *cptr = self->first_chunk; + unsigned int i = self->reader; + // Process all the chunks up to the last one + while (cptr != self->last_chunk) + { + (*self->item_destructor)(cptr->items[i++], closure); + if (i == ITEMS_PER_CHUNK) + { + cptr = cptr->next; + i = 0; + } + } + + // Process all the items in the last chunk + while (i < self->writer) + { + (*self->item_destructor)(cptr->items[i++], closure); + } + } +} + + +/*****************************************************************************/ void -fifo_delete(FIFO *self) +fifo_delete(struct fifo *self, void *closure) { - USER_DATA *udp; - - if (!self) + if (self != NULL) { - return; - } + call_item_destructor(self, closure); - if (!self->head) - { - /* FIFO is empty */ - g_free(self); - return; - } - - if (self->head == self->tail) - { - /* only one item in FIFO */ - if (self->auto_free) + // Now free all the chunks + struct chunk *cptr = self->first_chunk; + while (cptr != NULL) { - g_free(self->head->item); + struct chunk *next = cptr->next; + free(cptr); + cptr = next; } - g_free(self->head); - g_free(self); - return; + free(self); } - - /* more then one item in FIFO */ - while (self->head) - { - udp = self->head; - - if (self->auto_free) - { - g_free(udp->item); - } - - self->head = udp->next; - g_free(udp); - } - - g_free(self); } -/** - * Add an item to the specified FIFO - * - * @param self FIFO to operate on - * @param item item to add to specified FIFO - * - * @return 0 on success, -1 on error - *****************************************************************************/ +/*****************************************************************************/ +void +fifo_clear(struct fifo *self, void *closure) +{ + if (self != NULL) + { + call_item_destructor(self, closure); + + // Now free all the chunks except the last one + struct chunk *cptr = self->first_chunk; + while (cptr->next != NULL) + { + struct chunk *next = cptr->next; + free(cptr); + cptr = next; + } + + // Re-initialise fifo fields + self->first_chunk = cptr; + self->last_chunk = cptr; + self->reader = 0; + self->writer = 0; + } +} + +/*****************************************************************************/ int -fifo_add_item(FIFO *self, void *item) +fifo_add_item(struct fifo *self, void *item) { - USER_DATA *udp; - - if (!self || !item) + int rv = 0; + if (self != NULL && item != NULL) { - return -1; + if (self->writer == ITEMS_PER_CHUNK) + { + // Add another chunk to the chain + struct chunk *cptr; + cptr = (struct chunk *)malloc(sizeof(struct chunk)); + if (cptr == NULL) + { + return 0; + } + cptr->next = NULL; + self->last_chunk->next = cptr; + self->last_chunk = cptr; + self->writer = 0; + } + + self->last_chunk->items[self->writer++] = item; + rv = 1; } - - if ((udp = (USER_DATA *) g_malloc(sizeof(USER_DATA), 0)) == 0) - { - return -1; - } - - udp->item = item; - udp->next = 0; - - /* if fifo is empty, add to head */ - if (!self->head) - { - self->head = udp; - self->tail = udp; - return 0; - } - - /* add to tail */ - self->tail->next = udp; - self->tail = udp; - - return 0; + return rv; } -/** - * Return an item from top of FIFO - * - * @param self FIFO to operate on - * - * @return top item from FIFO or NULL if FIFO is empty - *****************************************************************************/ - +/*****************************************************************************/ void * -fifo_remove_item(FIFO *self) +fifo_remove_item(struct fifo *self) { - void *item; - USER_DATA *udp; - - if (!self || !self->head) + void *item = NULL; + if (self != NULL) { - return 0; - } + // More than one chunk in the fifo? + if (self->first_chunk != self->last_chunk) + { + /* We're not reading the last chunk. There + * must be something in the fifo */ + item = self->first_chunk->items[self->reader++]; - if (self->head == self->tail) - { - /* only one item in FIFO */ - item = self->head->item; - g_free(self->head); - self->head = 0; - self->tail = 0; - return item; + /* At the end of this chunk? */ + if (self->reader == ITEMS_PER_CHUNK) + { + struct chunk *old_chunk = self->first_chunk; + self->first_chunk = old_chunk->next; + free(old_chunk); + self->reader = 0; + } + } + else if (self->reader < self->writer) + { + /* We're reading the last chunk */ + item = self->first_chunk->items[self->reader++]; + if (self->reader == self->writer) + { + // fifo is now empty. We can reset the pointers + // to prevent unnecessary allocations in the future. + self->reader = 0; + self->writer = 0; + } + } } - - /* more then one item in FIFO */ - udp = self->head; - item = self->head->item; - self->head = self->head->next; - g_free(udp); return item; } -/** - * Return FIFO status - * - * @param self FIFO to operate on - * - * @return true if FIFO is empty, false otherwise - *****************************************************************************/ +/*****************************************************************************/ int -fifo_is_empty(FIFO *self) +fifo_is_empty(struct fifo *self) { - if (!self) - { - return 1; - } - - return (self->head == 0); + return (self == NULL || + (self->first_chunk == self->last_chunk && + self->reader == self->writer)); } diff --git a/common/fifo.h b/common/fifo.h index ace5c6b8..8dc8a628 100644 --- a/common/fifo.h +++ b/common/fifo.h @@ -14,34 +14,85 @@ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. + */ + +/** + * @file common/fifo.h + * @brief Fifo for storing generic pointers * - * FIFO implementation to store pointer to data struct + * Declares an unbounded FIFO-queue for void * pointers */ #ifndef _FIFO_H #define _FIFO_H -#include "arch.h" +struct fifo; -typedef struct user_data USER_DATA; +/** + * Function used by fifo_clear()/fifo_delete() to destroy items + * + * @param item Item being deleted + * @param closure Additional argument to function + * + * Use this function to free any allocated storage (e.g. if the items + * are dynamically allocated) + */ +typedef void (*fifo_item_destructor)(void *item, void *closure); -struct user_data -{ - USER_DATA *next; - void *item; -}; +/** + * Create new fifo + * + * @param item_destructor Destructor for fifo items, or NULL for none + * @return fifo, or NULL if no memory + */ +struct fifo * +fifo_create(fifo_item_destructor item_destructor); -typedef struct fifo -{ - USER_DATA *head; - USER_DATA *tail; - int auto_free; -} FIFO; +/** + * Delete an existing fifo + * + * Any existing entries on the fifo are passed in order to the + * item destructor specified when the fifo was created. + * + * @param self fifo to delete (may be NULL) + * @param closure Additional parameter for fifo item destructor + */ +void +fifo_delete(struct fifo *self, void *closure); -FIFO *fifo_create(void); -void fifo_delete(FIFO *self); -int fifo_add_item(FIFO *self, void *item); -void *fifo_remove_item(FIFO *self); -int fifo_is_empty(FIFO *self); +/** + * Clear(empty) an existing fifo + * + * Any existing entries on the fifo are passed in order to the + * item destructor specified when the fifo was created. + * + * @param self fifo to clear (may be NULL) + * @param closure Additional parameter for fifo item destructor + */ +void +fifo_clear(struct fifo *self, void *closure); + +/** Add an item to a fifo + * @param self fifo + * @param item Item to add + * @return 1 if successful, 0 for no memory, or tried to add NULL + */ +int +fifo_add_item(struct fifo *self, void *item); + +/** Remove an item from a fifo + * @param self fifo + * @return item if successful, NULL for no items in FIFO + */ +void * +fifo_remove_item(struct fifo *self); + +/** Is fifo empty? + * + * @param self fifo + * @return 1 if fifo is empty, 0 if not + */ +int +fifo_is_empty(struct fifo *self); #endif