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

Enable Python profiling for StreamingPythonScriptExecutor. #4953

Merged
merged 1 commit into from
Aug 27, 2018

Conversation

cmnbroad
Copy link
Collaborator

Fixes #4766. We might want to do a similar thing for the non-streaming executor, in a separate PR.

@cmnbroad cmnbroad force-pushed the cn_python_profiler branch from e16c9e6 to 00cb1a2 Compare June 27, 2018 14:39
@codecov-io
Copy link

codecov-io commented Jun 27, 2018

Codecov Report

Merging #4953 into master will increase coverage by 0.001%.
The diff coverage is 93.333%.

@@               Coverage Diff               @@
##              master     #4953       +/-   ##
===============================================
+ Coverage     86.387%   86.388%   +0.001%     
- Complexity     28846     28849        +3     
===============================================
  Files           1791      1791               
  Lines         133626    133653       +27     
  Branches       14921     14925        +4     
===============================================
+ Hits          115435    115460       +25     
  Misses         12793     12793               
- Partials        5398      5400        +2
Impacted Files Coverage Δ Complexity Δ
...er/utils/python/StreamingPythonScriptExecutor.java 86.957% <100%> (+1.38%) 22 <5> (+2) ⬆️
...ellbender/tools/walkers/vqsr/CNNScoreVariants.java 74.757% <100%> (ø) 40 <0> (ø) ⬇️
.../python/StreamingPythonScriptExecutorUnitTest.java 86.4% <88.235%> (+0.161%) 16 <1> (+1) ⬆️

@droazen
Copy link
Contributor

droazen commented Aug 24, 2018

@jonn-smith and @lucidtronix please review when you get a chance. Thanks!

Copy link
Collaborator

@jonn-smith jonn-smith left a comment

Choose a reason for hiding this comment

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

This seems pretty good to me.

I haven't done a lot of python profiling - do we want to make the profiling have some options? If so, it can be a separate PR.

@cmnbroad
Copy link
Collaborator Author

cmnbroad commented Aug 27, 2018

@jonn-smith Yes, I think if this turns out to be useful, we may want to add more options, i.e. which stats to collect, but I think we'll have to use it a bit to figure out which ones.

@cmnbroad cmnbroad merged commit 3f1d75d into master Aug 27, 2018
@cmnbroad cmnbroad deleted the cn_python_profiler branch August 27, 2018 17:35
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.

5 participants