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

[aat] Consume glyph insertion from buffer's max_ops #2223

Merged
merged 1 commit into from
Jul 14, 2020

Conversation

ebraminio
Copy link
Collaborator

@ebraminio ebraminio commented Mar 3, 2020

Glyph insertion is an expensive operation and we like to have it limited
based on buffer's input size which is handled by buffer's max_ops.

clusterfuzz-testcase-minimized-harfbuzz_fuzzer-5754958982021120:

Before the change: 0.67s user 0.00s system 99% cpu 0.674 total
After the change: 0.02s user 0.00s system 98% cpu 0.024 total

Which takes much longer on valgrind and tsan bots.

What makes this expensive is specially the fact multiple insertion making lots of glyphs and then the happened rearrangement becomes much expensive.

Glyph insertion is an expensive operation and we like to have it limited
based on buffer's input size which is handled by buffer's max_ops.

clusterfuzz-testcase-minimized-harfbuzz_fuzzer-5754958982021120:

Before the change: 0.67s user 0.00s system 99% cpu 0.674 total
 After the change: 0.02s user 0.00s system 98% cpu 0.024 total

Which takes much longer on valgrind and tsan bots.
@ebraminio ebraminio requested a review from behdad March 3, 2020 18:04
@khaledhosny khaledhosny added the aat Apple Advanced Typography label Mar 8, 2020
@ebraminio
Copy link
Collaborator Author

@jfkthame can you have a look please?

@behdad behdad merged commit 11d583a into harfbuzz:master Jul 14, 2020
@ebraminio ebraminio deleted the insert-limit branch July 14, 2020 07:02
@jfkthame
Copy link
Collaborator

@jfkthame can you have a look please?

Yeah, seems fine - I see behdad already merged it. :)

@ebraminio
Copy link
Collaborator Author

Thank you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aat Apple Advanced Typography
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants