Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add a temporary function-cache in AlternateCS.prototype.getRgbBuffer #19428

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Snuffleupagus
Copy link
Collaborator

This supplements, rather than replaces, the existing caching in PDFFunction.constructPostScript since that one still makes sense given that functions are cached on the page-level.
Using an additional cache helps improve performance because:

  • There are many fewer function calls, and overall less parsing this way.

  • For the common case of Uint8Array-data we're able to use integer cache-keys, which is a lot faster than string concatenation.

This significantly improves performance of the pr5134 test-case with isEvalSupported = false set, and testing locally in the viewer:

  • With the master branch and isEvalSupported = true, page 2 renders in approx. 340 milliseconds.

  • With the master branch and isEvalSupported = false, page 2 renders in approx. 1200 milliseconds.

  • With this patch and isEvalSupported = false, page 2 renders in approx. 380 milliseconds.

While this is obviously still slower, the difference is now small enough that it shouldn't be too much of an issue in practice (compare with PR #18070) and the pr5134 test-case is an especially "bad" one w.r.t. its PostScript function use.

@Snuffleupagus
Copy link
Collaborator Author

/botio test

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.241.84.105:8877/7625531251bb676/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Received

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

Live output at: http://54.193.163.58:8877/cac5f481ec59fdc/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/7625531251bb676/output.txt

Total script time: 29.58 mins

  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: Passed

@Snuffleupagus Snuffleupagus force-pushed the AlternateCS-getRgbBuffer-tintCache branch from d36ce6c to 40d5b7c Compare February 6, 2025 11:31
@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/cac5f481ec59fdc/output.txt

Total script time: 58.83 mins

  • Unit tests: Passed
  • Integration Tests: FAILED
  • Regression tests: Passed

@Snuffleupagus Snuffleupagus force-pushed the AlternateCS-getRgbBuffer-tintCache branch 3 times, most recently from 648e0af to 47c9a21 Compare February 21, 2025 10:38
This supplements, rather than replaces, the existing caching in `PDFFunction.constructPostScript` since that one still makes sense given that functions are cached on the page-level.
Using an additional cache helps improve performance because:

 - There are many fewer function calls, and overall less parsing this way.

 - For the common case of `Uint8Array`-data we're able to use integer cache-keys, which is a lot faster than string concatenation.

This significantly improves performance of the `pr5134` test-case with `isEvalSupported = false` set, and testing locally in the viewer:

 - With the `master` branch and `isEvalSupported = true`, page 2 renders in approx. 340 milliseconds.

 - With the `master` branch and `isEvalSupported = false`, page 2 renders in approx. 1200 milliseconds.

 - With this patch and `isEvalSupported = false`, page 2 renders in approx. 380 milliseconds.

While this is obviously still slower, the difference is now small enough that it shouldn't be too much of an issue in practice (compare with PR 18070) and the `pr5134` test-case is an especially "bad" one w.r.t. its PostScript function use.
@Snuffleupagus Snuffleupagus force-pushed the AlternateCS-getRgbBuffer-tintCache branch from 47c9a21 to adfc49c Compare February 27, 2025 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants