Bug 240605 – MotionMark's multiply suite reaches max complexity on faster devices

• Component: New Bugs
• Status: RESOLVED FIXED

[Scott Violet]

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.

[Scott Violet]

Created attachment 459558 [details]
Patch

[Scott Violet]

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.

[Simon Fraser (smfr)]

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.

[Scott Violet]

Thanks for looking Simon. Can you suggest how we move forward and what the timeframe would be for reving the version?

[Radar WebKit Bug Importer]

<rdar://problem/93931692>

[EWS]

No reviewer information in commit message, blocking PR #None

[Scott Violet]

Simon, any update as to how we can move forward here?

[Simon Fraser (smfr)]

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.

[Scott Violet]

Created attachment 460455 [details]
Patch

[Scott Violet]

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.

[Chris Dumez]

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.

[Scott Violet]

Created attachment 460459 [details]
Patch

[Scott Violet]

Created attachment 460460 [details]
Patch

[Scott Violet]

Created attachment 460462 [details]
Patch

[Scott Violet]

Created attachment 460463 [details]
Patch

[Scott Violet]

Created attachment 460464 [details]
Patch

[Scott Violet]

Created attachment 460465 [details]
Patch

[Scott Violet]

Created attachment 460466 [details]
Patch

[Kenneth Russell]

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.

[Chris Dumez]

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.

[Kenneth Russell]

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.

[EWS]

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].

[Simon Fraser (smfr)]

Retitled to be more accurate (sorry I didn't do this before it landed).