From 75b3691740dc1fc515973e84664f4299aa29e0c0 Mon Sep 17 00:00:00 2001 From: Ivan Kozlovic Date: Thu, 3 Jul 2025 19:36:38 -0600 Subject: [PATCH] [IMPROVED] Timer insertion We use a linked list to hold timers. They are ordered by absolute time. Inserting a timer was always looking for the spot starting at the beginning. We now check first if it should be added directly at the end of the list, which in most cases - say creating new connections that creates a timer for sending PINGs, they will likely be sequential. Added a test that shows that for 100,000 timers, the time spent is now very small. Signed-off-by: Ivan Kozlovic --- src/glib/glib_timer.c | 54 +++++++++++++++++----- src/glib/glibp.h | 5 +- test/test.c | 104 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 150 insertions(+), 13 deletions(-) diff --git a/src/glib/glib_timer.c b/src/glib/glib_timer.c index ba99480a6..744cb386b 100644 --- a/src/glib/glib_timer.c +++ b/src/glib/glib_timer.c @@ -1,4 +1,4 @@ -// Copyright 2015-2024 The NATS Authors +// Copyright 2015-2025 The NATS Authors // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at @@ -25,9 +25,32 @@ void nats_freeTimers(natsLib *lib) static void _insertTimer(natsTimer *t) { - natsTimer *cur = nats_lib()->timers.timers; - natsTimer *prev = NULL; + natsLibTimers *timers = &(nats_lib()->timers); + natsTimer *tail = timers->tail; + natsTimer *cur = NULL; + natsTimer *prev = NULL; + // Can we insert as the last in the list? + if ((tail != NULL) && (tail->absoluteTime <= t->absoluteTime)) + { + t->prev = tail; + t->next = NULL; + // If there was a previous tail, then link to `t`. + if (tail != NULL) + tail->next = t; + else + { + // There was no tail, so no head either. + timers->head = t; + } + // This is the new tail. + timers->tail = t; + // We are done + return; + } + + // Find the spot. Start at the beginning. + cur = timers->head; while ((cur != NULL) && (cur->absoluteTime <= t->absoluteTime)) { prev = cur; @@ -51,7 +74,9 @@ _insertTimer(natsTimer *t) } if (prev == NULL) - nats_lib()->timers.timers = t; + timers->head = t; + if (t->next == NULL) + timers->tail = t; } // Locks must be held before entering this function @@ -69,9 +94,14 @@ _removeTimer(natsLib *lib, natsTimer *t) t->prev->next = t->next; if (t->next != NULL) t->next->prev = t->prev; - - if (t == lib->timers.timers) - lib->timers.timers = t->next; + else + { + // The timer had no "next", so it was the tail. Move back the tail. + lib->timers.tail = t->prev; + } + // If we removed the first, move the head. + if (t == lib->timers.head) + lib->timers.head = t->next; t->prev = NULL; t->next = NULL; @@ -183,7 +213,7 @@ nats_getTimersCountInList(void) natsMutex_Lock(lib->timers.lock); - t = lib->timers.timers; + t = lib->timers.head; while (t != NULL) { count++; @@ -211,7 +241,7 @@ void nats_timerThreadf(void *arg) while (!(timers->shutdown)) { // Take the first timer that needs to fire. - t = timers->timers; + t = timers->head; if (t == NULL) { @@ -244,9 +274,11 @@ void nats_timerThreadf(void *arg) natsMutex_Lock(t->mu); // Remove timer from the list: - timers->timers = t->next; + timers->head = t->next; if (t->next != NULL) t->next->prev = NULL; + else + timers->tail = NULL; t->prev = NULL; t->next = NULL; @@ -298,7 +330,7 @@ void nats_timerThreadf(void *arg) // Process the timers that were left in the list (not stopped) when the // library is shutdown. - while ((t = timers->timers) != NULL) + while ((t = timers->head) != NULL) { natsMutex_Lock(t->mu); diff --git a/src/glib/glibp.h b/src/glib/glibp.h index fa658f139..debb1ea9d 100644 --- a/src/glib/glibp.h +++ b/src/glib/glibp.h @@ -1,4 +1,4 @@ -// Copyright 2015-2024 The NATS Authors +// Copyright 2015-2025 The NATS Authors // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. // You may obtain a copy of the License at @@ -46,7 +46,8 @@ typedef struct __natsLibTimers natsMutex *lock; natsCondition *cond; natsThread *thread; - natsTimer *timers; + natsTimer *head; + natsTimer *tail; int count; bool changed; bool shutdown; diff --git a/test/test.c b/test/test.c index cb445760c..1cf9e5700 100644 --- a/test/test.c +++ b/test/test.c @@ -1237,9 +1237,20 @@ void test_natsTimer(void) natsTimer *t = NULL; struct threadArg tArg; int refs; + int numTimers = 100000; + natsTimer **timers; + int i; + int64_t start = 0; + int64_t dur = 0; test("Setup test: "); s = _createDefaultThreadArgsForCbTests(&tArg); + if (s == NATS_OK) + { + timers = (natsTimer**) calloc(numTimers, sizeof(natsTimer*)); + if (timers == NULL) + s = NATS_ERR; + } testCond(s == NATS_OK); tArg.control = 0; @@ -1444,6 +1455,99 @@ void test_natsTimer(void) _destroyDefaultThreadArgs(&tArg); + // Test insert code and make sure timers are added in the proper order. + test("Add as first: "); + s = natsTimer_Create(&(timers[0]), _dummyTimerCB, NULL, 10000, NULL); + testCond(s == NATS_OK); + + test("Add to the end: "); + s = natsTimer_Create(&(timers[1]), _dummyTimerCB, NULL, 20000, NULL); + testCond(s == NATS_OK); + + test("Add to the end again: "); + s = natsTimer_Create(&(timers[2]), _dummyTimerCB, NULL, 30000, NULL); + testCond(s == NATS_OK); + + test("Add as first again: "); + s = natsTimer_Create(&(timers[3]), _dummyTimerCB, NULL, 1000, NULL); + testCond(s == NATS_OK); + + test("Insert in between: "); + s = natsTimer_Create(&(timers[4]), _dummyTimerCB, NULL, 15000, NULL); + testCond(s == NATS_OK); + + test("Verify count: "); + testCond(nats_getTimersCountInList() == 5); + + test("Verify order: "); + { + natsLibTimers *timers = &(nats_lib()->timers); + natsTimer *cur = NULL; + int idx = 0; + + s = NATS_OK; + natsMutex_Lock(timers->lock); + cur = timers->head; + while ((s == NATS_OK) && (cur != NULL)) + { + switch (idx) + { + case 0: + s = (cur->interval == 1000 ? NATS_OK : NATS_ERR); + break; + case 1: + s = (cur->interval == 10000 ? NATS_OK : NATS_ERR); + break; + case 2: + s = (cur->interval == 15000 ? NATS_OK : NATS_ERR); + break; + case 3: + s = (cur->interval == 20000 ? NATS_OK : NATS_ERR); + break; + case 4: + s = (cur->interval == 30000 ? NATS_OK : NATS_ERR); + break; + default: + s = NATS_ERR; + } + cur = cur->next; + idx++; + } + natsMutex_Unlock(timers->lock); + } + testCond(s == NATS_OK); + + for (i=0; i<5; i++) + { + natsTimer_Destroy(timers[i]); + timers[i] = NULL; + } + + test("Create performance: "); + s = NATS_OK; + start = nats_NowInNanoSeconds(); + for (i=0; (s == NATS_OK) && (i