I have an array of a couple thousand strings

['7/21/2011', '7/21/2011', '7/21/2011', '7/20/2011', etc]

I am currently, running this code to group by the string and get the max group value:

var max = 0;
var group = {};
arr.map(function (value) {
  if (group[value]) {
    group[value]++;
  } else {
    group[value] = 1;
  }
  max = Math.max(max, group[value]);
});

Are there any improvements to make this code run faster?

EDIT: The results are in:

EDIT EDIT: that test was flawed. Mike Samuel's code was the fastest.

6000 entries test ->

10K entries test ->

I have an array of a couple thousand strings

['7/21/2011', '7/21/2011', '7/21/2011', '7/20/2011', etc]

I am currently, running this code to group by the string and get the max group value:

var max = 0;
var group = {};
arr.map(function (value) {
  if (group[value]) {
    group[value]++;
  } else {
    group[value] = 1;
  }
  max = Math.max(max, group[value]);
});

Are there any improvements to make this code run faster?

EDIT: The results are in: http://jsperf.com/javascript-array-grouping2

EDIT EDIT: that test was flawed. Mike Samuel's code was the fastest.

6000 entries test -> http://jsperf.com/javascript-array-grouping2

10K entries test -> http://jsperf.com/javascript-array-grouping

• javascript
• arrays

• 1 Sounds like a job for codereview.stackexchange.com – Sebastian Paaske Tørholm Commented Jul 21, 2011 at 19:39
• @Sebastian, Sorry, still new to asking questions? Can I migrate it myself? – Joe Commented Jul 21, 2011 at 19:42
• @Joey: I see a couple of good answers below. You could try them out using jsperf.com - I'd be curious to see the results – Flambino Commented Jul 21, 2011 at 20:02
• @Joey: If you wish to migrate, you can flag for moderator attention, but it looks like the question worked fine here. :) Up to you, really. – Sebastian Paaske Tørholm Commented Jul 31, 2011 at 12:27

3 Answers 3

If you're sure this is a hotspot and speed is really important, I would try to cut out several thousand function calls by inlining max and map.

You can also make the body of your function faster by cutting out a comparison.

var max = 0;
var group = {};
for (var i = arr.length; --i >= 0;) {
  var value = arr[i];
  var n = group[value] = 1 - -(group[value] | 0);
  if (n > max) { max = n; }
}

The best thing to do is measure on the browsers you care about.

Yes certainly. I would calculate the max last, instead of every iteration, and not use an if:

var group = {};
arr.map(function (value) {
    group[value] = (group[value] || 0) + 1;
});

var max = 0;
for (key in group) {
    if (group[key] > max) max = group[key];
}

EDIT: As Mike Samuel says you might get faster by using an index instead of map:

var group = {};
var max = 0;

for (var i = arr.length; --i >= 0;) {
    group[value] = (group[value] || 0) + 1;
}
for (key in group) {
    if (group[key] > max) max = group[key];
}

I think it really depends on the JS engine that you will run this code on. An alternative I think it's worth a shot is using

n = group[value] = (group[value]||0) + 1;
if (n > max) max = n;

for each element.

I also think that may be using a regular loop can be faster because the variables you will use will be just locals and not closed-over variables of a closure (that are normally slower) and you will also save a function call per element. Both those problems are non-issues if the implementation can inline this closure, but I don't know if there are JS implementations smart enough for that.