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

Cache repeated subtables while building COLR #3215

Closed
wants to merge 3 commits into from

Conversation

simoncozens
Copy link
Collaborator

When building a complex COLR table, particularly for a font with a large number of glyphs, there can be many repeated subtables. Instead of rebuilding each subtable from scratch, a simple cache can be used to speed up the build.

For example, building Bitcount without the cache takes 274 seconds on my computer; with the cache, it takes 94.

@simoncozens
Copy link
Collaborator Author

...but this only works when you're sending a plain JSON-style object to buildPaint, not a mix of dictionaries and pre-compiled subtables...

@anthrotype
Copy link
Member

hm yeah, the API supports a mix of raw dicts and prebuilt subtables and I'd like to keep that way. Also I'm a bit meh on having KBs of JSON for the cache dict keys..

@anthrotype
Copy link
Member

anthrotype commented Jul 21, 2023

I'm worried about the potential risk of collisions when using hash instead of the strings themselves as the key. This SO suggests using more cryptographically robust hashlib (also in stdlib).
Also I think pickle is faster than json in general.

EDIT: forgot to paste the link: https://stackoverflow.com/a/68048420

@anthrotype
Copy link
Member

also how about we make this caching optional by adding a parameter to the buildCOLR table that trickles down to the TableBuilder constructor? Maybe for your particular use case this may make things faster, but for the general case I am not sure whether the cost of pickling and caching outweighs the benefits

@simoncozens
Copy link
Collaborator Author

Closing in favour of #3221

@anthrotype
Copy link
Member

Closing in favour of #3221

my PR was based off yours, the idea was if you liked that suggestion you could merge that first into your PR branch and proceed from there. But since you closed this one I have rebased my PR onto main. Feel free to pick that one up

@khaledhosny khaledhosny deleted the cache-colr-builds branch November 5, 2023 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants