On 10/01/2013 06:08 PM, Amos Jeffries wrote:
> On 2/10/2013 12:59 p.m., Alex Rousskov wrote:
>> On 09/12/2013 10:48 AM, Alex Rousskov wrote:
>>> The only potentially bad side effect of this change I can think of is
>>> that some events will be scheduled and fired a few microseconds before
>>> they are due (instead of a few microseconds after they are due). I hope
>>> that would not break any existing Squid code. If it does, we can adjust
>>> EventSchedule to round up and, hence, fire all events at or after their
>>> deadline.
>> I am still concerned about this, even though I do not know of any
>> specific cases where firing events a little earlier would break things.
>> It just feels wrong, and I know that libevent or libev specifically
>> avoids firing events early. Should we change this before the patch is
>> committed? Or just keep the code simple[r] until we know a change is
>> needed?
>>
>> To change this, we would need to introduce a minimum I/O wait time (X),
>> essentially: If some events will become ready to fire in less than X
>> milliseconds (but are not yet ready now), Squid would still wait for I/O
>> at most X milliseconds. I suggest setting X to 0.1 millisecond. Any
>> better ideas?
> Nope. Sounds good.
>
> There is no guarantee for any event that it will be run within the same
> millisecond (or even same whole second) than which it was scheduled due
> to already queued events or Calls taking up time. So adding an explicit
> delay will not hurt (much).
The attached revised patch honors the "no event fired before it is due"
guarantee.
Thank you,
Alex.
P.S. Despite all these changes, idle Squid still runs hotter than it
should because the maximum waiting time is artificially capped outside
the event queue to one second (EVENT_LOOP_TIMEOUT). This causes at most
one extra loop iteration per second (because the external cap overwrites
the careful math done inside the new code). This remaining problem is
outside this project scope. IIRC, the artificial 1s cap is due to some
delay pools code (that should be using regular timed events instead of
this hard-coded 1s hack, I guess).
Avoid "hot idle": A series of rapid select() calls with zero timeout.
Squid uses "infinite" precision when it comes to deciding whether the next
timed event is ready but uses millisecond (1e-3) precision when deciding how
long to wait before the next event will be ready. This inconsistency results
in the EventSchedule engine telling the main loop that it has 0 milliseconds
to poll pending I/O, but when asked again (after the I/O is quickly polled),
the EventScheduler engine often does not schedule the promised event and tells
the main loop to poll for another 0 milliseconds again. This cycling may
happen many times in a row (until enough time is wasted for the next event to
become ready using higher precision).
The fixed code adds a minimum 1ms delay for not-yet-ready events. It also
places both decisions into one method (EventScheduler::timeRemaining), and
tries to polish/document decision logic (which is more complex than it may
seem) because the code has to avoid both inconsistent decisions and hot idle
loops while maintaining the traditional "no event is fired before it is due"
guarantee.
TODO: Idle Squid still runs hotter than it should because the maximum waiting
time is artificially capped outside the event queue to 1000ms. This causes
at most one extra loop iteration per second.
=== modified file 'src/event.cc'
--- src/event.cc 2013-10-25 00:13:46 +0000
+++ src/event.cc 2013-10-30 23:23:26 +0000
@@ -22,40 +22,44 @@
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program; if not, write to the Free Software
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111, USA.
*
*/
#include "squid.h"
#include "compat/drand48.h"
#include "event.h"
#include "mgr/Registration.h"
#include "profiler/Profiler.h"
#include "SquidTime.h"
#include "Store.h"
#include "tools.h"
+#if HAVE_MATH_H
+#include <math.h>
+#endif
+
/* The list of event processes */
static OBJH eventDump;
static const char *last_event_ran = NULL;
// This AsyncCall dialer can be configured to check that the event cbdata is
// valid before calling the event handler
class EventDialer: public CallDialer
{
public:
typedef CallDialer Parent;
EventDialer(EVH *aHandler, void *anArg, bool lockedArg);
EventDialer(const EventDialer &d);
virtual ~EventDialer();
virtual void print(std::ostream &os) const;
virtual bool canDial(AsyncCall &call);
void dial(AsyncCall &) { theHandler(theArg); }
@@ -202,94 +206,94 @@ EventScheduler::cancel(EVH * func, void
if (arg)
return;
/*
* DPW 2007-04-12
* Since this method may now delete multiple events (when
* arg is NULL) it no longer returns after a deletion and
* we have a potential NULL pointer problem. If we just
* deleted the last event in the list then *E is now equal
* to NULL. We need to break here or else we'll get a NULL
* pointer dereference in the last clause of the for loop.
*/
if (NULL == *E)
break;
}
if (arg)
debug_trap("eventDelete: event not found");
}
+// The event API does not guarantee exact timing, but guarantees that no event
+// is fired before it is due. We may delay firing, but never fire too early.
int
-EventScheduler::checkDelay()
+EventScheduler::timeRemaining() const
{
if (!tasks)
return EVENT_IDLE;
- int result = (int) ((tasks->when - current_dtime) * 1000);
-
- if (result < 0)
- return 0;
+ if (tasks->when <= current_dtime) // we are on time or late
+ return 0; // fire the event ASAP
- return result;
+ const double diff = tasks->when - current_dtime; // microseconds
+ // Round UP: If we come back a nanosecond earlier, we will wait again!
+ const int timeLeft = static_cast<int>(ceil(1000*diff)); // milliseconds
+ // Avoid hot idle: A series of rapid select() calls with zero timeout.
+ const int minDelay = 1; // millisecond
+ return max(minDelay, timeLeft);
}
int
EventScheduler::checkEvents(int timeout)
{
-
- ev_entry *event = NULL;
-
- if (NULL == tasks)
- return checkDelay();
-
- if (tasks->when > current_dtime)
- return checkDelay();
+ int result = timeRemaining();
+ if (result != 0)
+ return result;
PROF_start(eventRun);
- debugs(41, 5, HERE << "checkEvents");
-
- while ((event = tasks)) {
- if (event->when > current_dtime)
- break;
+ do {
+ ev_entry *event = tasks;
+ assert(event);
/* XXX assumes event->name is static memory! */
AsyncCall::Pointer call = asyncCall(41,5, event->name,
EventDialer(event->func, event->arg, event->cbdata));
ScheduleCallHere(call);
last_event_ran = event->name; // XXX: move this to AsyncCallQueue
const bool heavy = event->weight &&
(!event->cbdata || cbdataReferenceValid(event->arg));
tasks = event->next;
delete event;
+ result = timeRemaining();
+
// XXX: We may be called again during the same event loop iteration.
// Is there a point in breaking now?
if (heavy)
break; // do not dequeue events following a heavy event
- }
+ } while (result == 0);
PROF_stop(eventRun);
- return checkDelay();
+ return result;
}
void
EventScheduler::clean()
{
while (ev_entry * event = tasks) {
tasks = event->next;
delete event;
}
tasks = NULL;
}
void
EventScheduler::dump(StoreEntry * sentry)
{
ev_entry *e = tasks;
if (last_event_ran)
=== modified file 'src/event.h'
--- src/event.h 2013-10-25 00:13:46 +0000
+++ src/event.h 2013-10-30 22:21:53 +0000
@@ -63,37 +63,37 @@ public:
int weight;
bool cbdata;
ev_entry *next;
};
MEMPROXY_CLASS_INLINE(ev_entry);
// manages time-based events
class EventScheduler : public AsyncEngine
{
public:
EventScheduler();
~EventScheduler();
/* cancel a scheduled but not dispatched event */
void cancel(EVH * func, void * arg);
/* clean up the used memory in the scheduler */
void clean();
- /* how long until the next event ? */
- int checkDelay();
+ /* either EVENT_IDLE or milliseconds remaining until the next event */
+ int timeRemaining() const;
/* cache manager output for the event queue */
void dump(StoreEntry *);
/* find a scheduled event */
bool find(EVH * func, void * arg);
/* schedule a callback function to run in when seconds */
void schedule(const char *name, EVH * func, void *arg, double when, int weight, bool cbdata=true);
int checkEvents(int timeout);
static EventScheduler *GetInstance();
private:
static EventScheduler _instance;
ev_entry * tasks;
};
#endif /* SQUID_EVENT_H */
