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

[ufoLib] userNameToFileName very slow for large existing lists #2421

Closed
madig opened this issue Oct 6, 2021 · 1 comment · Fixed by #2422
Closed

[ufoLib] userNameToFileName very slow for large existing lists #2421

madig opened this issue Oct 6, 2021 · 1 comment · Fixed by #2422

Comments

@madig
Copy link
Contributor

madig commented Oct 6, 2021

Profiling https://github.com/madig/noto-amalgamated/blob/main/amalgamate-noto.py shows that ~2 mins of ~10 mins are spent in userNameToFileName, specifically

if fullName.lower() in existing:

since existing is derived from self._existingFileNames.values() in

fileName = self.glyphNameToFileName(glyphName, self._existingFileNames.values())
.

writeGlyph is called per-glyph, I don't see an easy way to fix this. Changing the call to set(self._existingFileNames.values()) only makes it slower because now you make a set every time. And I'm not sure the list is guaranteed to be ordered?

@justvanrossum
Copy link
Collaborator

Good catch.

I think GlyphSet._existingFileNames needs to become a set. The only time its keys are used is in GlyphSet.deleteGlyph, and unless I'm missing something, I think we can write self._existingFileNames.remove(self.contents[glyphName]) there, instead of the current del self._existingFileNames[fileName].

(In other words, I think it's userNameToFileName()'s caller's responsibility to pass a set-like thing for the existing argument.)

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 a pull request may close this issue.

2 participants