Consolidate pre- and post-clang-format Python steps to improve performance #109
Conversation
jamieQ
left a comment
jamieQ
left a comment
There was a problem hiding this comment.
looks reasonable to my (naive) eye! sort of a shocking performance improvement – i guess process spawning isn't free?
| if len(self.formatters) == 0: | ||
| return "".join(lines) |
There was a problem hiding this comment.
nit: should this just be an error instead of ignoring it? seems like there should be at least one formatter if using this type
There was a problem hiding this comment.
Yeah good point, but I don't think we have a good way to surface an error here. What we output will get written to the temporary file set up by the script. Perhaps we could exit with a nonzero exit code but I don't know if the script handles that well.
| /usr/bin/python3 "$DIR"/custom/LiteralSymbolSpacer.py | | ||
| /usr/bin/python3 "$DIR"/custom/InlineConstructorOnSingleLine.py | | ||
| /usr/bin/python3 "$DIR"/custom/MacroSemicolonAppender.py | | ||
| /usr/bin/python3 "$DIR"/custom/DoubleNewlineInserter.py | |
There was a problem hiding this comment.
for my own education – does each | spawn a new process?
There was a problem hiding this comment.
Yes, and since it's invoking python that means spinning up a python interpreter each time
|
Thank you for this! Our repo is significantly smaller, but it took a full reformat from 72 to 29 seconds for us. |
|
@ksuther Thanks so much for testing and sharing! Wasn't expecting anyone external to Square to see this so quickly 😅 |
Consolidate the various Python formatter commands into a single invocation for changes pre
clang-formatand postclang-formatto reduce extra overhead involved in spinning up a new process for each step.This makes a significant difference on a large internal monorepo with over 12k Objective-C files, reducing the time of
format-objc-files-in-repo.shfrom 10m53s to 6m14s (42% reduction)