[Reporting] Optimize images buffers handling on PDF generation #136537
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dokmic
added
review
(Deprecated) Feature:Reporting
|
Pinging @elastic/kibana-app-services (Team:AppServicesUx) |
jloleysens
approved these changes
Jul 20, 2022
There was a problem hiding this comment.
@dokmic these changes look great, good job digging into the pdfkit library - hopefully a new release will come out soon!
Just curious, what magnitude of memory reduction where you seeing?
Otherwise happy for this to be merged.
x-pack/plugins/screenshotting/server/formats/pdf/pdf_maker/worker.ts
Outdated
Show resolved
Hide resolved
| content, | ||
| content: _.cloneDeepWith(content, (value) => | ||
| // The `pdfkit` library used underneath is using `Buffer.from(new Uint8Array(src))` construction to cast the image source. | ||
| // According to the Node.js docs, it will create a copy of the source `ArrayBuffer` which should be avoided. |
@jloleysens It's hard to get a relative value out of the measurements I took because all the values vary. But I saw a decrease in numbers for the |
💚 Build Succeeded
Metrics [docs]
History
To update your PR or re-run it, just comment with: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Resolves #129719.
The original idea was to reuse the virtual file system to pass forward the image buffers to the
pdfmakelibrary. But it's only supported in version 0.3, which is still in the beta.After inspecting the version 0.2 source code and the underlying
pdfkitlibrary, there was discovered a more elegant way to achieve the same result. The thing is that thepdfmakedoesn't perform any transformations of the image source and just forwards that down to thepdfkit. The trick was to convert the transferredArrayBufferto aBufferinstance to avoid inefficient instantiation on thepdfkitside.After running some local tests against sample data and measuring memory consumption using
process.memoryUsage, there was some decrease in memory usage.For maintainers