Skip to content
This repository has been archived by the owner on May 20, 2022. It is now read-only.

fontcrunch: Replace Rc<Option> with Option<Rc> #36

Merged
merged 1 commit into from Jun 10, 2021

Conversation

cmyr
Copy link
Collaborator

@cmyr cmyr commented Jun 10, 2021

This lets us avoid an allocation in the None case.
Profiling allocations on a sample font, this cuts the
total number of allocations by ~60%.

This lets us avoid an allocation in the None case.
Profiling allocations on a sample font, this cuts the
total number of allocations by ~60%.
@cmyr
Copy link
Collaborator Author

cmyr commented Jun 10, 2021

I don't have a good sense of how much this helps with runtime performance; I've timed it a bunch and the data is pretty noisy, but it doesn't seem huge? It shouldn't really hurt, though.

@simoncozens
Copy link
Owner

Nice, thank you!

@simoncozens simoncozens merged commit 39b1f64 into simoncozens:main Jun 10, 2021
@cmyr cmyr deleted the fontcrunch-less-alloc branch June 10, 2021 14:58
@cmyr
Copy link
Collaborator Author

cmyr commented Jun 10, 2021

I'm also going to dig into actually getting rid of the linked list, will let you know how that goes.

@simoncozens
Copy link
Owner

I ran flamegraph and the real hotspot is the numerical integration stuff - arclength and error measuring. So much so that adding “#[inline]” to the functors gained a 10% improvement.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants