Skip to content

Added a trend line switch "-s"#269

Merged
fabianp merged 2 commits intopythonprofilers:masterfrom
tonivega:feature/trendline
Mar 13, 2020
Merged

Added a trend line switch "-s"#269
fabianp merged 2 commits intopythonprofilers:masterfrom
tonivega:feature/trendline

Conversation

@tonivega
Copy link
Copy Markdown
Contributor

I've added an experimental switch "-s" in order to plot illustrative trend lines and the actual trend slope in the labels.

I've found it useful for myself and I'm sharing it.

Intended usage : After running the profiling over a significant time period, if the slope reads >0 it might mean a memory leak.

Please just ping me if there is any issue, suggestion or comment.

Copy link
Copy Markdown
Collaborator

@fabianp fabianp left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution @tonivega ! Looks good for me except for the (very minor) linewidth in the plot

Comment thread mprof.py

if show_trend_slope:
# Plot the trend line
pl.plot(t, t*mem_trend[0] + mem_trend[1], "--", linewidth=0.5, color="#00e3d8")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

why so thin? It's barely visible in my screen. I would use linewidth=2

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've set it that thin because the line itself is "illustrative", having some visual feedback is nice but think that the actual value of this feature is the numeric slope in the label.
That said, I don't have any issue making it width 2

@fabianp
Copy link
Copy Markdown
Collaborator

fabianp commented Mar 12, 2020

Another issue is that it doesn't seem that images/trend_slope.png is used (or I couldn't find where it is used)

@tonivega
Copy link
Copy Markdown
Contributor Author

tonivega commented Mar 12, 2020

Another issue is that it doesn't seem that images/trend_slope.png is used (or I couldn't find where it is used)

It was meant for being included in the README documentation, but I've seen that the other images use an absolute path and I've finally left it out (I have to learn how to do this properly in github, any clues are welcome).

@fabianp
Copy link
Copy Markdown
Collaborator

fabianp commented Mar 13, 2020

merging, please send another pull request to make use of the image

@fabianp fabianp merged commit bc08f4d into pythonprofilers:master Mar 13, 2020
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.

2 participants