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 an AppOptions.setAll method, and use it in PDFViewerApplication._readPreferences #12636

Merged
merged 2 commits into from
Nov 18, 2020

Conversation

Snuffleupagus
Copy link
Collaborator

Given that it's generally faster to call one function and have it loop through an object, rather than looping through an object and calling a function for every iteration, this patch will reduce the total time spent in PDFViewerApplication._readPreferences ever so slightly.
Also, over time we've been adding more and more preferences, rather than removing them, so using the new AppOptions.setAll method should be generally beneficial as well.

While the effect of these changes is quite small, it does reduces the time it takes for the preferences to be fully initialized. Given the amount of asynchronous code during viewer initialization, every bit of time that we can save should thus help.
Especially considering the recently added viewerCssTheme preference, which needs to be read very early to reduce the risk of the viewer UI "flashing" visibly as the theme changes, I figured that a couple of small patches reducing the time spend reading preferences cannot hurt.

…ess actually necessary

Given that only two debugging hash parameters (i.e. `disableWorker` and `pdfBug`) will make this method asynchronous, we can avoid what's most of the time is an unnecessary `Promise.all` invocation.
…n._readPreferences`

Given that it's generally faster to call *one* function and have it loop through an object, rather than looping through an object and calling a function for every iteration, this patch will reduce the total time spent in `PDFViewerApplication._readPreferences` ever so slightly.
Also, over time we've been adding more and more preferences, rather than removing them, so using the new `AppOptions.setAll` method should be generally beneficial as well.

While the effect of these changes is quite small, it does reduces the time it takes for the preferences to be fully initialized. Given the amount of asynchronous code during viewer initialization, every bit of time that we can save should thus help.
Especially considering the recently added `viewerCssTheme` preference, which needs to be read very early to reduce the risk of the viewer UI "flashing" visibly as the theme changes, I figured that a couple of small patches reducing the time spend reading preferences cannot hurt.
@timvandermeij
Copy link
Contributor

/botio-linux preview

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Received

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

Live output at: http://54.67.70.0:8877/3540c02116cbe6e/output.txt

@pdfjsbot
Copy link

From: Bot.io (Linux m4)


Success

Full output at http://54.67.70.0:8877/3540c02116cbe6e/output.txt

Total script time: 4.20 mins

Published

@timvandermeij timvandermeij merged commit d3936ac into mozilla:master Nov 18, 2020
@timvandermeij
Copy link
Contributor

Thanks!

@Snuffleupagus Snuffleupagus deleted the AppOptions-setAll branch November 19, 2020 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants