r82151 changed ShareableBitmap::paint to use CGBitmapContextCreateImage in its implementation, rather than creating an image using a custom data provider. On Mac, CGBitmapContextCreateImage uses a copy-on-write implementation, but on Windows it just copies the data. This means that we're now making an extra copy of every pixel that we paint into a WKView. This is likely a performance regression, though that has not been confirmed.
<rdar://problem/9198788>
It has now been confirmed that r82151 is a performance regression. Once upon a time we had a policy of rolling out performance regressions. I guess that's no longer the policy.
Created attachment 88643 [details] Patch for performance testing Here's a patch that should get rid of the extra copy. I haven't performance tested it, though.
This patch is a 5% speed-up on iBench HTML cached.
Comment on attachment 88643 [details] Patch for performance testing I don't think this patch is safe. If someone modifies the ShareableBitmap after the CGImageRef has been made but before the newMapping has been written into, the newMapping will see the modifications. This is exactly what we're trying to avoid. (And since newMapping should never be written into, we're basically never safe.) This is just how copy-on-write views work in Windows. If I have two views of the same mapping, A and B, and A has FILE_MAP_WRITE and B has FILE_MAP_COPY, B will see writes to A until B is written into. From then on B will be operating on its own copy that is insulated from A.
(In reply to comment #5) > (From update of attachment 88643 [details]) > I don't think this patch is safe. If someone modifies the ShareableBitmap after the CGImageRef has been made but before the newMapping has been written into, the newMapping will see the modifications. This is exactly what we're trying to avoid. (And since newMapping should never be written into, we're basically never safe.) > > This is just how copy-on-write views work in Windows. If I have two views of the same mapping, A and B, and A has FILE_MAP_WRITE and B has FILE_MAP_COPY, B will see writes to A until B is written into. From then on B will be operating on its own copy that is insulated from A. I guess we could use A for the CGImageRef and use B for the ShareableBitmap from then on. The CGImageRef can't be written to, so that should work.
(In reply to comment #6) > (In reply to comment #5) > > (From update of attachment 88643 [details] [details]) > > I don't think this patch is safe. If someone modifies the ShareableBitmap after the CGImageRef has been made but before the newMapping has been written into, the newMapping will see the modifications. This is exactly what we're trying to avoid. (And since newMapping should never be written into, we're basically never safe.) > > > > This is just how copy-on-write views work in Windows. If I have two views of the same mapping, A and B, and A has FILE_MAP_WRITE and B has FILE_MAP_COPY, B will see writes to A until B is written into. From then on B will be operating on its own copy that is insulated from A. > > I guess we could use A for the CGImageRef and use B for the ShareableBitmap from then on. The CGImageRef can't be written to, so that should work. Trying this out now.
Created attachment 92261 [details] Make ShareableBitmap::makeCGImageCopy use a copy-on-write copy on Windows when possible
Committed r85754: <http://trac.webkit.org/changeset/85754>