1

I recently learned that performance in for loops could be greatly increased by moving the length / count outside the loop and thought I would take a look at implementing that throughout my site.

Doing a full regex search using for\(.*<.*\.length showed that I had about 600 matches across 70 of the 12,000 files searched and as I was going through, I realized I was just cutting and pasting over and over with little variation where I would just copy the original code that calculated length, remove the dots and capitalize the first char.

For example: for(c=a.children.length-1;0<=c;c--)d would become

var AChildrenLength=a.children.length;
for(c=AChildrenLength-1;0<=c;c--)d

Once I realized what I was doing was monotonous, it seemed time for some fun with regular expressions which could be turned into a shell script that could modify regular or minified files.

Seemed simple enough to get started, but then I ran into some trouble with some of the matches that were separated by dots. Here are some examples of ones I am having a hard time matching:

for($j = 0; $j < incomingData.customer['data'].length; $j++){

for(c=a.children.length-1;0<=c;c--)d

for(l=0;l<g.length;l++){d=g[l]

// this is a fun one to look at, I didn't know you could do a for loop like that.
for(h in b)if(!h.match(d||/margin|text\-align|width|border|padding/i))if(g)g=!1;else{var l=new CKEDITOR.htmlParser.element("span");l.attributes.style=h+":"+b[h];c.add(l);c=l;delete b[h]}CKEDITOR.tools.isEmpty(b)?delete a.attributes.style:a.attributes.style=CKEDITOR.tools.writeCssText(b);for(b=0;b<e.length;b++)c.add(e[b])},sortStyles:function(a){for(var b=["border","border-bottom","font-size","background"],c=m.parseCssText(a.attributes.style),d=m.objectKeys(c),e=[],g=[],h=0;h<d.length;h++)

for(var f=0,n=0,l=0,p,e=a.$.rows.length;l<e;l++)

for(var i = 0; i < this.audioLayerControl.listOfSequenceEditors.length; ++i)

I am using this regex: (?:for\(.*<\W?)(([\w]+)\.?)+(?:\[?)|(?:\.length)

You can see what is matched color coded here: https://regex101.com/r/GsPieq/2

For some reason it is only matching the last word in each. Can someone help me understand why it isn't working? I need it to match between 1 and probably 5 words, separated by dots, not include the .length or anything in the brackets like {"data"] where I can then loop through each match transforming the first char to capital.

It should not match anything that has already been properly formed such as for(c=AChildrenLength-1;0<=c;c--)d

Here was my thought process:

(?:for\(.*<\W?) // Non capture but match any for loop up to the test
(
   ([\w]+)\.? // capture any alphanumeric char up to a dot if it exists
)+ // repeat if possible
(?:\[?)|(?:\.length)  // non capture find where it ends
Alan
  • 2,046
  • 2
  • 20
  • 43
  • 1
    https://stackoverflow.com/questions/5752906/is-reading-the-length-property-of-an-array-really-that-expensive-an-operation – rlemon Oct 30 '18 at 15:46
  • 1
    Alan, why are you worried about such trivial so-called micro-optimizations? – Rafael Oct 30 '18 at 16:00
  • Maybe you need to have a wrapper https://regex101.com/r/GsPieq/3 – revo Oct 30 '18 at 16:02
  • And probably it is better to have [`for\([^<)]*<\W*(\w+(?:(?!\.length)\.\w+)*)`](https://regex101.com/r/iiPGPh/1) – revo Oct 30 '18 at 16:10
  • Thanks Revo, I can use the output from this and split the string again. I was trying to do it all in a single query, but maybe I should just do it in steps. – Alan Oct 30 '18 at 16:11
  • @Rafael I have some javascript that accepts 10k-50k customer records, loops through to decide what to display, creates interactive map markers and popups. All that data is destroyed and rebuilt in real time as the map is interacted with. Trivial is only trivial till it affects the user experience. While there are some things I can't control, if this helps, it is worth it. – Alan Oct 30 '18 at 16:11
  • @rlemon that is a great find, I will test performance both ways to see what I can find. I have some pages that take about 2-3 seconds for the javascript to build after all the info is received from the server. – Alan Oct 30 '18 at 16:13
  • Your own regex will match some false-positives. Try the one I suggested. Do you mean all replacements in one go? – revo Oct 30 '18 at 16:14
  • I realize I didn't make my intentions clear, but I was thinking of having it match each word separately so that I could camel case them, but as I said, I can just make that a second line with `split('.')` – Alan Oct 30 '18 at 16:16
  • `I was thinking of having it match each word separately` You can do this in the Dot-Net regex engine using capture collection. Other engines only offer a finite, fixed (at compile time) group's. The work around is to capture the whole section in a single group, then work on that group as a second step. It could still all be done within a callback. See @revo regex for an example. –  Oct 30 '18 at 16:40
  • @Alan I __highly__ doubt that the length caching is doing anything to impact your performance (if anything at all today). If pages are taking long to load profile them and pinpoint the issue, stabbing in the dark like this isn't going to do any magic performance wise. – rlemon Oct 30 '18 at 16:57
  • I don't think you realize the initialization of `c` in your example is not _inside the loop_ - it is done before the loop starts, so this just slows it down a little. – NetMage Oct 31 '18 at 21:36

0 Answers0