From 4c3e900017b72d0e588012cef40084df8cc68870 Mon Sep 17 00:00:00 2001 From: Richard Fuchs Date: Thu, 20 Feb 2025 11:45:42 -0400 Subject: [PATCH] MT#62181 wheeltimer: move out expiry values Don't store the relative expiry duration in the timer object. Only store the absolute value. Pass the relative expiry for a new timer as a method argument when inserting the timer into the tree instead. No functional/behavioural changes. Change-Id: I24817b0ac5801a5addf05bf9155fcb31017f2f68 --- core/AmAppTimer.cpp | 21 ++++++++++----------- core/AmAppTimer.h | 2 +- core/sip/sip_trans.cpp | 10 +++++----- core/sip/sip_trans.h | 9 +++++---- core/sip/tr_blacklist.cpp | 4 ++-- core/sip/tr_blacklist.h | 4 ++-- core/sip/wheeltimer.cpp | 21 +++++++++++++-------- core/sip/wheeltimer.h | 24 +++++++++--------------- 8 files changed, 47 insertions(+), 48 deletions(-) diff --git a/core/AmAppTimer.cpp b/core/AmAppTimer.cpp index 8066947a..24bc8acc 100644 --- a/core/AmAppTimer.cpp +++ b/core/AmAppTimer.cpp @@ -38,8 +38,8 @@ class app_timer : public timer int timer_id; public: - app_timer(const string& q_id, int timer_id, uint64_t expires) - : timer(expires), timer_id(timer_id), q_id(q_id) {} + app_timer(const string& q_id, int timer_id) + : timer_id(timer_id), q_id(q_id) {} ~app_timer() {} @@ -58,8 +58,8 @@ class direct_app_timer public: DirectAppTimer* dt; - direct_app_timer(DirectAppTimer* dt, uint64_t expires) - : timer(expires), dt(dt) {} + direct_app_timer(DirectAppTimer* dt) + : dt(dt) {} ~direct_app_timer() {} @@ -148,10 +148,9 @@ app_timer* _AmAppTimer::erase_timer(const string& q_id, int id) return res; } -app_timer* _AmAppTimer::create_timer(const string& q_id, int id, - uint64_t expires) +app_timer* _AmAppTimer::create_timer(const string& q_id, int id) { - app_timer* timer = new app_timer(q_id, id, expires); + app_timer* timer = new app_timer(q_id, id); if (!timer) return NULL; @@ -181,9 +180,9 @@ void _AmAppTimer::setTimer(const string& eventqueue_name, int timer_id, double t if (NULL != t) { remove_timer(t); } - t = create_timer(eventqueue_name, timer_id, expires); + t = create_timer(eventqueue_name, timer_id); if (NULL != t) { - insert_timer(t); + insert_timer(t, expires); } user_timers_mut.unlock(); } @@ -217,7 +216,7 @@ void _AmAppTimer::setTimer_unsafe(DirectAppTimer* t, double timeout) { uint64_t expires = timeout * 1000000.; // microseconds - direct_app_timer* dt = new direct_app_timer(t,expires); + direct_app_timer* dt = new direct_app_timer(t); if(!dt) return; DirectTimers::iterator dt_it = direct_timers.find(t); @@ -228,7 +227,7 @@ void _AmAppTimer::setTimer_unsafe(DirectAppTimer* t, double timeout) else { direct_timers[t] = dt; } - insert_timer(dt); + insert_timer(dt, expires); } void _AmAppTimer::setTimer(DirectAppTimer* t, double timeout) diff --git a/core/AmAppTimer.h b/core/AmAppTimer.h index 212057d3..0c2ac585 100644 --- a/core/AmAppTimer.h +++ b/core/AmAppTimer.h @@ -59,7 +59,7 @@ class _AmAppTimer DirectTimers direct_timers; /** creates timer object and inserts it into our container */ - app_timer* create_timer(const string& q_id, int id, uint64_t expires); + app_timer* create_timer(const string& q_id, int id); /** erases timer - does not delete timer object @return timer object pointer, if found */ app_timer* erase_timer(const string& q_id, int id); diff --git a/core/sip/sip_trans.cpp b/core/sip/sip_trans.cpp index 529e0d86..117dd087 100644 --- a/core/sip/sip_trans.cpp +++ b/core/sip/sip_trans.cpp @@ -180,7 +180,7 @@ const char* _state_name_lookup[] = { * @param t the new timer * @param timer_type @see sip_timer_type */ -void sip_trans::reset_timer(trans_timer* t, unsigned int timer_type) +void sip_trans::reset_timer(trans_timer* t, unsigned int timer_type, uint64_t expires_us) { trans_timer** tp = fetch_timer(timer_type,timers); @@ -194,7 +194,7 @@ void sip_trans::reset_timer(trans_timer* t, unsigned int timer_type) *tp = t; if(t) - wheeltimer::instance()->insert_timer((timer*)t); + wheeltimer::instance()->insert_timer((timer*)t, expires_us); } void trans_timer::fire() @@ -204,7 +204,7 @@ void trans_timer::fire() bucket->lock(); if(bucket->exist(t)){ DBG("Transaction timer expired: type=%s, trans=%p, eta=%" PRIu64 ", t=%" PRIu64 "\n", - timer_name(type),t,expires,wheeltimer::instance()->get_wall_clock()); + timer_name(type),t,this->get_absolute_expiry(),wheeltimer::instance()->get_wall_clock()); trans_timer* tt = t->get_timer(this->type & 0xFFFF); if(tt != this) { @@ -246,10 +246,10 @@ void sip_trans::reset_timer(unsigned int timer_type, uint64_t expire_delay /* ms DBG("New timer of type %s at time=%" PRIu64 " (repeated=%i)\n", timer_name(timer_type),expires,timer_type>>16); - trans_timer* t = new trans_timer(timer_type,expires, + trans_timer* t = new trans_timer(timer_type, bucket_id,this); - reset_timer(t,timer_type); + reset_timer(t,timer_type,expires); } void sip_trans::clear_timer(unsigned int timer_type) diff --git a/core/sip/sip_trans.h b/core/sip/sip_trans.h index 7e882038..ba9a9d4e 100644 --- a/core/sip/sip_trans.h +++ b/core/sip/sip_trans.h @@ -90,9 +90,9 @@ public: unsigned int bucket_id; sip_trans* t; - trans_timer(unsigned int timer_type, uint64_t expires, + trans_timer(unsigned int timer_type, int bucket_id, sip_trans* t) - : timer(expires), type(timer_type), + : type(timer_type), bucket_id(bucket_id), t(t) {} @@ -187,10 +187,11 @@ class sip_trans /** * Resets a specific timer * - * @param t the new timer + * @param t the new timer, can be NULL * @param timer_type @see sip_timer_type + * @param expires_us relative expiry in microseconds, if not already set in t */ - void reset_timer(trans_timer* t, unsigned int timer_type); + void reset_timer(trans_timer* t, unsigned int timer_type, uint64_t expires_us = 0); /** * Clears a specfic timer diff --git a/core/sip/tr_blacklist.cpp b/core/sip/tr_blacklist.cpp index 2f824bfb..e79223bc 100644 --- a/core/sip/tr_blacklist.cpp +++ b/core/sip/tr_blacklist.cpp @@ -51,7 +51,7 @@ bool blacklist_bucket::insert(const bl_addr& addr, uint64_t duration /* ms */, wheeltimer* wt = wheeltimer::instance(); uint64_t expires = duration * 1000; - bl_timer* t = new bl_timer(addr,expires); + bl_timer* t = new bl_timer(addr); bl_entry* bl_e = new bl_entry(t); if(!bl_bucket_base::insert(addr,bl_e)) { @@ -63,7 +63,7 @@ bool blacklist_bucket::insert(const bl_addr& addr, uint64_t duration /* ms */, am_inet_ntop(&addr).c_str(),am_get_port(&addr), reason,(float)duration/1000.0); - wt->insert_timer(t); + wt->insert_timer(t, expires); return true; } diff --git a/core/sip/tr_blacklist.h b/core/sip/tr_blacklist.h index 28816171..d30f09a3 100644 --- a/core/sip/tr_blacklist.h +++ b/core/sip/tr_blacklist.h @@ -59,8 +59,8 @@ struct bl_timer : timer(), addr() {} - bl_timer(const bl_addr& addr, uint64_t expires) - : timer(expires), addr(addr) + bl_timer(const bl_addr& addr) + : addr(addr) {} void fire(); diff --git a/core/sip/wheeltimer.cpp b/core/sip/wheeltimer.cpp index cafaf1d7..a67a24d6 100644 --- a/core/sip/wheeltimer.cpp +++ b/core/sip/wheeltimer.cpp @@ -45,10 +45,10 @@ timer::~timer() // DBG("timer::~timer(this=%p)\n",this); } -void _wheeltimer::insert_timer(timer* t) +void _wheeltimer::insert_timer(timer* t, uint64_t relative_us) { std::lock_guard lock(buckets_mut); - place_timer(t); + place_timer(t, relative_us); // Wake up worker thread: The new timer might be the next one to run buckets_cond.notify_one(); } @@ -124,24 +124,29 @@ void _wheeltimer::process_current_timers(timer_list& list, std::unique_lockget_absolute_expiry(); - - // scale expiry based on resolution: this is the bucket index - uint64_t bucket = ((exp / resolution) + 1) * resolution; + uint64_t bucket = get_timer_bucket(exp); // if expiry is too soon or in the past, put the timer in the next bucket up auto now = gettimeofday_us(); if (bucket <= now) - bucket = ((now / resolution) + 1) * resolution; + bucket = get_timer_bucket(now); return bucket; } -void _wheeltimer::place_timer(timer* t) +void _wheeltimer::place_timer(timer* t, uint64_t relative_us) { - t->arm(); + t->arm(relative_us); uint64_t bucket = get_timer_bucket(t); add_timer_to_bucket(t, bucket); } diff --git a/core/sip/wheeltimer.h b/core/sip/wheeltimer.h index f28b9373..089695ef 100644 --- a/core/sip/wheeltimer.h +++ b/core/sip/wheeltimer.h @@ -54,26 +54,22 @@ class timer public: timer() - : list(NULL), expires(0), expires_rel(0) - {} - - timer(uint64_t expires) - : list(NULL), expires(0), expires_rel(expires) + : list(NULL), expires(0) {} timer(const timer &t) - : list(NULL), expires(t.expires), expires_rel(t.expires_rel) + : list(NULL), expires(t.expires) {} virtual ~timer(); virtual void fire()=0; - void arm() + void arm(uint64_t relative) { if (expires) return; - expires = expires_rel + gettimeofday_us(); + expires = relative + gettimeofday_us(); } void link(timer_list& new_list) @@ -99,11 +95,8 @@ public: return expires; } -protected: - uint64_t expires; // absolute, microseconds, set after arming timer - private: - uint64_t expires_rel; // relative, microseconds + uint64_t expires; // absolute, microseconds, set after arming timer }; #include "singleton.h" @@ -126,10 +119,11 @@ class _wheeltimer: // future (or if no timers exist). Needed not to miss the shutdown flag being set. const uint64_t max_sleep_time = 500000; // half a second - void place_timer(timer* t); + void place_timer(timer* t, uint64_t); void add_timer_to_bucket(timer* t, uint64_t); - uint64_t get_timer_bucket(timer* t); + uint64_t get_timer_bucket(uint64_t); + uint64_t get_timer_bucket(timer*); void delete_timer(timer* t); void process_current_timers(timer_list&, std::unique_lock&); @@ -162,7 +156,7 @@ public: return gettimeofday_us(); } - void insert_timer(timer* t); + void insert_timer(timer* t, uint64_t relative_expiry_us); void remove_timer(timer* t); };