Bug 235196

Created attachment 449103 [details] Patch

Comment on attachment 449103 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=449103&action=review > PerformanceTests/ChangeLog:12 > + this new API feature. That doesn’t make sense to me. The benchmark is intended as a proxy for performance on websites; changes in browsers should not result in changes to the benchmark. Changes to websites could, though. Has this change to the benchmark been discussed somewhere? Did the people discussing it consider the issue I am raising here?

Changing the benchmark before the websites change seems “cheeky” to me.

This change has not been discussed with anyone who Maintains MotionMark. We can discuss it here. Here is a summary of the situation: Many browser have heuristics to switch between between GPU and CPU-based rasterization of 2d canvas content. The reason for this is that GPU acceleration does not always perform better than without GPU acceleration, mostly because GPU readbacks are expensive, especially when they need to be synchronous, as is required by the getImageData and toDataURL methods. The problem with these heuristics is that they sometimes cause glitchy behavior when switching between GPU and CPU rendering on the fly. For example, anti-aliased edges might have a slightly different appearance. The sudden change in appearance may feel glitchy to users and the behaviour is sometimes surprising to web developers. Also, these heuristics result in performance characteristics that are surprising. For example: why is the first call to getImageData slower than the second one? To make the platform saner, it was proposed that these heuristic should be removed, and replaced by am API that gives developer explicit control in order to eliminate surprises. The new API feature is the willReadFrequently context creation attribute: https://html.spec.whatwg.org/multipage/canvas.html#concept-canvas-will-read-frequently Removing the heuristics will negatively affect the performance of web sites that call getImageData repeatedly without specifying {willReadFrequently: true} at context creation time. Therefore, there will be a devrel push to raise awareness of this issue so that Web devs can make the change before the heuristic removal goes live. This proposed change to MotionMark would assure continuity in performance for the benchmark. Basically it would assure that the benchmark continues to measure the same code path as before, which is the desirable code path.

(In reply to Justin Novosad from comment #4) > > To make the platform saner, it was proposed that these heuristic should be > removed, and replaced by am API that gives developer explicit control in > order to eliminate surprises. The new API feature is the willReadFrequently > context creation attribute: > > https://html.spec.whatwg.org/multipage/canvas.html#concept-canvas-will-read- > frequently > > Removing the heuristics will negatively affect the performance of web sites > that call getImageData repeatedly without specifying {willReadFrequently: > true} at context creation time. Therefore, there will be a devrel push to > raise awareness of this issue so that Web devs can make the change before > the heuristic removal goes live. > > This proposed change to MotionMark would assure continuity in performance > for the benchmark. Basically it would assure that the benchmark continues > to measure the same code path as before, which is the desirable code path. The benchmarks should measure the realistic browser performance. Making use of this new API in MotionMark should only be done after the vast majority of websites, NOT just the most popular websites, that use getImageData have adopted this new API. Otherwise, MotionMark will give unrealistically favorable numbers for browsers that implement such an new API but don't implement the aforementioned heuristics.

> Making use of this new API in MotionMark should only be done after the vast > majority of websites, NOT just the most popular websites, that use > getImageData have adopted this new API. Fair enough. I will add browser instrumentation to measure feature adoption. I will chime in at a later date once adoption is strong.

Comment on attachment 449103 [details] Patch r-ing this patch for now.

<rdar://problem/87846749>

I think this is a dup of https://github.com/WebKit/MotionMark/pull/1