-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Remove extra lookups and memory allocations from tsort graph construction #8694
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
Conversation
did you look at the actual perf wins ? |
GNU testsuite comparison:
|
a job is failing, you need to do |
It optimizes only initial input processing, so any gain can be visible only on large files. Where the initial read is somehow noticeable With 1 million edges linear chain (edges shuffled)
After
Actually, Itertools are not needed. And it's even better without them -- internal chunks' buffer overuses RefCell runtime checks No itertools
tsort (GNU coreutils) 9.4 -- (have't checked latest one)
|
please run hyperfine this way: |
CodSpeed Performance ReportMerging #8694 will not alter performanceComparing Summary
Footnotes
|
|
well done! |
GNU testsuite comparison:
|
_ => return Err(TsortError::NumTokensOdd(input.to_string_lossy().to_string()).into()), | ||
} | ||
|
||
let mut edge_tokens = data.split_whitespace(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe document this a bit more to explain what it is doing ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expanded a bit
GNU testsuite comparison:
|
GNU testsuite comparison:
|
GNU testsuite comparison:
|
GNU testsuite comparison:
|
GNU testsuite comparison:
|
Hi.
I was benchmarking several tools and found there some low hanging minor optimizations for input processing that can be done: