Optimize simplifying paths for faster project generation #857
Conversation
| @@ -11,7 +11,14 @@ extension Path { | |||
| /// - `../a/b` simplifies to `../a/b` | |||
| /// - `a/../../c` simplifies to `../c` | |||
| public func simplifyingParentDirectoryReferences() -> Path { | |||
| normalize().components.reduce(Path(), +) | |||
| if !string.contains("..") { // Skip simplifying if its already simple | |||
There was a problem hiding this comment.
Do we need to look for ../ instead?
There was a problem hiding this comment.
Sorry for late response. Wouldn't it be like if !(string.contains("/..") || string.contains("../")) if we want it to match real parent references only?
I preferred to keep it simple than perfect because false positives like ./file..name/ or ... are edge cases and won't be big problems IMO, while false negatives can break things.
I guess we could possibly rewrite the complete normalization by ourselves to get both better performance and correctness, as a future task maybe.
|
This makes our project generation 1-2 seconds faster (~9%). |
There was a problem hiding this comment.
I think this is good as is (but would welcome a rewrite of the normalization as a future PR).
Thanks @akkyie!
|
Thanks! I'll find time to give it a try 👍 |
We are using XcodeGen with a ~3000 files project, and the project generation for it takes about 3s.
I wanted to know what's taking time, and Instruments told me that the time of
Path.simplifyingParentDirectoryReferencescalls reaches almost 30% of the whole.I also found that repeated invocation of
Path.+is time-consuming as it causes String copying and conversion between NSString.This looks kind of overkill to me, because most of the paths in our project don't include parent path reference nor have trailing slashes. So, this change avoids
Path.+when the path does not include..in its middle, while manually removing trailing slashes with a simpler way instead.It cuts off our project generation time down to about 2/3 (2s). I think it's not common case where many paths in a project needs the actual simplification, but the overhead of this change for these cases would be ignorable.
I didn't completely rewrite the whole function to minimize the possibility of regression for special cases. I want to know if even this fast-path has any patterns to cause regression.