Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Incrementally render by sending the operator list by chunks as they're ready. #3461

Merged
merged 1 commit into from
Jul 31, 2013

Conversation

brendandahl
Copy link
Contributor

To make this much easier I stripped out much of uneeded promise code for getting operator lists.

I've created RenderTask/InternalRenderTask to help with rendering that make it much easier to cancel the rendering and resume when more chunks are ready. I don't really like extending the Promise for page.render(), but this way we don't break anyone's code. I've also created an OperatorList class so we have one central one that we can flush as it is built. This also cleans up a lot of code since we had little pseudo operator list objects in a number of places.

Things we should further explore:

  • Chunk size - played a bit not sure what's optimal yet.
  • Render skipping images if they are ready, then restarting when they are.
  • On the main thread empty the operator list as it comes in, instead of appending it. This would help with memory but complicates things if we have multiple renderTasks going at once.

Should also note for complex pdfs this drops the rendering time quite a bit on my machine. For instance:
http://www.sfbike.org/download/map.pdf
Old code: nothing appears for 15s, 28s to finish rendering
New: appears ~2s, 15s to finish rendering

@brendandahl
Copy link
Contributor Author

/botio test

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_test from @brendandahl received. Current queue size: 0

Live output at: http://107.22.172.223:8877/b5ffa5c04ca1bd9/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux)


Received

Command cmd_test from @brendandahl received. Current queue size: 0

Live output at: http://107.21.233.14:8877/568cd594709cb8d/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux)


Failed

Full output at http://107.21.233.14:8877/568cd594709cb8d/output.txt

Total script time: 24.34 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: FAILED

Image differences available at: http://107.21.233.14:8877/568cd594709cb8d/reftest-analyzer.xhtml#web=eq.log

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://107.22.172.223:8877/b5ffa5c04ca1bd9/output.txt

Total script time: 28.95 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: FAILED

Image differences available at: http://107.22.172.223:8877/b5ffa5c04ca1bd9/reftest-analyzer.xhtml#web=eq.log

@Snuffleupagus
Copy link
Collaborator

@brendandahl This is a really impressive performance improvement!

If I understand it correctly, this PR supersedes #3164.

@brendandahl
Copy link
Contributor Author

/botio test

@pdfjsbot
Copy link

From: Bot.io (Linux)


Received

Command cmd_test from @brendandahl received. Current queue size: 0

Live output at: http://107.21.233.14:8877/69d02d7c5d405d8/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_test from @brendandahl received. Current queue size: 0

Live output at: http://107.22.172.223:8877/af25a5bcae60b65/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux)


Failed

Full output at http://107.21.233.14:8877/69d02d7c5d405d8/output.txt

Total script time: 23.75 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: FAILED

Image differences available at: http://107.21.233.14:8877/69d02d7c5d405d8/reftest-analyzer.xhtml#web=eq.log

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://107.22.172.223:8877/af25a5bcae60b65/output.txt

Total script time: 28.19 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: FAILED

Image differences available at: http://107.22.172.223:8877/af25a5bcae60b65/reftest-analyzer.xhtml#web=eq.log

@brendandahl
Copy link
Contributor Author

Waiting for #3477, then I'll update this.

@timvandermeij
Copy link
Contributor

@brendandahl Sounds like a great optimization! Can't wait to test out a preview. #3477 has been merged.

@brendandahl
Copy link
Contributor Author

/botio test

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_test from @brendandahl received. Current queue size: 0

Live output at: http://107.22.172.223:8877/2b0994cb1fcfad2/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux)


Received

Command cmd_test from @brendandahl received. Current queue size: 0

Live output at: http://107.21.233.14:8877/c275bea24571421/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux)


Failed

Full output at http://107.21.233.14:8877/c275bea24571421/output.txt

Total script time: 24.48 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: FAILED

Image differences available at: http://107.21.233.14:8877/c275bea24571421/reftest-analyzer.xhtml#web=eq.log

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://107.22.172.223:8877/2b0994cb1fcfad2/output.txt

Total script time: 28.24 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: FAILED

Image differences available at: http://107.22.172.223:8877/2b0994cb1fcfad2/reftest-analyzer.xhtml#web=eq.log

function pageStartRenderingFromOperatorListEnsureFonts() {
self.displayReadyPromise.resolve();
function complete(error) {
for (var i = 0; i < self.renderTasks.length; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think var i = self.renderTasks.indexOf(internalRenderTask); if (i >= 0) ... will look better

@ghost ghost assigned yurydelendik Jul 31, 2013
@yurydelendik
Copy link
Contributor

Looks good. We have to start playing with it more. Landing time?

/botio-windows preview

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_preview from @yurydelendik received. Current queue size: 0

Live output at: http://107.22.172.223:8877/b6ca2f8455ffea7/output.txt

@pdfjsbot
Copy link

@yurydelendik
Copy link
Contributor

/botio test

@pdfjsbot
Copy link

From: Bot.io (Linux)


Received

Command cmd_test from @yurydelendik received. Current queue size: 0

Live output at: http://107.21.233.14:8877/77a8fbae21c47e9/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_test from @yurydelendik received. Current queue size: 0

Live output at: http://107.22.172.223:8877/2eb7fda7021a849/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux)


Failed

Full output at http://107.21.233.14:8877/77a8fbae21c47e9/output.txt

Total script time: 23.03 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: FAILED

Image differences available at: http://107.21.233.14:8877/77a8fbae21c47e9/reftest-analyzer.xhtml#web=eq.log

@pdfjsbot
Copy link

From: Bot.io (Windows)


Failed

Full output at http://107.22.172.223:8877/2eb7fda7021a849/output.txt

Total script time: 28.61 mins

  • Font tests: Passed
  • Unit tests: Passed
  • Regression tests: FAILED

Image differences available at: http://107.22.172.223:8877/2eb7fda7021a849/reftest-analyzer.xhtml#web=eq.log

@yurydelendik
Copy link
Contributor

/botio makeref

@pdfjsbot
Copy link

From: Bot.io (Linux)


Received

Command cmd_makeref from @yurydelendik received. Current queue size: 0

Live output at: http://107.21.233.14:8877/78aca2dc935bfcf/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

Command cmd_makeref from @yurydelendik received. Current queue size: 0

Live output at: http://107.22.172.223:8877/3f28f53a05ed57b/output.txt

yurydelendik added a commit that referenced this pull request Jul 31, 2013
Incrementally render by sending the operator list by chunks as they're ready.
@yurydelendik yurydelendik merged commit 8d386a5 into mozilla:master Jul 31, 2013
@timvandermeij
Copy link
Contributor

That speed optimization is just... wow. :-D

That large map used to take 22 seconds to be fully rendered, whereas it now takes 7 seconds!

@pdfjsbot
Copy link

From: Bot.io (Linux)


Success

Full output at http://107.21.233.14:8877/78aca2dc935bfcf/output.txt

Total script time: 23.24 mins

  • Lint: Passed
  • Make references: Passed
  • Check references: Passed

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

Full output at http://107.22.172.223:8877/3f28f53a05ed57b/output.txt

Total script time: 28.72 mins

  • Lint: Ignored
  • Make references: Passed
  • Check references: Passed

panel.appendChild(content);
this.table = table;
},
updateOperatorList: function updateOperatorList(operatorList) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is dead code now, because updateOperatorList is not called anywhere.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never mind, it is called in display/api.js.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants