Slow performance when using findObj and sendChat for rolls

I have written a script to make a roll for a skill for each selected token and send the output to the chat. If someone could help me how to speed up this script I would be very happy. In my campaign this takes about 5-15 seconds for 2 rolls - which means 2 selected tokens. I can wait a bit, but the script becomes so slow it does not feel like it is responding for the user anymore, altough it will finish when the user waits. However, the time between clicking the button and until the result shows simply became too long. This obviously is not the whole code and I tried to strip everything not directly related to what I consider responsible for the slowness. I guess the script is slow either because of the line var tokens = findObj /.... or _.each /.... I have no idea how performant _.each is ... or simply because of the slowness of rolling from the API with sendChat. However, is there any way to obviously increase the speed by for example doing all rolls in a single sendChat instead of issuing several sendChat commands? I am not really sure where to start to find out how to improve this script. var handleSkillCheck = function(msg,skill){ for(var s in msg['selected']){//FOR EACH SELECTED GRAPHIC s = msg['selected'][s]; var id = s["_id"]; var tokens = findObjs({_type: 'graphic', _id: id,_subtype:"token"},{caseInsensitive: true}); for(var t in tokens){//FOR EACH SELECTED TOKEN t = tokens[t]; var represents = t.get("represents"); var character = ...//get the character "represents" var nameRegex = /repeating_skillsaction_[^_]+_skillsaction_id/; //each repeating row has an attribute id which I store using Sheet Workers such that I can retrieve a specific row //from the API and Sheet Workers var skillIDs = filterObjs(function(o){return o.get('type')==='attribute' && o.get('characterid') === represents && o.get('name').match(nameRegex);}); _.each(skillIDs,function(sid){ /* get the value of the repeating_skillsaction_<sid>_skillsaction_<field> from "character". * This essentiallyare the fields of a row. Each of those is a findObj call looking for matching name and character id. */ var sname,satt,sbase,smod,.... if(sname.toLowerCase().trim() == skill.toLowerCase().trim()){ //If the name of the skill matches the name specified by "skill" then we want to rol var pool = satt + sbase + smod; var process = function(x) { var RollValue = x[0].inlinerolls[0]; var roll = buildInline(RollValue, undefined, undefined); // this is stolen from powercards and does the html formatting for the inline rolls var s = parseInt(RollValue["results"]["total"]); var result = "&{template:roll}"...//format the output in a rolltemplate //send "result" to player/gm }; //roll from the API sendChat(t.get("name"), "/roll [["+pool+"d6sacs5cs6>5]]", process); } }); } } };

Here's the _.each implementation (I've removed some code blocks that are unreachable in the API's environment, and inlined a few functions to make this post slightly easier to read): _.each = function(obj, iteratee, context) { iteratee = optimizeCb(iteratee, context); var i, length; if (isArrayLike(obj)) { for (i = 0, length = obj.length; i < length; i++) { iteratee(obj[i], i, obj); } } else { var keys = _.keys(obj); for (i = 0, length = keys.length; i < length; i++) { iteratee(obj[keys[i]], keys[i], obj); } } return obj; }; _.keys = function(obj) { return _.isObject(obj) ? Object.keys(obj) : []; }; _.isObject = function(obj) { return typeof obj === 'function' || typeof obj === 'object' && !!obj; }; var isArrayLike = function(collection) { var length = collection == null ? undefined : collection.length; return typeof length == 'number' && length >= 0 && length <= (Math.pow(2, 53) - 1); }; var optimizeCb = function(func, context) { if (context === undefined) return func; return function(value, index, collection) { return func.call(context, value, index, collection); }; }; As you can see, the time complexity of _.each is O(n) where n is the number of keys. Object.keys is the only other potential consideration for adding to the time complexity (Math.pow is O(1)), and its native implementation is also O(n). Constants are ignored in time complexity analysis, so O(2n) is the same as O(n). Using a single sendChat would definitely be a good start for speeding up your code. Using regex matching within filterObjs may be a problem as well, but at least you're short-circuiting away most objects before running the regex. Also, since you have the object type and id, getObj will absolutely be faster than findObjs. If you want to be certain it's a token, simply check the subtype after getting the object: var token = getObj('graphic', id); if (!(token && token.get('subtype') === 'token')) { return; // if you change the outer-most for loop to _.each continue; // if you leave the outer-most for loop as-is } Your second for loop isn't necessary regardless, since the tokens array you have now is guaranteed to have either 0 or 1 element in it. You should also consider taking your process function declaration out of the loops, if you can. None of the code you've posted requires process to be within that _.each. But considering it's the only asynchronous operation in your entire script, definitely look at getting down to just one sendChat. Something like this should probably work: var rolls = []; _.each(skillIds, function(sid) { var sname,satt,sbase,smod,... pool = satt + sbase + smod; if (sname.toLowerCase().trim() === skill.toLowerCase().trim()) { rolls.push('[[' + pool + 'd6sacs5cs6>5]]'); } }); sendChat('', rolls.join(' '), process); Then in your process function, instead of just using inlinerolls[0], you would iterate over the inlinerolls array.

For the most part Brian is completely right.   However, I don't know if I'd suggest a single big sendChat() based on experience.  I used to do that with GroupInit, but found that if the rolls got particularly complex/nested, it would fail to expand them in random ways.  GroupInit rolls each token's initiative separately, has them fill a shared structure and call a finalize() function when finished.  The finalize() function only does something on the Nth execution, where N is the number of sendChat() calls.  That allows all the sendChat() calls to run in parallel, then synchronize when the final sendChat() finishes.  You can see this at line 933 in the current version. Paired down, it looks roughly like this: var rolls = [], results = [], finalize; rolls = /* build a list of things to roll */ finalize = _.after(rolls.length,function(){ / * use the results of all the rolls */ }; _.each(rolls,function(r){ sendChat('',r,function(msg){ results.push( /* some results */); finalize(); }); });

Thank you both. 1) How do I get the process function out there properly? If I define the variables and process out of the most outer for-loop I get as result a mixup of whatever value the variables have at the time process is called. If I define it after the for loop it is not defined when the sendChat is process and the rolls just go to the chat. The full process looks like this: var process = function(x) { var RollValue = x[0].inlinerolls[0]; var roll = buildInline(RollValue, undefined, undefined); var s = parseInt(RollValue["results"]["total"]); var result = "&{template:roll}"+ "{{text="+sname+"}}"+ "{{color=darkred}}"+ "{{basis="+satt+"+"+sbase+"}}"+ "{{mod="+smod+"}}"+ "{{total="+pool+"}}"+ "{{erfolge="+roll+"}}"; whisperToGM(x[0].who,msg.who,result);//This is essentially a sendChat call }; The values of the variables in the rolltemplate do not match anymore the correct values anymore If i pull process out of the _.each 2) Sorry if I did not understood the concept, Aaron, but I don't get why finalize would improve anything here? For each token in the inner most loop a sendChat for the roll is made. Which essentially should be the same as your _.each call on sendChat per roll. Why would it be better to build the rolls first and then again use ._each to roll them using sendChat. Doesn't this boil down to exactly the same? 3) Is there a better way to get all ids of one repeating section than the regex match filterObj call? Or is there a better way to get attributes of a character by name? I am using something similar to  getAttrByName  in the wiki for each and every attribute lookup.  4) How much can I rely on the API console log output when it comes to performance? I tried to find out what exactly is so super slow and just used log("I am at xyz now"); statements. Roll20 enters my callback for the chatmessage pretty quickly and there is a long list of  if-else branches (nested 2 levels deep with about 15 branches in the second level) of string comparisons "if action=="a" then do callA() else if action ="b" then do callB() ...) structure and it takes very long until I get the message that "handleSkillCheck" is called at all.  Then the output of everything inside "handleSkillCheck" is very fast. Edit: Okay, it looks like I can not rely on the log output at all when it comes to when which part of my code is reached. If I remove the call to handleSkillCheck this vastly influences when the log output before the call is printed to the console. Edit2: Ok I identified one of the biggest slow downs. Getting the attributes of each row is super slow. What is the best way to get all attributes of a repeating row? I have simply removed everything inside the loops but the attribute lookup and this is what makes everything so slow. The asynchronous call obviously adds to this slowness, but this is the bigger part of the waiting time. FOR the character of EACH selected token DO var nameRegex = /repeating_skillsaction_[^_]+_skillsaction_id/; var skillIDs = filterObjs(function(o){return o.get('type')==='attribute' && o.get('characterid') === represents && o.get('name').match(nameRegex);}); _.each(skillIDs,function(sid){ sid = sid.get("current"); var a = findObj(... name:"repeating_<section>_"+sid+"_<fieldA>" ... var b = findObj(... name:"repeating_<section>_"+sid+"_<fieldB>" ... var c = findObj(... name:"repeating_<section>_"+sid+"_<fieldC>" ... ... I assume this is such a huge slow down because each findObj has to work with all objects and then filter all of them, therefore I begin to search FOR EACH token FOR EACH row N times, where n is the number of attributes of a row. Is there a way to do subsequent searches on previous cached search results? Edit3: I am down at 6 seconds from 15 seconds with two filterObject calls. In the first I get all IDs and then per ID I once call filterObj to get all fields of a row at once, process them in a for instead of a _.each. Still a long way to go, I hope I can get down to something like 2-3 seconds, because 6 seconds feels slow.

1) You should be able to just define your process function as a sibling of the handleSkillCheck function. 2) I'm not sure what you're doing with your results, but from experience, trying to join them all together and do one huge roll, then split the results back out doesn't work.  If you need to have all the results together at the end and say, present table of the results, then what I'm describing would allow you to do that by rolling them all separately and collecting the results, then using them when the last one finishes. Possibly not what you need to do though. 4) Logging is asynchronous, so you can't really time the log to get an idea of how fast things are running.  You can use  _.now() to get the current time in milliseconds and store it before and after a function call to see how long it took: let before,after; before=_.now(); doSomething(); after=_.now(); log(`Elapsed time for 'doSomething()' was ${ (after-before)/1000 } seconds`); 3) There isn't a better way, but there is probably a better time. I've rewritten your first set of code (see below) the way I would do it, down to the point where you actually process. It starts by building lookups for all the characters and repeating attributes using a single filterObjs() call.  I used the above code to time it.  My test game has 13 characters with repeating rows which have 140 repeating row attributes, here's the results of the timing: "Elapsed time for 'building lookups' was 0.035 seconds" "Characters: 13" "Attributes: 140" Here's the actual code: // your process function var process = function(x) {   "use strict";   var RollValue = x[0].inlinerolls[0];    // this is stolen from powercards and does the html formatting for the inline rolls   var roll = buildInline(RollValue, undefined, undefined);   var s = parseInt(RollValue.results.total);   var result = "&{template:roll}"; //format the output in a rolltemplate   //send "result" to player/gm }; var handleSkillCheck = function(msg,skill){   "use strict"; // --- Building Lookups --- //   let attrIDLookup={},   attrLookup={};   // one filterObjs call to build a full lookup for all characters and all repeating attributes   filterObjs( o => {     let matches;     if( o.get('type') === 'attribute') {       matches = o.get('name').match(/^repeating_(.*)_(-[^_]+)_(.*)$/);       if(matches) {         attrIDLookup[o.get('characterid')] = (attrIDLookup[o.get('characterid')]||{});         attrIDLookup[o.get('characterid')][matches[1]] = _.uniq((attrIDLookup[o.get('characterid')][matches[1]]||[]).concat(matches[2]));         attrLookup[matches[2]] = ( attrLookup[matches[2]] || {})[matches[3]]=o;       }     }     return false;   }); // --- finished building lookups --- //   // chain to build the collection of selected tokens and their characters   _.chain(msg.selected)   // turns chain into <roll20 token object>s   .map( s => getObj('graphic',s._id))   // gets rid of selected things that weren't graphics   .reject(_.isUndefined)   // adds represented character and switches chain to object containing token and character   .map( t => {     return {       token: t,       character: getObj('character',t.get('represents'))     };   })    // remove tokens that didn't represent a character   .reject( o => _.isUndefined(o.character))   .each( o => {     // o is an object with { token: <roll20 token object>, character: <roll20 character object> }     //     // attrIDLookup is an object with properties for each character.id     // mapping to an object with properties for each repeating section.  That section contains     // an array of ids for rows of that section.     //     // attrLookup is an object with properties for each row id which contains an object with     // properties for each field in that row which contain the <roll20 attribute object>     //      // This will iterate across each of the row ids of the skillsaction repeating section for     // the currently considered character     _.each(attrIDLookup[o.character.id].skillsaction, (id) => {       // fields will be an object with each of the attributes for this row       let fields=attrLookup[id];       // assuming there is a repeating_skillsaction_<ID>_skill_name, this will compare the entered       // name to the passed in skill name       if(fields.skill_name && fields.skill_name.get('current') === skill){         // Not sure where these come from:         //   var pool = satt + sbase + smod;         //         // but if they are repeating attrs for this row:         //   'repeating_skillsactions_<ID>_sattr'         //   'repeating_skillsactions_<ID>_sbase'         //   'repeating_skillsactions_<ID>_smod'          //         var pool = parseInt(fields.satt.get('current'),10) +           parseInt(fields.sbase.get('current'),10) +           parseInt(fields.smod.get('current'),10);         //roll from the API         sendChat(''/* name not necessary going to api */, "/roll [["+pool+"d6sacs5cs6>5]]", process);       }     });   }); };

Holy ... Wow. I will have to take some time until I have worked through that. Thank you Aaron for your effort - I will definetly look through that, but this will take some time! In the meantime I have found a solution on my own and the solution has been to simply not use findObj/filterObj more than once and only work with _.filter from there one. This means I have to process all attributes only a single time. All attributes of a character only a single time and finally all skills and all rows only a single time. The important thing in my solution is how _.filter is applied to subsequent subsets of previous _.filter operations to narrrow down from all attributes->character attributes -> repeating section -> row -> field Just one question about the sibling process function: How would I pass the skillname to this function, s.t. the template prints the skillname? If i define the function inside of all the loops/_each then I am able to rely on the javascript context. This is my solution so far which returns almost instantly. I will of course work through yours! attrs = filterObjs(function(o){return o.get('type')==='attribute'});//request ALL attributes from roll20. Huge set -> very slow FOR EACH token represents = token.get("represents") chr_attr = _.filter(attrs,function(o){return o.get('characterid') === represents});//attributes of this character. Large set -> quite slow chr_skillsaction = _.filter(chr_attr,function(o){return o.get('name').match(/repeating_skillsaction_.+/g); });//attributes of the repeating section. Medium set -> slow ids = _.filter(chr_skillsaction,function(o){return o.get('name').match(IDregex);});//filter all IDs. Small set -> FAST _.each(ids,function(id){ rowregex = new RegExp("repeating_skillsaction_"+id+"_skillsaction_[^_]+"); row = _.filter(chr_skillsaction,function(o){return o.get('name').toLowerCase().match(rowregex);});//Small set. Fast. a = _.filter(row,function(o){return o.get('name').toLowerCase() == "repeating_skillsaction_"+id+"_skillsaction_attribut";})[0].get("current");//Tiny set. Super fast. ... )};

Yeah, this is pretty much the age old trade off in Computer Science: Speed vs. Size That first line could be: attrs = findObjs({type: 'attribute'}); Not sure there's an efficiency gain there, but certainly the lack of a stack frame can't hurt. For the process function, just add it as another parameter: var process = function(skillname, msg) {   "use strict";   var RollValue = msg[0].inlinerolls[0];    // this is stolen from powercards and does the html formatting for the inline rolls   var roll = buildInline(RollValue, undefined, undefined);   var s = parseInt(RollValue.results.total);   var result = "&{template:roll}"; //format the output in a rolltemplate   //send "result" to player/gm }; Then when you pass it to sendChat(), you can use _.partial() to bind the first parameter: sendChat('','[[ expression ]]', _.partial(process,skill)); _.partial() takes a function (the function, not an invocation of the function) and returns a function that will call the passed in function with the parameters filled in from the additional list of parameters.  Example: var func = function(a,b,c,d,e){ } /* this: */ var pfunc = _.partial(func,1,2,3,4); /* is equivalent to this: */ var pfunc = function(e){ return func(1,2,3,4,e); };

Oh wow, I have been looking for something like partial since I began writing rolls :D Thank you!

Yeah, underscore has lots of nice functions. =D You can also leave earlier parameters unfilled like this: var func = function(a,b,c,d,e){ } /* this: */ var pfunc = _.partial(func,1,_,3,4); /* is equivalent to this: */ var pfunc = function(b,e){ return func(1,b,3,4,e); }; You can feel secure knowing that _.partial() only became available in the API a few months ago, so you haven't been misisng it for long.  =D