Some perf improvements to sheriff-o-matics updates.

Early out from sheriff-o-matic loop.

Don't compute all the keys for a loop that we almost
always early out from. Also, the early out was wrong because
it was just returning from the anonymous function.

NOTRY=true

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181699

[ojan, 2014-09-10 02:22:10]

ojan@chromium.org changed reviewers:
+ esprehn@chromium.org, teravest@chromium.org

[esprehn, 2014-09-10 02:40:23]

https://codereview.chromium.org/557173002/diff/1/Tools/GardeningServer/model/...
File Tools/GardeningServer/model/ct-failure-group.html (right):

https://codereview.chromium.org/557173002/diff/1/Tools/GardeningServer/model/...
Tools/GardeningServer/model/ct-failure-group.html:50: var resultNodes =
this.data.dataToExamine()[i].resultNodesByBuilder;
What does dataToExamine() do? Calling it repeatedly in a loop looks suspect
here.

https://codereview.chromium.org/557173002/diff/1/Tools/GardeningServer/model/...
Tools/GardeningServer/model/ct-failure-group.html:57: return totalFailures == 1;
This can just return true right? If you got here you must have <= 1
totalFailures.

https://codereview.chromium.org/557173002/diff/1/Tools/GardeningServer/model/...
File Tools/GardeningServer/model/ct-failure.html (right):

https://codereview.chromium.org/557173002/diff/1/Tools/GardeningServer/model/...
Tools/GardeningServer/model/ct-failure.html:71: return str.replace(':', '_');
This doesn't do the same thing, it used to replace all : now it only does the
first one.

[ojan, 2014-09-10 03:41:18]

ptal

https://codereview.chromium.org/557173002/diff/1/Tools/GardeningServer/model/...
File Tools/GardeningServer/model/ct-failure-group.html (right):

https://codereview.chromium.org/557173002/diff/1/Tools/GardeningServer/model/...
Tools/GardeningServer/model/ct-failure-group.html:50: var resultNodes =
this.data.dataToExamine()[i].resultNodesByBuilder;
On 2014/09/10 02:40:23, esprehn wrote:
> What does dataToExamine() do? Calling it repeatedly in a loop looks suspect
> here.

Yeah, this isn't right. Particularly, this concept only applies to one of the
types of failure groups.

https://codereview.chromium.org/557173002/diff/1/Tools/GardeningServer/model/...
Tools/GardeningServer/model/ct-failure-group.html:57: return totalFailures == 1;
On 2014/09/10 02:40:23, esprehn wrote:
> This can just return true right? If you got here you must have <= 1
> totalFailures.

Technically it's possible to have no failures in the list and we'd want to
return false. Not sure that can actually happen in practice though.

[esprehn, 2014-09-10 03:59:14]

lgtm

https://codereview.chromium.org/557173002/diff/20001/Tools/GardeningServer/mo...
File Tools/GardeningServer/model/ct-sheriff-failure-group-data.html (right):

https://codereview.chromium.org/557173002/diff/20001/Tools/GardeningServer/mo...
Tools/GardeningServer/model/ct-sheriff-failure-group-data.html:32:
CTSheriffFailureGroupData.prototype.failedOnce = function() {
This is much better.

[ojan, 2014-09-10 04:06:15]

The CQ bit was checked by ojan@chromium.org

[commit-bot: I haz the power, 2014-09-10 04:06:49]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/ojan@chromium.org/557173002/20001

[commit-bot: I haz the power, 2014-09-10 04:07:20]

Committed patchset #2 (id:20001) as 181699