Open Bug 1122523 Opened 5 years ago Updated 2 years ago

Improve knockout compareSmallArrayToBigArray function performance

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

People

(Reporter: h4writer, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

No description provided.
Attached file testcase.js
Reduced the knockout benchmark to a 50 line testcase that has the same issue.

V8: 1866ms
SM: 3360ms
SM --no-ggc: 3894ms

So ggc already helped here. Also I don't see the spikes on this testcase. So this might be showing the non-minorgc slowdowns we have on this benchmark.
Blocks: 1118938
Observations:
1) We take 25%-50% longer to initialize all holes values when doing:
> a = []
> a[500] = 1

Because we store values having 64bits, while v8 stores 32bit smi values. Therefore depending on bug 1116855, since that will allow us to fix this by only storing objects, instead of object + tag.

2) Using push instead of thisRow[bigIndex] = will decrease performance of V8 a lot. Then they are the same speed as us. Are they doing something special?
> ...
>                } else if (smlArray[smlIndex - 1] === bigArray[bigIndex - 1]) {                   
> +                  if (thisRow.length == bigIndex)
> +                      thisRow.push(lastRow[bigIndex - 1]);
> +                  else
>                        thisRow[bigIndex] = lastRow[bigIndex - 1]; 
>                } else {                                                                              
>                    var northDistance = lastRow[bigIndex] || maxDistance;
>                    var westDistance = thisRow[bigIndex - 1] || maxDistance;
>  +                 if (thisRow.length == bigIndex)
>  +                     thisRow.push( myMin(northDistance, westDistance) + 1);                        
>  +                 else
>                        thisRow[bigIndex] = myMin(northDistance, westDistance) + 1;               
>                }      
> ...


V8: 4118
SM: 4445

V8: 3063
SM: 4437

Very interesting

3) Our thisRow[bigIndex] is using the OOL path the whole time, since we are appending after the length of the array. Now that is why I tried push, which for 99% of the cases uses the inline path. But as seen above, this doesn't help a lot ?!
Depends on: 1116855
bug 1106828 is a better bug to depend on, because that is also about unboxed arrays. There is one caveat and that is that the unboxed arrays would get boxed when having holes. (Which we have in this case)!
Depends on: 1106828
No longer depends on: 1116855
You need to log in before you can comment on or make changes to this bug.