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

Improve performance by avoiding overhead of func + #58

Closed
wants to merge 1 commit into from

Conversation

kastiglione
Copy link

@kastiglione kastiglione commented Oct 8, 2018

This change improves the performance of PathKit by avoiding unnecessary cpu and memory costs in func +.

For the project I am testing on, which uses XcodeGen among others, the cpu and memory costs of func + are reduced fairly significantly. Memory use is down from ~15mb to 1.5mb, and these values are consistent across Instruments runes. Here are some before and after screenshots:

Memory Use Before

screen shot 2018-10-08 at 12 38 09 pm

Memory Use After

screen shot 2018-10-08 at 12 38 21 pm

For CPU, the times vary a little across Instruments runs, but the performance is also improved by an order of magnitude, from ~300ms to ~40ms. Here are some before and after screenshots:

CPU Use Before

screen shot 2018-10-08 at 1 04 11 pm

CPU Use After

screen shot 2018-10-08 at 1 04 20 pm

Explanation

func + runs in O(N) time and space, where N is the number of elements in the path. This means it performs worse for longer paths than shorter paths, which is not an ideal performance trait. For both path arguments, it creates an array of path components, then filters out "." elements, which is often just a copy, resolves some (but not all) .. references, then joins the results into a new string. For any string with no .. or . elements, this is all completely unnecessary. This change adds the concept of direct paths, paths with no .. or .. Now, func + can avoid all the unnecessary work when the paths are direct.

@@ -563,7 +581,7 @@ extension Path {
///
public func children() throws -> [Path] {
return try Path.fileManager.contentsOfDirectory(atPath: path).map {
self + Path($0)
self + Path(direct: $0)
Copy link
Author

@kastiglione kastiglione Oct 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note this + would previously do the same redundant work self (splitting, filtering), for every child, when it should only be done most at once. It's also known that $0 will never contain .. or .. These facts are used to improve performance.

jerrymarino added a commit to bazel-xcode/xchammer that referenced this pull request Oct 9, 2018
jerrymarino added a commit to bazel-xcode/xchammer that referenced this pull request Oct 9, 2018
@kastiglione
Copy link
Author

Update: I just did a handful of timing runs with XCHammer (which uses XcodeGen and xcodeproj, all of which use PathKit)

The primary hotspots I've seen are, func +, Path.children, and Path.parent. The latter two call func +, and so their performance is improved along with func +.

As measured by Instruments, here are the rough before and after numbers with this change, using our XCHammer workload:

func +: ~600ms reduced to ~90ms
Path.children: ~500ms reduced to ~300ms
Path.parent: ~200ms reduced to ~10ms

In total, func + was reduced from ~17% of cpu time spent to ~8% of cpu time. To give context, these runs take around 8s, some of which which is spent doing IO.

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.

1 participant