Created attachment 459557 [details] Chrome running the multiply suite On laptops the multiply suite has a maximum complexity of ~3100. STP 145 is pretty close to this as is Chrome. On recent MacBook Pros with ProMotion, Chrome receives an extremely low score (typically less than 10) even though it can generally handle the complexity (maintains a frame rate of 60hz). Chrome receives a low score because of two factors: 1. Chrome processes requestAnimationFrame at 120Hz. 2. The maximum complexity is not high enough. These two result in Chrome getting a low score on multiply because the line fitting used to determine the score generally does not cross the 60hz line. In this case the scoring typically uses the first value seen, which is a complexity of around 1. I've attached a screenshot illustrating this. I'm happy to attach any other data that would be helpful (perhaps the JSON results). All the other suites in MotionMark run long enough such that the line fitting uses a reasonable value.
Created attachment 459558 [details] Patch
Change the description to better reflect the problem as there are numerous possible solutions. I've attached a patch that increases the totalRows. totalRows directly influences the maximum complexity. In other words, this patch effectively increases the maximum complexity multiply runs to. This solution gives more headroom, and effectively fixes the issue. It may make sense to use a slightly higher number, possibly 60. As the scoring generally assumes 60hz, another possibility is to use Math.max(1000/60, frame_rate) when determining frame-rate for scoring. This is a bit more invasive, but is another possibility. This has the result of smoothing out the frame rates, so it may have other side effects. A final possibility is to run a number of initial frames to determine rAF, and then score based on how well that frame rate is maintained. This would mean you can not compare scores between Safari and Chrome or Firefox, which is not ideal.
Having MM respond to rAF frequency is something we plan, but out of scope for this particular bug. The patch in the bug seems reasonable, though. To incorporate it into the public benchmark we'd need to rev the version number, in which case we might also want to consider other minor improvements to the benchmark.
Thanks for looking Simon. Can you suggest how we move forward and what the timeframe would be for reving the version?
<rdar://problem/93931692>
No reviewer information in commit message, blocking PR #None
Simon, any update as to how we can move forward here?
We can land this patch (the commit message just needs a "Reviewed By" line), but it will be some time before we update the public benchmark version.
Created attachment 460455 [details] Patch
Sorry, I'm not familiar with the process. I uploaded a new patch (which should be exactly the same as the first). Please let me know if I need to do something.
Comment on attachment 460455 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=460455&action=review > COMMIT_MESSAGE:8 > +https://bugs.webkit.org/show_bug.cgi?id=240605 I believe the format of your commit message is wrong. The bug URL should be on the line right under the bug title. Also, you're missing the "Reviewed By Nobody" line and our commit hook should have added for you.
Created attachment 460459 [details] Patch
Created attachment 460460 [details] Patch
Created attachment 460462 [details] Patch
Created attachment 460463 [details] Patch
Created attachment 460464 [details] Patch
Created attachment 460465 [details] Patch
Created attachment 460466 [details] Patch
We couldn't figure out how to get webkit-patch to reinitialize the commit message, so tried modifying the patch's commit message locally. Let me help Scott try to commit this.
Comment on attachment 460466 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=460466&action=review > COMMIT_MESSAGE:2 > + If it fails, it will be because of this blank line here. > COMMIT_MESSAGE:3 > + And this one.
Comment on attachment 460466 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=460466&action=review >> COMMIT_MESSAGE:3 >> + > > And this one. Yes - we had difficulty figuring out how to edit the patch locally (in mbox format) to produce a well-formed COMMIT_MESSAGE. Deleting the blank line between the subject and the bug URL completely eliminated the newline between the two in the COMMIT_MESSAGE produced by the review tool. Hopefully this'll pass the CQ and then in future patches we'll use the Github PR workflow.
Committed 251810@main (86daf4fcece9): <https://commits.webkit.org/251810@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 460466 [details].
Retitled to be more accurate (sorry I didn't do this before it landed).