Eliminate memory copy when reading font data #6254
Merged
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
ghost
added
the
ghost
requested review from
|
Performance data obtained by running the test app, maximising on a 1600x1200 screen, then scrolling the textbox all the way to the bottom. Before
After
Comparison
|
|
This PR is being considered for next community test pass. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fixes #6236
Description
Using a
<Resource>font in WPF currently causes a lot of allocation, GC pinning, and memory copying. This is entirely avoidable by returning a pointer to the already-loaded font data to DWrite.Customer Impact
Without this fix, customers are forced to hook DWrite APIs and supply their own font-loading implementation; e.g., https://faithlife.codes/blog/2019/06/improving-wpf-text-display-performance/.
Regression
Not a regression.
Testing
Manual testing using a test app: https://github.com/bgrainger/EmbeddedFontPerformance
Risk
Risk is low. This fix has been prototyped in a commercial WPF application that's been shipping for 5 years (see blog post above). Since the font source is an
UnmanagedMemoryStream, we can rely on its data being present in memory at least as long as we hold a reference to the stream. (And since, practically speaking, the font data is backed by the assembly that contains it being mmap'ed into the process' address space, it's going to be safe to dereference it for the lifetime of the application.)