record: make all info in text format #1435
Conversation
|
Hi @qpakzk, thanks very much for doing this. I will have a look when I come back from holiday. |
|
Thanks for looking into that. I've been thinking about the file format changes and I think we also need to change data formats for better parsing of arguments and longer than 48-bit address spaces. |
|
@qpakzk Thanks for waiting. I have tested your patch and have a few comments. First, we must support backward compatibility. This patch makes Second, |
@namhyung What do you think about this approach? |
qpakzk
force-pushed
the
feature/info-text-format
branch
3 times, most recently
from
b87d4e0 to
b22ffb7
Compare
|
Sorry for the late reply. I agree that we should provide backward compatibility to the old data. |
There was a problem hiding this comment.
Thanks for the update! It looks good and works fine in general cases, but I have left a few comments. I also see some strange failures in our tests. Especially WARN: creating directory failed: File exists looks not good.
$ cd tests
$ ./runtest.py -j1
Start 271 tests with 1 worker
Test case pg finstrument-fu
------------------------: O0 O1 O2 O3 Os O0 O1 O2 O3 Os
001 basic : OK OK OK OK OK OK OK OK OK OK
002 argument : OK OK OK OK OK OK OK OK OK OK
003 thread : OK OK OK OK OK OK OK OK OK OK
004 filter_F : OK OK OK OK OK OK OK OK OK OK
005 filter_N : OK OK OK OK OK OK OK OK OK OK
006 filter_FN : OK OK OK OK OK OK OK OK OK OK
007 library : OK OK OK OK OK OK OK OK OK OK
008 daemon : OK OK OK OK OK OK OK OK OK OK
009 fork : OK OK OK OK OK OK OK OK OK OK
010 forkexec : OK OK OK OK OK OK OK OK OK OK
WARN: creating directory failed: File exists
WARN: creating directory failed: File exists
WARN: creating directory failed: File exists
WARN: creating directory failed: File exists
WARN: creating directory failed: File exists
011 vforkexec : OK OK OK OK OK OK OK OK OK OK
WARN: creating directory failed: File exists
WARN: creating directory failed: File exists
WARN: creating directory failed: File exists
WARN: creating directory failed: File exists
012 demangle : OK OK OK OK OK OK OK OK OK OK
...
100 dump_depth : OK OK OK OK OK OK OK OK OK OK
101 dump_chrome : NG NG NG NG NG NG NG NG NG NG
102 dump_flamegraph : OK OK OK OK OK OK OK OK OK OK
103 dump_kernel : SK SK SK SK SK SK SK SK SK SK
104 graph_kernel : SK SK SK SK SK SK SK SK SK SK
105 replay_time : OK OK OK OK OK OK OK OK OK OK
106 report_time : OK OK OK OK OK OK OK OK OK OK
107 dump_time : NG NG NG NG NG NG NG NG NG NG
108 graph_time : OK OK OK OK OK OK OK OK OK OK
109 replay_time_A : NG NG NG NG NG SK SK SK SK SK
110 replay_time_T : OK OK OK OK OK OK OK OK OK OK
...
Please check the test failures. Some of them are known issues as I posted at #1436 (comment).
qpakzk
force-pushed
the
feature/info-text-format
branch
from
b22ffb7 to
70407f5
Compare
|
@honggyukim Thanks for your feedback. I updated my commit, but I'll check test failures soon. |
There was a problem hiding this comment.
Hi @qpakzk, thanks for the update.
I'll check test failures soon.
The failed test cases are as follows.
Test case pg finstrument-fu
------------------------: O0 O1 O2 O3 Os O0 O1 O2 O3 Os
087 arg_variadic : NZ OK OK NZ OK SK SK SK SK SK
101 dump_chrome : NG NG NG NG NG NG NG NG NG NG
107 dump_time : NG NG NG NG NG NG NG NG NG NG
109 replay_time_A : OK NG OK NG NG SK SK SK SK SK
117 time_range : OK OK OK NG OK OK OK OK NG OK
125 report_range : OK OK OK OK OK OK OK OK NG OK
135 trigger_time2 : OK OK OK OK OK OK NG OK OK OK
141 recv_basic : NZ NZ NZ NZ NZ NZ NZ NZ NZ NZ
142 recv_multi : NZ NZ NZ NZ NZ NZ NZ NZ NZ NZ
150 recv_event : NZ NZ NZ NZ NZ NZ NZ NZ NZ NZ
151 recv_runcmd : NG NG NG NG NG NG NG NG NG NG
162 pltbind_now_pie : BI BI BI BI BI BI BI BI BI BI
166 dump_sched : NG NG NG NG NG NG NG NG NG NG
167 recv_sched : NZ NZ NZ NZ NZ NZ NZ NZ NZ NZ
179 arg_auto2 : OK NZ OK OK NZ OK OK NZ OK OK
182 thread_exit : OK OK OK OK OK OK OK OK NG OK
196 chrome_taskname : NG NG NG NG NG NG NG NG NG NG
202 arg_dwarf2 : NG NG NG NG NG SK SK SK SK SK
203 arg_dwarf3 : NG OK NG NG NG SK SK SK SK SK
204 arg_dwarf4 : OK OK NG NG OK SK SK SK SK SK
205 arg_auto4 : OK OK NZ OK OK SK SK SK SK SK
222 external_data : OK OK OK OK OK NG OK OK OK OK
251 exception4 : NG NG NG NG NG NG NG NG NG NG
You can run a single test with a single option as follows. -vdp is for verbose, diff, pg and -O0 means optimization level. The last 101 is a string for matching with tests.
$ ./runtest.py -vdp -O0 101
...
t101_dump_chrome: diff result of -pg -O0
--- expect 2022-06-21 23:12:09.172745571 +0100
+++ result 2022-06-21 23:12:09.172745571 +0100
@@ -1,10 +1 @@
-M process_name t-abc
-M thread_name t-abc
-B main
-B a
-B b
-B c
-E c
-E b
-E a
-E main
+
101 dump_chrome : NG
There was a problem hiding this comment.
We have another missing change as follows.
diff --git a/cmds/dump.c b/cmds/dump.c
index 0c3b3bbc..04ed1947 100644
--- a/cmds/dump.c
+++ b/cmds/dump.c
@@ -1013,9 +1013,12 @@ static void dump_chrome_footer(struct uftrace_dump_ops *ops,
struct uftrace_chrome_dump *chrome = container_of(ops, typeof(*chrome), ops);
/* read recorded date and time */
- snprintf(buf, sizeof(buf), "%s/info", opts->dirname);
- if (stat(buf, &statbuf) < 0)
- return;
+ snprintf(buf, sizeof(buf), "%s/info.txt", opts->dirname);
+ if (stat(buf, &statbuf) < 0) {
+ snprintf(buf, sizeof(buf), "%s/info", opts->dirname);
+ if (stat(buf, &statbuf) < 0)
+ return;
+ }
ctime_r(&statbuf.st_mtime, buf);
buf[strlen(buf) - 1] = '\0';
|
I'd like to hold this for a while and make the change with other file format changes like I mentioned before. |
qpakzk
force-pushed
the
feature/info-text-format
branch
2 times, most recently
from
a55013e to
ab895eb
Compare
@namhyung Could you please let us know when you have plan? @qpakzk Anyway, this PR has a few problems. First, you shouldn't create a merge commit in the PR. Second, the travis result is broken because of the previously merged patch, but this is fixed in the current master. To handle for both problems, you should rebase the branch properly and update the PR again. |
qpakzk
force-pushed
the
feature/info-text-format
branch
2 times, most recently
from
a11a390 to
2c05369
Compare
|
@honggyukim Thanks for your comment. I rebased it and updated my commit. |
I don't have ETA but it's not an urgent issue so please expect it takes long |
qpakzk
force-pushed
the
feature/info-text-format
branch
from
2c05369 to
c200d23
Compare
|
For the same reason, we have applied clang-format globally so your change now shows horrible conflicts so I've rebased your change and put the patch at master...honggyukim:uftrace:clang-format/feature/info-text-format. Please pick it and update your PR again. |
qpakzk
force-pushed
the
feature/info-text-format
branch
from
c200d23 to
e3098b3
Compare
All header info was written by binary format. But this commit changes all info to uftrace.data/info.txt in text format. To support backward compatibility with previous versions, if uftrace.data/info.txt doesn't exist, then find uftrace.data/info written by binary format. Fixed: namhyung#1408 Signed-off-by: Sangwon Hong <[email protected]>
qpakzk
force-pushed
the
feature/info-text-format
branch
from
e3098b3 to
a49e65c
Compare
|
@honggyukim Thanks for rebasing my change. I updated my PR. |
All header info was written by binary format. But this commit changes all info to uftrace.data/info.txt in text format.
Fixed: #1408.