Refine file naming scheme for profiling data #337
Conversation
src/main.c
Outdated
| char *prog_basename = basename(opt_prog_name); | ||
| prof_out_file = malloc(strlen(prog_basename) + 12 + 1); | ||
| assert(prof_out_file); | ||
| sprintf(prof_out_file, "./prof/%s.prof", prog_basename); |
There was a problem hiding this comment.
The leading prof/ directory name is not really making sense. Instead, let's place the profiling data in the same directory where the RV32 ELF file locates.
There was a problem hiding this comment.
I think you mean build directory
There was a problem hiding this comment.
I think putting it in the build directory is reasonable, we can distinguish the executable files and profiling data by filename extension.
ChinYikMing
force-pushed
the
pf_basename
branch
from
0128cc2 to
e7711fa
Compare
|
Note that the trailing spaces are removed automatically by editor when I motify "README" and "rv_profiler", not my primary intention. |
jserv
changed the title
src/main.c
Outdated
| char *prog_basename = basename(opt_prog_name); | ||
| prof_out_file = malloc(strlen(prog_basename) + 11 + 1); | ||
| assert(prof_out_file); | ||
| sprintf(prof_out_file, "build/%s.prof", prog_basename); |
There was a problem hiding this comment.
Is the leading build/ necessary?
There was a problem hiding this comment.
It depends on working directory of rv32emu executable. There are 2 cases to run profiler:
// outside build directory
xxx/build/rv32emu -p xxx/build/xxx.elf
// inside build directory
./rv32emu -p xxx.elf
Without leading build/, the first case will always write to cwd which is parent directory of xxx. For second case, it is all right.
I think the new approach could be concatenating the cwd and relative path (e.g., xxx/build/ in first case and ./ in second case) to rv32emu executable. This concatenated path will always point to the build directory.
There was a problem hiding this comment.
I think the new approach could be concatenating the cwd and relative path (e.g.,
xxx/build/in first case and./in second case) torv32emuexecutable. This concatenated path will always point to thebuilddirectory.
I think the new one is better, and I expect a cleaner and elegant implementation.
- fix incorrect basename generated by dynamic profiler - force profiling data store in build directory regardless of the cwd - update README Close sysprog21#336
ChinYikMing
force-pushed
the
pf_basename
branch
from
e7711fa to
0b8a712
Compare
|
Thank @ChinYikMing for contributing! |
Refine file naming scheme for profiling data
Close #336