Performance regression introduced or former bug fixed?

[louis-langholtz]

Expected/Desired Behavior or Experience:

1. Bug(s) fixed.
2. No performance regressions.

Actual Behavior:

A performance regression may have been introduced in commit 8082f1c. It seems pronounced in TOI processing and may be limited therein.

Steps to Reproduce the Actual Behavior:

This can be seen in the Benchmark AddPairStressTestPlayRho400 test at step 16 and beyond.

Results from before this commit:

AddPairStressTestPlayRho400/0          1141093 ns      1141066 ns          606
AddPairStressTestPlayRho400/10          348761 ns       348767 ns         2045
AddPairStressTestPlayRho400/15          399606 ns       399516 ns         1771
AddPairStressTestPlayRho400/16          649553 ns       649496 ns         1070
AddPairStressTestPlayRho400/17          632504 ns       632505 ns         1109
AddPairStressTestPlayRho400/18          632638 ns       632630 ns         1105
AddPairStressTestPlayRho400/19          629745 ns       629718 ns         1108
AddPairStressTestPlayRho400/20          627224 ns       627221 ns         1117
AddPairStressTestPlayRho400/30          581333 ns       581270 ns         1219

Results since this commit show drop-off at step 16 onwards:

AddPairStressTestPlayRho400/0          1135751 ns      1135759 ns          610
AddPairStressTestPlayRho400/10          343841 ns       343856 ns         2090
AddPairStressTestPlayRho400/15          392199 ns       392183 ns         1825
AddPairStressTestPlayRho400/16         7263209 ns      7263220 ns          100
AddPairStressTestPlayRho400/17        16049173 ns     16049156 ns           45
AddPairStressTestPlayRho400/18        24153032 ns     24153103 ns           29
AddPairStressTestPlayRho400/19         9867822 ns      9867849 ns           73
AddPairStressTestPlayRho400/20         6869526 ns      6869471 ns          104
AddPairStressTestPlayRho400/30         1789326 ns      1789270 ns          393

OTOH, a similar drop-off is seen when benchmarking the similar test case with Box2D:

AddPairStressTestBox2D400/0             943542 ns       943537 ns          739
AddPairStressTestBox2D400/10            532310 ns       532331 ns         1324
AddPairStressTestBox2D400/15            704578 ns       704590 ns          995
AddPairStressTestBox2D400/16          10533853 ns     10533615 ns           65
AddPairStressTestBox2D400/17          23079288 ns     23079161 ns           31
AddPairStressTestBox2D400/18          34102853 ns     34103000 ns           21
AddPairStressTestBox2D400/19          32312089 ns     32303045 ns           22
AddPairStressTestBox2D400/20          18573567 ns     18572949 ns           39
AddPairStressTestBox2D400/30           2562930 ns      2560591 ns          274

So either:

1. Code before this commit wasn't working properly; or,
2. Changes in this commit introduce a regression.

This commit seems to have been merged in as part of pull request #414. I'm recalling a PR around this time that I did believe may have found and fixed a real problem like this commit may be doing (fixing a bug of code before it).

[louis-langholtz]

Testbed (at step 16 showing step statistics) from build before the changes of this commit:

Testbed (at step 16 showing step statistics) from build with the changes of the commit:

The step stats are showing more work being done that step with the changes of the commit. Assuming the work indeed makes sense to be done, I'd expect a slow down. Not as much as seen however.

I'd really like to narrow down what part of the code diffs cause this performance change. Nothing has jumped out on me though while reviewing the diffs manually so far. There's changes to memory buffering but my testing of those isn't suggesting that's what I'm looking for.