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

Replace getAll with getKeys in PartialEvaluator_hasBlendModes to speed up loading of badly generated PDF files (issue 6961) #6971

Merged
merged 1 commit into from
Feb 10, 2016

Conversation

Snuffleupagus
Copy link
Collaborator

Some bad PDF generators, in particular "Scribus PDF", duplicates resources a lot at various levels of the PDF files. This can lead to PartialEvaluator_hasBlendModes taking an unreasonable amount of time to complete.
The reason is that the current code is using Dict_getAll, which recursively dereferences all indirect objects, which can be really slow. This patch instead uses Dict_getKeys, and then manually looks up only the necessary indirect objects.

I've added the PDF file as a load test. The most important thing here is probably to ensure that the file remains available in the repo, and the comment should help reduce the chance of regressions. (Note that locally, the load test times out without this patch, but we cannot really assume that that always happens.)

Fixes #6961.

@Snuffleupagus
Copy link
Collaborator Author

/botio-linux preview

@pdfjsbot
Copy link

From: Bot.io (Linux)


Received

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

Live output at: http://107.21.233.14:8877/1b80e8aebcddf84/output.txt

…o speed up loading of badly generated PDF files (issue 6961)

Some bad PDF generators, in particular "Scribus PDF", duplicates resources *a lot* at various levels of the PDF files. This can lead to `PartialEvaluator_hasBlendModes` taking an unreasonable amount of time to complete.
The reason is that the current code is using `Dict_getAll`, which recursively dereferences *all* indirect objects, which can be really slow. This patch instead uses `Dict_getKeys`, and then manually looks up only the necessary indirect objects.

I've added the PDF file as a `load` test. The most important thing here is probably to ensure that the file remains available in the repo, and the comment should help reduced the chance of regressions. (Note that locally, the `load` test times out without this patch, but we cannot really assume that that always happens.)

Fixes 6961.
@Snuffleupagus
Copy link
Collaborator Author

/botio test

@pdfjsbot
Copy link

From: Bot.io (Linux)


Received

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

Live output at: http://107.21.233.14:8877/2f4bb738ccf5b16/output.txt

@pdfjsbot
Copy link

From: Bot.io (Windows)


Received

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

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

@pdfjsbot
Copy link

From: Bot.io (Windows)


Success

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

Total script time: 20.11 mins

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

@pdfjsbot
Copy link

From: Bot.io (Linux)


Success

Full output at http://107.21.233.14:8877/2f4bb738ccf5b16/output.txt

Total script time: 21.35 mins

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

@yurydelendik
Copy link
Contributor

Looks good. Thank you for the patch.

yurydelendik added a commit that referenced this pull request Feb 10, 2016
Replace `getAll` with `getKeys` in `PartialEvaluator_hasBlendModes` to speed up loading of badly generated PDF files (issue 6961)
@yurydelendik yurydelendik merged commit 33b57d7 into mozilla:master Feb 10, 2016
@Snuffleupagus Snuffleupagus deleted the issue-6961 branch February 10, 2016 17:34
@timvandermeij
Copy link
Contributor

Awesome work, thanks for fixing this so quickly!

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.

None yet

4 participants