Circumventing 'Possible infinite loop detected' for slow operations

As a consequence of  Issue , loops that try to find a large number of attributes in a large campaign take very long to complete, so long that the internal infinite loop detector goes off and the API goes down. Now, that's bad. I'm trying to circumvent that issue by performing all operations inside the loop in an asynchronous way. Now, I've been trying this (simplified version of the real deal, but that doesn't matter): list.forEach(function(charid) { _.each(setting, function(attrValue,attrName) { _.delay(setSingleAttribute, 1000, charid, attrName, attrValue); }); }); Here, setSingleAttribute is the function that does all the expensive stuff (there's one findObjs call in each execution, plus possibly a createObj, plus setting that attribute to a value). This seems to work just fine if list  is short. If list  is very long (e.g. all characters in a 200+ character campaign with a lot of attributes from the sheet), it does manage to go through the loop successfully ... and then never seems to do any of the stuff inside  setSingleAttribute (I've added a sendChat debug call inside setSingleAttribute, and that never gets sent to chat, not even once)? Then, after a while the API crashes with 'possible infinite loop detected'. Now, I've tried to do this with _.defer and setTimeout too, for the same effect. But then, I know nothing about JavaScript. Can this be done asynchronously in a way that does not crash the API? All the executions of setSingleAttribute are independent, and I really don't care when they finish or how long they take, just that they finish eventually without complaints by the API. Forum, can you help me? For reference, here are (again, simplified versions of) the functions called by the above loop. setSingleAttribute = function (charid, attrName, attrValue) { let attr, current, max; attr = myGetAttrByName(charid,attrName,createMissing); if (attr) { if (attrValue.current !== undefined) { attr.set('current', attrValue.current); } if (attrValue.max !== undefined) { attr.set('max',attrValue.max); } } }, myGetAttrByName = function(charid, attrName) { let attr = findObjs({type: 'attribute', characterid: charid, name: attrName}, {caseInsensitive: true})[0]; if (!attr) { attr = createObj('attribute', {characterid: charid,name: attrName}); } return attr; },

The problem is that instead of doing all the work right now and causing the heartbeat check to fail, you do the same amount of work in 1000ms. What you need to do is build a Queue of work to perform and have your work function do the first thing and then call itself on a delay recursively. That will allow it to spread the work out without triggering the heartbeat fail. I use about 50ms between recursive calls.  Let me know if you want an example. I'd type you one, but I'm on my phone... :)

I would love an example, no idea how to write something like this.

Ok. I'll get some coffee and my laptop...

The Aaron, you're slowly inspiring me to learn js. I wanna be as cool as you and help people

@Franky H. -- Hahahahah, you should do that. =D So, here's an example that does the delay by character:        doDelayByCharacter = function(list,setting){         // cloning the list since arrays are passed by reference, I don't want to modify the source array         var cList = _.clone(list),             // Work function to do all the work for one character             dWork = function(charid){                 // the actual work                 _.each(setting, function(attrValue,attrName) {                     setSingleAttribute(charid, attrName, attrValue);                 });                 // Base case for recursion, there is still work to do                 if(cList.length) {                     // put off this function 50ms and do the next item on the list                     _.delay(dWork, 50, cList.shift());                 }             };         // start the recursive chain.         dWork(cList.shift());     }; That gives the concept. Something else you can do is make more efficient use of findObjs()/filterObjs().  I've not texted the below, but it should illustrate the idea.  findObjs() and filterObjs() both iterate over all objects in the Game when you call them.  If you have to look at all of them anyway, you might as well get everything you need on the first go.  Here's a function based on your other two which does all the setting of attributes for a character with one filterObjs() call:     setMultipleAttributes = function(settings, charid){         var aKeys = _.keys(attribute),             attrs={};                      // Normally, you'd return true to get an array of attributes.           // In this case I'm using filterObjs() to iterate the attributes // and collect them in an object by name.           filterObjs(function(o){             if(o.get('characterid')===charid && _.contains(aKeys,o.get('name'))){                 attrs[o.get('name')]=o;                 return true;             }         });         // create all the attributes that were not found         _.each(_.difference(aKeys,_.keys(attrs)),function(key){             attrs[key]=createObj('attribute', {characterid: charid,name: attrName});         });         // set all the attributes         _.each(settings, function(v,k){             if(_.has(v,'current')){                 attrs[k].set('current',v.current);             }             if(_.has(v,'max')){                 attrs[k].set('max',v.max);             }         });     }, You could also apply this same idea to doing all the attribute sets for all the characters in one go, which might mean you don't need the delayed calls version.

Amazing, you're the best. I really want to implement your second suggestion (which is not so simple since I've had to do all sorts of hacks to get repeating attributes working as they should), but for now, the first one will work great. Thanks!

No worries!  After I finish moving in a few weeks, I'd be happy to help more directly. :)

Now you've got me thinking if I can't implement the second approach for all characters at once, but something is fishy here. The following code takes forever to execute (again, only if list  is large, and in a campaign with a lot of characters - but it takes 248 ms for four characters alone in the same large campaign).  filterObjs(function(o) { if (o.get('_type') === 'attribute') { id = o.get('_characterid'); name = o.get('name'); if (_.contains(list,id) && _.contains(allKeys,name)) { allAttrs[id][name] = o; return true; } } }); I don't get it, the only thing that happens if you increase the size of list is that you do more runs of allAttrs[id][name] = o; How can that take longer than 30 seconds? (EDIT: and, granted, more calls of _.contains, but now we're getting ridiculous, list has about 200 entries, not 200 million).

This may be related to what Lucian is talking about regarding findObjs in this  bug thread .

I agree, that might be all the more reason to do the filterObjs() one time and build a lookup by characterid to each of the attributes you're interested in.  The script Scott mentions in the bug thread (rewrite of RecursiveTables) caches all the table rows locally at startup (and maintains that cache as tables are add/modified/deleted).  I feel like we tried doing them all in one go but found it killed the script.  Possibly more profiling would reveal something we didn't realize.  Hopefully it's something that can be fixed soon, otherwise, you might have to bite the bullet and build a similar caching system...

Yep, we also tried doing it asynchronously with promises (which solved many other problems), but to no avail.

I will definitely not want to build a caching system for what is essentially an edge case in ChatSetAttr for large campaigns. So I suppose I'll have to go with the recursive version until the time that roll20 fixes their database access time. Because if a single filterObjs in the whole function can't do the trick, I don't know what will.

That's fair. =D  Possibly I can help with something along those lines after my move...