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

Port difffolded.pl as inferno-diff-folded #96

Merged
merged 13 commits into from
Mar 16, 2019

Conversation

jasonrhansen
Copy link
Collaborator

Fixes #62

I also changed flamegraph to always sort input lines since the output of diff-folded isn't sorted. This matches the behavior of upstream flamegraph.

@codecov-io
Copy link

codecov-io commented Mar 15, 2019

Codecov Report

Merging #96 into master will increase coverage by 0.54%.
The diff coverage is 98.7%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #96      +/-   ##
==========================================
+ Coverage   88.48%   89.02%   +0.54%     
==========================================
  Files          11       12       +1     
  Lines        1294     1367      +73     
==========================================
+ Hits         1145     1217      +72     
- Misses        149      150       +1
Impacted Files Coverage Δ
src/flamegraph/mod.rs 95.86% <100%> (+0.07%) ⬆️
src/flamegraph/merge.rs 94.94% <100%> (+0.32%) ⬆️
src/differential/mod.rs 98.38% <98.38%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb52b9e...d352b48. Read the comment docs.

src/lib.rs Outdated Show resolved Hide resolved
src/lib.rs Outdated Show resolved Hide resolved
@jasonrhansen
Copy link
Collaborator Author

I'm bothered enough about the sorting this PR adds to flamegraph that I think we should provide a way to opt out if you know the input is already sorted. We could make compatibility with upstream the default, but have a flag like --no-sort which is basically a promise that the input is already sorted. What do think?

src/diff_folded/mod.rs Outdated Show resolved Hide resolved
src/diff_folded/mod.rs Outdated Show resolved Hide resolved
src/diff_folded/mod.rs Outdated Show resolved Hide resolved
src/diff_folded/mod.rs Outdated Show resolved Hide resolved
src/diff_folded/mod.rs Outdated Show resolved Hide resolved
src/diff_folded/mod.rs Outdated Show resolved Hide resolved
@jonhoo
Copy link
Owner

jonhoo commented Mar 15, 2019

@jasonrhansen I think something like --no-sort is a great idea. We may then want to warn if we detect unsorted input though. That can be an entirely local check. Just make sure that each line lexiographically follows the previous one. Maybe even do it really stupidly by only checking the first character.

@jasonrhansen
Copy link
Collaborator Author

I like the idea of detecting and warning about unsorted input when using --no-sort.

tests/flamegraph.rs Outdated Show resolved Hide resolved
src/flamegraph/merge.rs Outdated Show resolved Hide resolved
src/differential/mod.rs Outdated Show resolved Hide resolved
@jasonrhansen jasonrhansen merged commit 755d05a into jonhoo:master Mar 16, 2019
@jasonrhansen jasonrhansen deleted the diff-folded branch March 16, 2019 16:30
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.

None yet

3 participants