New issue
Advanced search Search tips
Starred by 22 users

Issue metadata

Status: Assigned
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 2
Type: Feature

Blocking:
issue 585001
issue 768844



Sign in to add a comment

createImageBitmap should perform decoding on a background thread or using idle period decoding

Project Member Reported by junov@chromium.org, Jan 21 2016

Issue description

The current implementation of createImageBitmap could jank the main thread by posting a large decode task. This can be worked around by calling createImageBitmap on a Worker. Using a worker should not be necessary. the browser should be able to transparently schedule all the heavy lifting on a separate thread by using BackgroundTaskRunner
 

Comment 1 by junov@chromium.org, Jan 21 2016

Labels: M-50
IMHO: Launch of ImageBitmap does not need to be blocked on this, but this would be really nice to have in time for launch.
Status: Assigned
Summary: createImageBitmap should perform decoding on a background thread or using idle period decoding (was: createImageBitmap should perform decoding on a background thread)
Based on the performance evaluation results from canvas.toBlob, we should use the idle time decoding instead of using a background thread.
Project Member

Comment 5 by bugdroid1@chromium.org, Feb 4 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/68bba9563388e23e91e808c91d5ad60f58731fdb

commit 68bba9563388e23e91e808c91d5ad60f58731fdb
Author: xidachen <xidachen@chromium.org>
Date: Thu Feb 04 12:18:14 2016

Make decoding of createImageBitmap(Blob) work on a background thread

At this moment, the decoding step of this function always happens on the
same thread as the thread that calls this function. For example, if
createImageBitmap(Blob) is performed on the main thread, then decoding
happens on the main thread which could jank the main thread for large
images. This CL intends to move the decoding part to a background thread.

For this particular change, as long as all the current layout tests
(particularly the ones of createImageBitmap-blob) pass, it should not
cause any problem.

BUG=580202

Review URL: https://codereview.chromium.org/1644433003

Cr-Commit-Position: refs/heads/master@{#373513}

[modify] http://crrev.com/68bba9563388e23e91e808c91d5ad60f58731fdb/third_party/WebKit/Source/core/imagebitmap/ImageBitmapFactories.cpp
[modify] http://crrev.com/68bba9563388e23e91e808c91d5ad60f58731fdb/third_party/WebKit/Source/core/imagebitmap/ImageBitmapFactories.h

Status: Started
Labels: -M-50 M-51
aiming at M-51.

Comment 8 by ashlaa...@gmail.com, Feb 11 2016

The commit seems to simply run on a thread if the data is over ~2MB in size. This severely bottlenecks batch processing. If you have 100 blobs which are 1MB big, even optimistically saying each decode takes just 2ms (not to mention weak mobile devices could be 16ms+ per image!) leaves the app vulnerable to severe jank, and will bottleneck on trying to run everything through one core. Moving all decoding to a thread guarantees you can maximise throughput on multi-core systems. For example a batch image-processing webapp will take 200ms of main-thread jankiness to burn its way through those 100 images, but a quad-core system could in theory do it in just 50ms of jank-free processing.

Please stop relying on main thread idle time to do work, I've seen it in other places - you get at best 75% of a single thread, often less, and still risk janking. To actually use the computing power of modern multi-core processors there should be a lot more that schedules straight to a thread. As it is you leave web developers to reimplement this themselves via Workers, which is limited to the types that are Transferable, which isn't much.

Comment 9 by ashlaa...@gmail.com, Feb 11 2016

I'd add contrary to what xidachen@ suggested, toBlob ended up using threads efficiently via a shared thread pool in blink ( issue 67587   #138 ).
@ashlaaays: Thanks for your interests and comments. I believe there is a slight misunderstanding from your side. createImageBitmap(Blob) always uses a background thread to perform the decoding task no matter what is the size of the input image. The ~2MB is used to let the background task decides whether it is a long-run task or short-run task.

If you look at the function ImageBitmapFactories::ImageBitmapLoader::scheduleAsyncImageBitmapDecoding(), it always call BackgroundTaskRunner::postOnBackgroundThread() no matter what is the size of the input.

Hope that makes sense.
Also, the background thread used in createImageBitmap(Blob) is from the shared thread pool in blink
Are you sure? I have been experimenting with this and it still produces a lot of "Image decode" blocks in the devtools timeline which is what gave me this impression. However I am no expert on Blink code so I definitely could have misinterpreted the code! Is the timeline inaccurate or maybe that's a separate issue?

Comment 13 by junov@chromium.org, Feb 11 2016

Oh, this is interesting. It is possible that the dev-tools instrumentation needs to adapt to this change. It is possible that the work is being improperly reported in the dev-tools timeline.

Blocking: 585001
I'm also not familiar with Blink but it looks rendering thread is actually blocked when drawing bitmap image returned by createImageBitmap.

I tested with attached HTML file on ToT.
It looks createImageBitmap complete after 1ms, then it takes few seconds to render it.
While the bitmap image is being rendered, I cannot update the color of box with mouse hover CSS style.

index.html
920 bytes View Download
Cc: hirono@chromium.org junov@chromium.org
@hirono: Sorry, somehow I didn't get notification about your reply here. I did a bit of debugging with your html file. The story here is that the image is too big. If you go to the webpage that has the jpg file, you will found that the jpg file is 12000*12000 pixels.

I used your html file, and locally I saw that the createImageBitmap takes about 1ms, but the image-decoding which is running in a background thread, takes about 800ms because the image is too big.

Could you please try this:
1. Download the jpg file from the website in your html file. Save the jpg file to be under the same directory as your html file.
2. Use a image tool like GIMP, resize the image to be 1000*1000 instead of 12000*12000.
3. Change this line:
image.src = 'http://goes.gsfc.nasa.gov/pub/goes/080913.ike.poster.jpg';
in your html file to be image.src = 'img.jpg', where 'img.jpg' is the file that you downloaded and saved on your disk.
4. Load the html file again. Now the image decoding takes about 14ms on my desktop.

Please let me know if you have further concern. In my opinion, this bug should not block the bug that you are working on.
Hi xidachen@,
Thank you for talking a look at this.
I wonder if createImageBitmap should return ImageBitmap before completing image-decoding. The spec of ImageBitmap says it should be pointed to a canvas without undue latency.
https://html.spec.whatwg.org/multipage/webappapis.html#imagebitmap

I assumed createImageBitmap takes 800ms, and Canvas#drawImage completes immediately. It's fine that createImageBitmap takes 800ms since it returns ImageBitmap asynchronously via Promise.
WDYT?

Hi,
It appears that my understanding of undue latency is different. I think the point of undue latency is that when the createImageBitmap() promise is resolved, the created bitmap is ready to be drawn. So for an ImageBitmap to be ready to be drawn, it has to be fully decoded.

It really depends on what you need to do with the ImageBitmap. Does your subsequent work requires the ImageBitmap to be drawn to a canvas? For example, you could do something like this:

var p = createImageBitmap(img);
// do some work while ImageBitmap is not resolved
p.then(function(imageBitmap) {
  // do some work with the ImageBitmap
});

Does that make sense?
The spec is deliberately vague by using the term "undue latency".  So it is really up to implementors to determine the optimal meaning of that term.  What is obvious is that being gated on a network transfer is "undue latency".  Whether the image should be pinned on the GPU, pinned in the decode cache, pinned in the memory resource cache, or just pinned in the disk cache, is undetermined.

The current implementation in blink strictly requires ImageBitmaps to be pre-decoded in RAM, which is an implementation choice.  A choice that we may need to revisit, for example, if RAM consumption becomes an issue with pages that use this feature. For now, we should probably instrument the code to gather usage metrics to see how this feature behaves.
Hi, thank you again for the explanation.
This is the code I attached at #15.

console.time('createImageBitmap');
createImageBitmap(image).then(function(bitmap) {
  console.timeEnd('createImageBitmap');
  console.log('decoded');
  var canvas = document.createElement('canvas');
  canvas.width = 300;
  canvas.height = 300;
  canvas.getContext('2d').drawImage(bitmap, 0, 0, canvas.width, canvas.height);
  document.body.appendChild(canvas);
});

I meant it took 1ms to get the returned promise resolved, then Context#drawImage took 800ms to render the bitmap. Is the bitmap image pre-decoded?
Maybe I should try it again on ToT. Let me confirm.

Hi xidachen@,
I did an experiment with the following code.

document.querySelector('#decode-button').addEventListener('click', function(e) {
  var image = document.createElement('img');
  image.src = 'image.jpg';
  image.onload = function() {
    console.time('Promise resolved');
    createImageBitmap(image).then(function(bitmap) {
      console.timeEnd('Promise resolved');
      var canvas = document.createElement('canvas');
      canvas.width = 300;
      canvas.height = 300;
      document.body.appendChild(canvas);
      console.time('Draw bitmap');
      var context = canvas.getContext('2d');
      context.drawImage(bitmap, 0, 0, canvas.width, canvas.height);
      console.timeEnd('Draw bitmap');
      console.time('getImageData');
      context.getImageData(0, 0, canvas.width, canvas.height);
      console.timeEnd('getImageData');
    });
  };
});

And the console output is:

Promise resolved: 1.827ms
Draw bitmap: 73.201ms
getImageData: 2033.628ms

It takes 2 seconds to access actual pixel data while the promise returned by createImageBitmap resolved after 2 ms.
It looks Chrome does not complete decoding when the promise is resolved.
Dev tool also shows that Chrome didn't start image decoding before calling context.getImageData. (Please see attached image)

WDYT?

Screenshot 2016-03-14 at 18.07.29.png
68.2 KB View Download
Hi hirono@:
I think the dev tool might be a little bit off when displaying this. The createImageBitmap should take 2 seconds, where the major portion of it is image-decoding.
The dev tools' timeline may show the result in a wrong way. But I'm not sure if the results from console.time and console.timeEnd are also broken.

The test code measures the time between console.time and console.timeEnd for each step (resolving promise, context.drawImage, and context.getImageData. And it outputs the result:

Promise resolved: 1.827ms
Draw bitmap: 73.201ms
getImageData: 2033.628ms

Maybe I can replace console.time and console.timeEnd with new Date() to see if the results are correct or not. Let me try.

I measured the time by using Date class instead of console.time/console.timeEnd and the result looks similar to the previous one.

createImageBitmap and promise resolved: 2ms
drawImage: 87ms
getImageData: 2113ms

Cc: kbr@chromium.org xidac...@chromium.org noel@chromium.org
 Issue 592590  has been merged into this issue.
For me, as Web Developer, createImageBitmap() promise should be resolved only when decoding is complete and data is back on main thread, so I can know that for sure that it's here and draw to a user. Otherwise, it makes no sense to have this function all at.

Comment 27 by kbr@chromium.org, Mar 19 2016

Let's please change the implementation of createImageBitmap so that the ImageBitmap is fully decoded, on a background thread, before the Promise is resolved. Without this the feature does not offer an improvement over the current APIs.

Currently there is a WIP CL, with the change there, the ImageBitmap will always be fully decoded when the promise is resolved.
Owner: ----
Status: Available (was: Started)

Comment 30 by junov@chromium.org, Dec 12 2016

Owner: xlai@chromium.org
Status: Assigned (was: Available)

Comment 31 by junov@chromium.org, Jul 12 2017

Owner: xidac...@chromium.org

Comment 32 by junov@chromium.org, Jul 12 2017

Labels: -Type-Bug -M-51 M-61 Type-Launch
The way the implementation is going, It looks like we will be launching background rendering of SVG images in M-61.  Even though there are no API changes, we should treat this as a launch and make sure it is publicized to developers.

@xidachen: please take this through the launch process (Intent to ship, entry on chromestatus, etc.)
Project Member

Comment 33 by bugdroid1@chromium.org, Jul 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7a1fd06507b3a2e4b33b521398e08147b4185a7d

commit 7a1fd06507b3a2e4b33b521398e08147b4185a7d
Author: Xida Chen <xidachen@chromium.org>
Date: Wed Jul 12 22:47:38 2017

Move raster work of createImageBitmap(SVG) to background thread

Right now in html canvas 2d's drawImage API, the image decoding are done
on the main thread, and this task could be heavy when the image is big.
What we hope is to give people an option to call createImageBitmap(SVG)
which does the image decoding in a background thread, and then canvas
drawImage API can draw the image bitmap which is already in a decoded
state.

Bug: 580202
Change-Id: I79cfd46b4e2f154e9fde111b932125a9745571a2
Reviewed-on: https://chromium-review.googlesource.com/558398
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Justin Novosad <junov@chromium.org>
Commit-Queue: Xida Chen <xidachen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#486144}
[delete] https://crrev.com/f47145f491d963103d5e099f17d44e8c83074c9c/third_party/WebKit/LayoutTests/fast/canvas/canvas-createImageBitmap-svg-expected.txt
[modify] https://crrev.com/7a1fd06507b3a2e4b33b521398e08147b4185a7d/third_party/WebKit/LayoutTests/fast/canvas/canvas-createImageBitmap-svg.html
[modify] https://crrev.com/7a1fd06507b3a2e4b33b521398e08147b4185a7d/third_party/WebKit/Source/core/html/canvas/ImageElementBase.cpp
[modify] https://crrev.com/7a1fd06507b3a2e4b33b521398e08147b4185a7d/third_party/WebKit/Source/core/imagebitmap/ImageBitmap.cpp
[modify] https://crrev.com/7a1fd06507b3a2e4b33b521398e08147b4185a7d/third_party/WebKit/Source/core/imagebitmap/ImageBitmap.h
[modify] https://crrev.com/7a1fd06507b3a2e4b33b521398e08147b4185a7d/third_party/WebKit/Source/core/svg/graphics/SVGImage.cpp
[modify] https://crrev.com/7a1fd06507b3a2e4b33b521398e08147b4185a7d/third_party/WebKit/Source/core/svg/graphics/SVGImage.h
[modify] https://crrev.com/7a1fd06507b3a2e4b33b521398e08147b4185a7d/third_party/WebKit/Source/platform/graphics/Image.h

Comment 34 by junov@chromium.org, Jul 13 2017

Labels: OWP-Type-ChangeBehavior
Why is this issue Google-restricted?
 jakearchibald@: looks like it is due to the type=launch label.
Labels: -Type-Launch Type-Feature
Blocking: 768844
Owner: zakerinasab@chromium.org

Sign in to add a comment