Profiler API Enhancements #15132

Zha0q1 · 2019-06-03T06:26:09Z

Description

This is the work in progress of the changes discussed in #15069. In stead of the original design which is to add a new api, get_summary(), to return the aggregate stats in JSON, I added a few parameters to the existing dumps().

Now dumps() (MXAggregateProfileStatsPrint) supports new parameters: format which controls the return string format, sort_by which specifies by which stat should the entries be sorted, and ascending which specify whether to sort ascendingly.

New api reset() is removed from the original proposal.

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

The PR title starts with [MXNET-$JIRA_ID], where $JIRA_ID refers to the relevant JIRA issue created (except PRs with tiny changes)
Changes are complete (i.e. I finished coding on this PR)
All changes have test coverage:
Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
Code is well-documented:
For user-facing API changes, API doc string has been updated.
For new C++ functions in header files, their functionalities and arguments are documented.
For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

adding parameters to dumps()

include/mxnet/c_api.h

piyushghai · 2019-06-04T20:07:56Z

Thanks for your contributions @Zha0q1 .
@mxnet-label-bot Add [Profiler, pr-work-in-progress]

sandeep-krishnamurthy

Thanks. nice start.
Test cases?

include/mxnet/c_api.h

python/mxnet/profiler.py

src/c_api/c_api_profile.cc

src/profiler/aggregate_stats.cc

sandeep-krishnamurthy · 2019-06-11T20:55:51Z

@access2rohit - Can you please help review this PR?

access2rohit · 2019-06-11T22:56:53Z

sure! Zhaoqi has fixed the failing tests now

access2rohit · 2019-06-12T08:00:34Z

Can you fix the failing test ?
ERROR: Failure: SyntaxError (invalid syntax (test_profiler.py, line 256))

Zha0q1 · 2019-06-12T08:04:34Z

Can you fix the failing test ?
ERROR: Failure: SyntaxError (invalid syntax (test_profiler.py, line 256))

It worked on my computer. Maybe online tests are using a different python version? I will fix this tomorrow.

tests/python/unittest/test_profiler.py

sandeep-krishnamurthy

Thanks. Last few minor comments.
As discussed offline, you are planning to update MXNet profiler tutorial in a new PR?

include/mxnet/c_api.h

python/mxnet/profiler.py

src/profiler/aggregate_stats.cc

src/profiler/aggregate_stats.h

tests/python/unittest/test_profiler.py

Zha0q1 · 2019-06-13T18:19:43Z

Thanks. Last few minor comments.
As discussed offline, you are planning to update MXNet profiler tutorial in a new PR?

Yeah, that I will do. Thanks for the great advices!

src/c_api/c_api_profile.cc

sandeep-krishnamurthy

One minor spelling issue.

include/mxnet/c_api.h

anirudh2290 · 2019-06-14T18:29:03Z

tests/python/unittest/test_profiler.py

+    debug_str = profiler.dumps(format = 'json')
+    assert(len(debug_str) > 0)
+    target_dict = json.loads(debug_str)
+    print(target_dict)


remove print

* add support for sorting and printing aggregate info in json * revert some overwrites * revert to old c apis; added MXAggregateProfileStatsPrintEx instead * style fix * style fixes * more style fixes * rebase * style fixes * fix infinite loop bug * test cases added and bugs fixed * fix test cases * fix testcases * testcases * testcases * testcases * fix doc * use enum to avoid hardcoding * sanity test fix * sanity test fix * add parameter validation * add parameter validation in frontend * validation * removing print() * fix typos

eric-haibin-lin · 2019-08-16T18:07:59Z

The ascending argument is great, but why is it missing the most important sort criteria "total time"? That's usually THE first thing people look at - what operator takes most of the time in a network. This is definitely a must-have for the 'sort-by' argument ..

Zha0q1 · 2019-08-16T18:14:56Z

The ascending argument is great, but why is it missing the most important sort criteria "total time"? That's usually THE first thing people look at - what operator takes most of the time in a network. This is definitely a must-have for the 'sort-by' argument ..

Sure. I will create a pr and add "total_time" soon.

Zha0q1 · 2019-08-17T01:56:49Z

The ascending argument is great, but why is it missing the most important sort criteria "total time"? That's usually THE first thing people look at - what operator takes most of the time in a network. This is definitely a must-have for the 'sort-by' argument ..

Sure. I will create a pr and add "total_time" soon.

will do this at my earliest availability after I come back from my trip. Now I don't have access to my laptop (back to US on aug 27, too many family events this week

Zha0q1 requested review from anirudh2290, eric-haibin-lin and szha as code owners June 3, 2019 06:26

anirudh2290 suggested changes Jun 3, 2019

View reviewed changes

include/mxnet/c_api.h Outdated Show resolved Hide resolved

marcoabreu added pr-work-in-progress PR is still work in progress Profiler MXNet profiling issues labels Jun 4, 2019

sandeep-krishnamurthy suggested changes Jun 7, 2019

View reviewed changes

Zha0q1 added 8 commits June 7, 2019 12:58

add support for sorting and printing aggregate info in json

ca00a64

revert some overwrites

b4b89b8

revert to old c apis; added MXAggregateProfileStatsPrintEx instead

005663c

style fix

5db64d8

style fixes

fd06eff

more style fixes

5915ee7

rebase

94d75da

style fixes

03e9d36

sandeep-krishnamurthy closed this Jun 7, 2019

sandeep-krishnamurthy reopened this Jun 7, 2019

fix infinite loop bug

4a8349a

test cases added and bugs fixed

8e8637a

fix test cases

542c269

access2rohit reviewed Jun 12, 2019

View reviewed changes

tests/python/unittest/test_profiler.py Outdated Show resolved Hide resolved

access2rohit reviewed Jun 12, 2019

View reviewed changes

tests/python/unittest/test_profiler.py Outdated Show resolved Hide resolved

access2rohit reviewed Jun 12, 2019

View reviewed changes

tests/python/unittest/test_profiler.py Outdated Show resolved Hide resolved

Zha0q1 added 2 commits June 12, 2019 11:05

fix testcases

712ef13

testcases

39aaf32

sandeep-krishnamurthy suggested changes Jun 13, 2019

View reviewed changes

fix doc

5fb0530

Zha0q1 added 4 commits June 13, 2019 11:54

use enum to avoid hardcoding

e0c1921

sanity test fix

8f2679e

sanity test fix

dd7d00d

add parameter validation

baf3aa2

sandeep-krishnamurthy reviewed Jun 13, 2019

View reviewed changes

src/c_api/c_api_profile.cc Outdated Show resolved Hide resolved

Zha0q1 added 2 commits June 13, 2019 16:08

add parameter validation in frontend

104ec1d

validation

23152a5

sandeep-krishnamurthy approved these changes Jun 14, 2019

View reviewed changes

Zha0q1 closed this Jun 14, 2019

Zha0q1 reopened this Jun 14, 2019

Zha0q1 closed this Jun 14, 2019

Zha0q1 reopened this Jun 14, 2019

anirudh2290 reviewed Jun 14, 2019

View reviewed changes

removing print()

8e494e5

anirudh2290 approved these changes Jun 14, 2019

View reviewed changes

fix typos

384768d

Zha0q1 closed this Jun 14, 2019

Zha0q1 reopened this Jun 14, 2019

Zha0q1 mentioned this pull request Jun 14, 2019

Fixing duplication in operator profiling #15240

Merged

5 tasks

sandeep-krishnamurthy merged commit 13cf5db into apache:master Jun 15, 2019

This was referenced Jul 24, 2019

More user-friendly profiler result #14100

Closed

Profiler RFC: Introducing new APIs #15069

Closed

Zha0q1 mentioned this pull request Aug 31, 2019

adding "total" (total time) to profiler aggregate stats sorting criteria #16055

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Profiler API Enhancements #15132

Profiler API Enhancements #15132

Zha0q1 commented Jun 3, 2019 •

edited

piyushghai commented Jun 4, 2019

sandeep-krishnamurthy left a comment

sandeep-krishnamurthy commented Jun 11, 2019

access2rohit commented Jun 11, 2019

access2rohit commented Jun 12, 2019

Zha0q1 commented Jun 12, 2019

sandeep-krishnamurthy left a comment

Zha0q1 commented Jun 13, 2019 •

edited

sandeep-krishnamurthy left a comment

anirudh2290 Jun 14, 2019

eric-haibin-lin commented Aug 16, 2019

Zha0q1 commented Aug 16, 2019

Zha0q1 commented Aug 17, 2019 •

edited

Profiler API Enhancements #15132

Profiler API Enhancements #15132

Conversation

Zha0q1 commented Jun 3, 2019 • edited

Description

Checklist

Essentials

Changes

piyushghai commented Jun 4, 2019

sandeep-krishnamurthy left a comment

Choose a reason for hiding this comment

sandeep-krishnamurthy commented Jun 11, 2019

access2rohit commented Jun 11, 2019

access2rohit commented Jun 12, 2019

Zha0q1 commented Jun 12, 2019

sandeep-krishnamurthy left a comment

Choose a reason for hiding this comment

Zha0q1 commented Jun 13, 2019 • edited

sandeep-krishnamurthy left a comment

Choose a reason for hiding this comment

anirudh2290 Jun 14, 2019

Choose a reason for hiding this comment

eric-haibin-lin commented Aug 16, 2019

Zha0q1 commented Aug 16, 2019

Zha0q1 commented Aug 17, 2019 • edited

Zha0q1 commented Jun 3, 2019 •

edited

Zha0q1 commented Jun 13, 2019 •

edited

Zha0q1 commented Aug 17, 2019 •

edited