Use of imp.wakeup in Promise class


#1

I’ve been experimenting heavily with Promises as they allow clean code separation between how to launch an asynchronous request and how to deal with the responses. Nothing that cannot be done with associating callbacks etc., but I like the fact that all my async methods are now using the same uniform interface irrespective of the underlying async actions).

However, when using the also very powerful Promise.all, Promise.race and Promise.loop aspects, I bumped into the dreaded “More than 20 timers” error in the agent. This after spending a considerable amount of time on building a timer factory framework for our own code to avoid hitting this limit. Quite frustrating to see it re-appear due to use of libraries…

When diving into the Promise code, i saw that it is regularly using the imp.wakeup(0, function(){...}); construction. Apparently, launching many promises simultaneously (as I guess happens with the .all method) hits the agent timer limit quite fast through this use.

2 questions :

  • what is the intended behaviour of launching a timer with zero timeout (I know I read it somewhere in a different post, but can’t find it back)
  • can’t this be done in a different way in the Promise class in order not to get into this timer limit issue again ? In general, I would call for not using excessive nr of timers in any of the libraries… as was done quite properly in the V2.0.0 of the Bullwinkle class versus V1.x.x, btw.

Can we expect to see an increase or full abolishing of the agent timer limit (put it at 100 or so) anywhere in the future ??? That would make life easier I guess :slight_smile:


#2

I haven’t used Promises, but I did get around this by using the following:
`
timer <- {
_idleJobs = []
// Jobs can be queued here, to be executed when other immediate tasks have been completed.
// add function to idle queue and restore onIdle handler
function whenIdle(func){
_idleJobs.push(func)
imp.onidle(_onIdle.bindenv(this))
}

// called when agent is idle
function _onIdle(){ 
    // this may fail if object has expired and been garbage-collected
    while (_idleJobs.len()>0)
        _idleJobs.remove(0)()
}    

}
`

In place of imp.wakeup(0,func…) I use timer.whenIdle(func). You are only limited by RAM at that point. In practice, my implementation is slightly more complicated than this, as I may have the need to tidily remove a job before it is executed.


#3

My version is a bit more sophisticated, but it works without having to modify all of the existing libs by hacking my own imp.wakeup function to replace the real imp.wakeup via methmethod override. Certainly not the nicest solution to the imp cloud (lots of extra, unnecessary processing) nor the most elegant, but its what I use to prevent the possibility of running out of timers on the agent…

`
// This is a really simple, reusable wrapper for any internal object to turn it into a class (courtesy of @ blindman2k).
// For memory and performance reasons, the built-in classes are not real classes so can’t
// be extended in the normal way.
class impWrapper {
_wrapped = null;

constructor(wrapped) {
    _wrapped = wrapped;
}

// Pass through the original methods and properties
function _get(idx) {
    if (idx in this) {
        return this[idx];
    } else if (idx in _wrapped && typeof _wrapped[idx] == "function") {
        return _wrapped[idx].bindenv(_wrapped);
    } else if (idx in _wrapped) {
        return _wrapped[idx];
    } else {
        throw null;
    }
}

}

class ImpWithMasterTimer extends impWrapper {

_wakeups = null; //Store the wakeup remaining durations and callback functions
_wakeupKeys = null; //
_lastWakeupTime = null;

// basic property of .wakeup() is that we want to call the callback function after the duration has expired, in the order that it was registered
constructor() {
this._wakeups = {}; // Key is a timestamp guid, value is an array of [time until wakeup occurs, callback]
this._wakeupKeys = []; //Sorted array of timestamp guids so that we always fire callbacks in the order they were registered if they expire at the same time. Since we are only appending large things to the end and removing things, the array is never unsorted, and therefore never needs .sort()!
base.constructor(imp);

this._lastWakeupTime = this._timestamp();
this._run();

}

// Override imp.wakeup
function wakeup(duration, callback){
local guid = this._timestamp()
if(duration < 0 )
throw "imp.wakeup duration must be >=0 - got " + duration
this._wakeups[guid] <- [duration.tofloat(), callback]; //by using our timestamp funciton, we can ensure that our GUIDs are in fact unique and will sort properly since squirrel is nice and single threaded
this._wakeupKeys.push(guid) //Whatever we are pushing is “bigger” than all others so no need for a sort here (since squirrel is single-threaded) - we do that when things are removed
return guid
}

// Override imp.cancelwakeup
function cancelwakeup(id){
if(id in this._wakeups){
delete this._wakeups[id]
this._wakeupKeys.remove(this._wakeupKeys.find(id))
}
}

function _timestamp() {
local ts = date();
return format("%d.%06d", ts.time, ts.usec);
}

function _run(){
local now = this._timestamp();

//local diff = this._diff(this._lastWakeupTime, now);
local t0 = split(this._lastWakeupTime, ".");
local t1 = split(now, ".");
local diff = (t1[0].tointeger() - t0[0].tointeger()) + (t1[1].tointeger() - t0[1].tointeger()) / 1000000.0;

//server.log("Diff = "  + diff)
this._lastWakeupTime = now

local deleteIDs = [];
local activeTimers = 1; //This one is reserved for our main _run loop
for(local i=0; i < this._wakeupKeys.len(); i++) {
  local id = this._wakeupKeys[i];
  local wu = this._wakeups[id];

  wu[0] -= diff; //update the time until this timer should fire
  if(wu[0] <= 0.0) {
    activeTimers++
    //prevent the callstack from getting too deep and running us out of memory with imp.wakeup
    _wrapped.wakeup(0, wu[1]);
    deleteIDs.push(id)  // We don't delete here because modifying the thing we are looping through causes bad things to happen

    if(activeTimers == 20)  //Only allow up to 20 active Timers (since that's all the Agent will currently provide!)
        break;
  }
}

for(local i = 0; i<deleteIDs.len(); i++) {
    delete this._wakeups[deleteIDs[i]];
    this._wakeupKeys.remove(this._wakeupKeys.find(deleteIDs[i]))
}

_wrapped.wakeup(0.0, this._run.bindenv(this)); //Back to MasterTimer.wakeup

}
}

/**

  • We override our imp class so that we can limit our number of “real” imp.wakeup timers to 20, which is all that the agent will provide
    */
    imp <- ImpWithMasterTimer();
    `

#4

@deldrid1 Very interesting. I had never thought of “extending” the inbuilt meta objects.


#5

Very interesting approach indeed. I have my own version of a timer factory but struggling with the libraries that obviously don’t use it. Promise is only an example and the observed behaviour of the Agent when hitting the timer limit is that it simply doesn’t create the timer (no restart). It really depends on the underlying code what the effect is of that. i.e; unpredictable behaviour at runtime which is a big no for any good code.
This way of overriding builtin functions may cut it without forking all the used libs (and the hassle that comes with following through on lib updates)… Thx !


#6

@vedecoid That’s exactly why i took the approach that I did :slight_smile:


#7

A quick update on supporting more than 20 timers natively in agents:

  • we’ve completed some testing and design work and have identified a way to significantly increase this limit
  • a task to implement this design should reach the top of our backlog in the next couple of months.

#8

Has this update been implemented in the meantime ? If so, what would be the ‘new’ limit (if any ) ?


#9

There has been significant behind the scenes work on timers, but the limit has not been raised as yet… @harryreiss might know more on this.


#10

Once the new timer implementation is rolled out, the default timer limit will remain the same. You’ll need to raise a support ticket in order to ask for a higher limit.

Part of the change is that, if you’ve got a higher timer limit configured, and you exceed that limit, we’ll throw an exception, rather than quietly not creating a new timer. This won’t apply to agents using the defaults.


#11

That makes sense. Especially throwing the exception is good because the real problem is the quiet cancellation of creating a timer. With an exception you can retry because these peaks in timer usage happen only when certain events all come together which is a situation that won’t last for more then seconds.


#12

The new timer code is now deployed to all servers. The default timer limit and error behaviour remains the same as previously, though there are extensive performance and scalability improvements under the hood.

However, rather than require users to request increased limits, we’re considering simply raising the default timer limit and turning on the hard error behaviour for everyone.

That’s not set in stone, however; we’ll be watching the behaviour and performance of this deploy for a week or so before making that change, which will require another server deploy.


#13

Given how damaging it is to the flow of an application to silently discard timers, I personally would rather see an exception.

blindman2k


#14

I second that (but you probably already know how I feel about timers). Additionally, the “too-deeply-nested” warning drives me nuts, as a warning can’t be used to track the circumstance where something would fail. If an exception is raised, I would get a dump of the call stack, plus I can catch it and output my own diagnostics.


#15

Yes back a " try { … } catch { … } " approach for timers.