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

Inline the flushChunks helper function, used in getPdfManager on the worker-thread #18992

Merged

Conversation

Snuffleupagus
Copy link
Collaborator

  • This helper function has only a single call-site, and the function is fairly short.

  • It'll only be invoked if range requests are disabled, or if the entire PDF manages to load before the headers are resolved (which is very unlikely).
    Hence, by default, this helper function is not invoked.

  • By inlining the code we're able to utilize the existing error-handling at the call-site, rather than having to duplicate it, which further reduces the size of this code.

Finally, while slightly unrelated, this patch also adds optional chaining in one spot in the file (PR #16424 follow-up).

…the worker-thread

 - This helper function has only a single call-site, and the function is fairly short.

 - It'll only be invoked if range requests are *disabled*, or if the entire PDF manages to load *before* the headers are resolved (which is very unlikely).
   Hence, by default, this helper function is not invoked.

 - By inlining the code we're able to utilize the existing error-handling at the call-site, rather than having to duplicate it, which further reduces the size of this code.

Finally, while slightly unrelated, this patch also adds optional chaining in one spot in the file (PR 16424 follow-up).
@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/15f1b6e27396d9c/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/291dfe3e477e010/output.txt

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Linux m4)


Failed

Full output at http://54.241.84.105:8877/15f1b6e27396d9c/output.txt

Total script time: 32.12 mins

  • Unit tests: Passed
  • Integration Tests: Passed
  • Regression tests: FAILED
  errors: 18
  different ref/snapshot: 17

Image differences available at: http://54.241.84.105:8877/15f1b6e27396d9c/reftest-analyzer.html#web=eq.log

@moz-tools-bot
Copy link
Collaborator

From: Bot.io (Windows)


Failed

Full output at http://54.193.163.58:8877/291dfe3e477e010/output.txt

Total script time: 46.44 mins

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

@timvandermeij timvandermeij merged commit e930f30 into mozilla:master Nov 2, 2024
10 checks passed
@timvandermeij
Copy link
Contributor

Thanks!

@Snuffleupagus Snuffleupagus deleted the getPdfManager-inline-flushChunks branch November 2, 2024 20:55
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