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

Add support for differential flame graphs #60

Merged
merged 12 commits into from
Feb 25, 2019

Conversation

jasonrhansen
Copy link
Collaborator

Implements #25

@jonhoo
Copy link
Owner

jonhoo commented Feb 25, 2019

Looks like clippy is complaining.

@jasonrhansen
Copy link
Collaborator Author

The first clippy fix is simple, but it's complaining about the complexity of from_sorted_lines. How do you want to break that up?

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

jonhoo commented Feb 25, 2019

Re from_sorted_lines, ultimately I want the code that draws each frame to be in its own function, but #52 already makes some headroom we can build on there. For the time being, just add

#[allow(clippy::cyclomatic_complexity)]

@jonhoo
Copy link
Owner

jonhoo commented Feb 25, 2019

Also, awesome work! ❤️

@codecov-io
Copy link

codecov-io commented Feb 25, 2019

Codecov Report

Merging #60 into master will increase coverage by 1.6%.
The diff coverage is 85.34%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master      #60     +/-   ##
=========================================
+ Coverage   75.78%   77.38%   +1.6%     
=========================================
  Files          14       15      +1     
  Lines        1210     1287     +77     
=========================================
+ Hits          917      996     +79     
+ Misses        293      291      -2
Impacted Files Coverage Δ
src/bin/flamegraph.rs 0% <0%> (ø) ⬆️
src/flamegraph/color/mod.rs 73.33% <100%> (+1.67%) ⬆️
src/flamegraph/merge.rs 91.66% <100%> (+6.81%) ⬆️
src/flamegraph/mod.rs 84.34% <100%> (+1.73%) ⬆️
tests/flamegraph-differential.rs 90% <90%> (ø)

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 ff4ae74...b6092f7. Read the comment docs.

src/flamegraph/merge.rs Outdated Show resolved Hide resolved
src/flamegraph/merge.rs Outdated Show resolved Hide resolved
@jasonrhansen jasonrhansen merged commit 65587d8 into jonhoo:master Feb 25, 2019
jonhoo added a commit that referenced this pull request Feb 25, 2019
@jonhoo jonhoo mentioned this pull request Feb 25, 2019
@jasonrhansen jasonrhansen mentioned this pull request Feb 25, 2019
@jasonrhansen jasonrhansen deleted the differentials branch March 2, 2019 22:16
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