graph: -f total-avg,self-avg,total-max,total-min,self-max,self-min in tui,cmds#1999
Conversation
e757783 to
34af737
Compare
namhyung
left a comment
There was a problem hiding this comment.
Please split the changes in graph and tui. I think it's better to add the feature to the graph command first. And the second commit adds tui support. Also please update the documentation for each command in the same commit. Finally you need to add a test case for graph command.
I tried to split commit specifically. And add description on Document on each cmds,tui. |
|
Can I get some information about test cmds code? I tried to test for graph field number "t187" by runtest.py but it printed like this. junyeong@Junyeongui-noteubug ~/maindir/uftrace master python3 tests/runtest.py tests/t187_graph_field.py
cannot find testcase for : tests/t187_graph_field.py |
namhyung
left a comment
There was a problem hiding this comment.
Please squash the documentation changes to related commits.
|
To run test, you can run the script with options and arguments. Please check I usually run it with the following options. The argument can be test number or name. Or you can run the same using |
f52d9aa to
9315882
Compare
I tried to create a comparison test. I modified the fields that use nr_calls to check against zero in tui and cmds. Also, I aligned the elements in graph_field according to the layout defined in field.h. |
29d5ef6 to
3477232
Compare
|
I believe I have addressed all the points from your review. Please kindly review it! |
namhyung
left a comment
There was a problem hiding this comment.
Overall, looks good to me, thanks! A few nitpicks in the commit messages.
- Format the commit message properly. Some line is too long, and some are unnecessarily indented.
- Move documentation changes into actual changes.
- Add an example output when you add new options or fields
Is this the way you intended the commits to be squashed? I’ve taken care of the other requested changes as well. |
namhyung
left a comment
There was a problem hiding this comment.
Thanks, I like the commits. But the last point is that you need to indent code or shell output in the commit message by 2 or more spaces so that it can be distinguished.
Due to total-min,total-max,self-min,self-max function add total,self element on uftrace graph_node Signed-off-by: Jun Yeong Kim <junyeonggim5@gmail.com>
This patch adds support for the following fields in the graph -f:
- total-avg, total-max, total-min
- self-avg, self-max, self-min
These fields show the average, maximum, and minimum time based on
each function call time.
The simple usage is as follows.
$ uftrace graph -f total-avg,total-max,total-min
========== FUNCTION CALL GRAPH ==========
SELF AVG SELF MAX SELF MIN FUNCTION
12.393 ms 12.393 ms 12.393 ms : (1) t-sort
123.375 us 123.375 us 123.375 us : +-(1) __monstartup
: |
0.375 us 0.375 us 0.375 us : +-(1) __cxa_atexit
: |
12.269 ms 12.269 ms 12.269 ms : +-(1) main
23.354 us 38.833 us 7.875 us : +-(2) foo
7.048 us 29.584 us 2.500 us : | (6) loop
: |
12.166 ms 12.166 ms 12.166 ms : +-(1) bar
12.143 ms 12.143 ms 12.143 ms : (1) usleep
$ uftrace graph -f self-avg,self-max,self-min
========== FUNCTION CALL GRAPH ==========
TOTAL AVG TOTAL MAX TOTAL MIN FUNCTION
: (1) t-sort
127.709 us 127.709 us 127.709 us : +-(1) __monstartup
: |
0.416 us 0.416 us 0.416 us : +-(1) __cxa_atexit
: |
54.417 us 54.417 us 54.417 us : +-(1) main
2.208 us 4.042 us 0.374 us : +-(2) foo
7.062 us 29.542 us 2.500 us : | (6) loop
: |
18.499 us 18.499 us 18.499 us : +-(1) bar
11.058 ms 11.058 ms 11.058 ms : (1) usleep
Fixed: namhyung#1451
Signed-off-by: Jun Yeong Kim <junyeonggim5@gmail.com>
This patch adds support for the following fields in the tui-graph: - total-avg, total-max, total-min - self-avg, self-max, self-min These fields show the average, maximum, and minimum time based on each function call time. In tui simply press 'f' and check the corresponding fields Fixed: namhyung#1451 Signed-off-by: Jun Yeong Kim <junyeonggim5@gmail.com>
To check new function for graph fields: - total-avg, total-max, total-min - self-avg, self-max, self-min Added tests for total,self field. Each test ensures that min <= avg <= max holds for every function called more than once. For functions that are executed only once, there is a test that checks whether their execution times are the same, excluding system initialization functions. $ make -j 8 test TESTARG=graph_field Running 0 test cases ====================== unit test stats ==================== 0 ran successfully 0 failed 0 skipped 0 signal caught 0 unknown result TEST test_all Start 2 tests with 2 worker Compiler gcc Runtime test case pg finstrument-fu fpatchable-fun ------------------------: O0 O1 O2 O3 Os O0 O1 O2 O3 Os O0 O1 O2 O3 Os 295 graph_field_total : OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK 296 graph_field_self : OK OK OK OK OK OK OK OK OK OK OK OK OK OK OK Signed-off-by: Jun Yeong Kim <junyeonggim5@gmail.com>
I apologize for the oversight. While I had addressed this issue in the cmds part, I unintentionally missed applying it in the tui part. |
|
@junyeong0619 I couldn't help you enough, but thanks for your work! |
I'm sorry to make new PR. I pushed force to my master branch. So the branch's history completely changed and cannot reopen old PR. Thanks for your advice and it really helped me.