Skip to content

Conversation

@rojepp
Copy link
Contributor

@rojepp rojepp commented Dec 30, 2016

Timing for 100k checks of the path "c:\dev\myproject\utilities\longstring with spaces\filename.fs":
Original ~250ms
This function ~100ms

I don't expect this to be noticeable in the compiler but it might be in IDEs.

Uses a mutable HashSet but doesn't let it escape the function.

Timing for 100k checks of the path "c:\dev\myproject\utilities\longstring with spaces\filename.fs":
Original ~250ms
This method ~100ms
@jack-pappas
Copy link
Contributor

Out of curiosity -- would you mind benchmarking the original function again, but modifying it to iterate over path using the string enumerator (via for c in path do)? I'm wondering if some part of the speedup you're seeing is due to using the string enumerator instead of accessing the path characters via explicit indexing.

@rojepp
Copy link
Contributor Author

rojepp commented Dec 30, 2016

Thanks for the suggestion. I tried it, and it didn't help. The improvement comes from HashSet.
For reference, I took the bunch of things I tried and added it to a gist, here: https://gist.github.com/rojepp/6d5cfbbc9b1235645df895ab9c51ee7b
Your suggestion is checkPathForIllegalChars8

Copy link
Contributor

@KevinRansom KevinRansom left a comment

Choose a reason for hiding this comment

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

Looks good.

@KevinRansom KevinRansom merged commit 1d5962e into dotnet:master Dec 31, 2016
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.

4 participants