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.

Sign up for GitHub

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

Jump to bottom

Implement ignore #28

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Implement ignore #28

wants to merge 4 commits into from

Conversation

Bojun-Seo
Copy link
Collaborator

Sometimes user wants to remove some reports not necessary to be traced.
Suppressions feature will filter user defined symbols, functions and files.
This feature resembles suppressions feature in llvm-project

@Bojun-Seo Bojun-Seo mentioned this pull request Mar 15, 2023
Open
@honggyukim
Copy link
Owner

It'd be helpful if you write some execution sequence for the usage of this suppression feature in the commit message.

@Bojun-Seo
Copy link
Collaborator Author

I'll add the execution sequence for the usage of this suppression feature on commit message after #29 is merged.
Since this patch(#28) will conflict with #29

@honggyukim
Copy link
Owner

Thanks for the PR, but I think the name suppressions is better to be ignore.

@Bojun-Seo
Copy link
Collaborator Author

That sounds nice, I'll change the name from suppressions to ignore

@Bojun-Seo Bojun-Seo force-pushed the suppressions branch 2 times, most recently from fd7de06 to 4db7c2f Compare November 6, 2023 04:04
@Bojun-Seo Bojun-Seo changed the title stacktrace: Implement suppressions Implement ignore Nov 6, 2023
@honggyukim
Copy link
Owner

I couldn't look into the details but just tested this PR. It looks working fine so I don't want to hold this PR.

I just want to ask you to give 2 whitespaces when pasting the console output.

Your current commit message is written as follows.

You can use ignore option as follows.
$ ./heaptrace --ignore=ignore.txt samples/factorial.out

But I think this is better to be written as follows.

You can use ignore option as follows.

  $ ./heaptrace --ignore=ignore.txt samples/factorial.out

And please rebase this PR onto the current main branch.

@Bojun-Seo
heaptrace: Add ignore feature interface
1acd31a 
This patch adds ignore feature interface only
Sometimes user wants to remove some reports not necessary to be traced
Ignore feature will filter user defined symbols, functions and files
This feature resembles suppressions feature in llvm-project

Signed-off-by: Bojun Seo <bojun.seo.0@gmail.com>
@Bojun-Seo
Copy link
Collaborator Author

I changed the patch as you suggested.
Thank you for the review!

@Bojun-Seo
stacktrace: Change print_backtrace_symbol to get_backtrace_string
1855b50 
To make backtrace string could be printed selectively, Change each
function not to print inside the function rather return the report
string.

  print_backtrace_symbol            -> get_backtrace_string
  print_backtrace_symbol_flamegraph -> get_backtrace_string_flamegraph

Signed-off-by: Bojun Seo <bojun.seo.0@gmail.com>
@Bojun-Seo
stacktrace: Implement is_ignored function
1444784 
is_ignored function can be used to check whether the input `report` contains
any ignore string. Ignore string can be read from the given file. Each line
of the file is considered as ignore string. Ignore string could be function
name, library name as well as something you want.

Ignore file example:

  $ cat ignore.txt
  libfoo
  bar
  baz

Signed-off-by: Bojun Seo <bojun.seo.0@gmail.com>
@Bojun-Seo Bojun-Seo force-pushed the suppressions branch 2 times, most recently from e6f6318 to 597169d Compare November 15, 2023 05:25
@Bojun-Seo
stacktrace: Implement ignore
b005e18 
If user want to ignore certain report, it could be filtered out.
The filtering list can be given with the file, each line is used
on filtering. The usage is as follows:

  $ ./heaptrace --ignore=ignore.txt samples/factorial.out
  [heaptrace] initialized for /proc/227960/maps (factorial.out)
  [heaptrace]   finalized for /proc/227960/maps (factorial.out)
  =================================================================
  [heaptrace] dump allocation sorted by 'size' for /proc/227960/maps (factorial.out)
  [heaptrace] heap traced num of backtrace : 9
  [heaptrace] heap traced allocation size  : 48 bytes
  [heaptrace] allocator info (virtual)     : 135.168 KB
  [heaptrace] allocator info (resident)    : 89.904 KB
  [heaptrace] statm info (VSS/RSS/shared)  : 6.270 MB / 3.932 MB / 3.801 MB
  =================================================================

  $ cat ignore.txt
  libfoo
  bar
  fac
  baz

The original execution of samples/factorial.out is like followings:

  $ ./heaptrace samples/factorial.out
  [heaptrace] initialized for /proc/227946/maps (factorial.out)
  [heaptrace]   finalized for /proc/227946/maps (factorial.out)
  =================================================================
  [heaptrace] dump allocation sorted by 'size' for /proc/227946/maps (factorial.out)
  === backtrace honggyukim#1 === [count/peak: 1/1] [size/peak: 10 bytes/10 bytes] [age: 325.674 us]
  0 [0x7f71089b8b6f] malloc +0x1f (./libheaptrace.so +0x4b6f)
  1 [0x557ebd3191c4] fac +0xe (samples/factorial.out +0x11c4)
  2 [0x557ebd3191d8] fac +0x13 (samples/factorial.out +0x11d8)
  3 [0x557ebd3191d8] fac +0x13 (samples/factorial.out +0x11d8)
  4 [0x557ebd3191d8] fac +0x13 (samples/factorial.out +0x11d8)
  5 [0x557ebd3191d8] fac +0x13 (samples/factorial.out +0x11d8)
  6 [0x557ebd3191d8] fac +0x13 (samples/factorial.out +0x11d8)
  7 [0x557ebd3191d8] fac +0x13 (samples/factorial.out +0x11d8)

  --- snip ---

  === backtrace honggyukim#9 === [count/peak: 2/2] [size/peak: 1 bytes/1 bytes] [age: 330.780 us]
  0 [0x7f71089b8b6f] malloc +0x1f (./libheaptrace.so +0x4b6f)
  1 [0x557ebd3191a5] fac +0x7 (samples/factorial.out +0x11a5)
  2 [0x557ebd3191d8] fac +0x13 (samples/factorial.out +0x11d8)
  3 [0x557ebd3191d8] fac +0x13 (samples/factorial.out +0x11d8)
  4 [0x557ebd3191d8] fac +0x13 (samples/factorial.out +0x11d8)
  5 [0x557ebd3191d8] fac +0x13 (samples/factorial.out +0x11d8)
  6 [0x557ebd3191d8] fac +0x13 (samples/factorial.out +0x11d8)
  7 [0x557ebd3191d8] fac +0x13 (samples/factorial.out +0x11d8)

  [heaptrace] heap traced num of backtrace : 9
  [heaptrace] heap traced allocation size  : 48 bytes
  [heaptrace] allocator info (virtual)     : 135.168 KB
  [heaptrace] allocator info (resident)    : 89.776 KB
  [heaptrace] statm info (VSS/RSS/shared)  : 6.270 MB / 3.932 MB / 3.801 MB
  =================================================================

Signed-off-by: Bojun Seo <bojun.seo.0@gmail.com>
honggyukim
honggyukim approved these changes Nov 25, 2023
View reviewed changes
Copy link
Owner

@honggyukim honggyukim left a comment

Choose a reason for hiding this comment

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

I think it looks good so please test it again yourself and merge this into master.

src/stacktrace.cc Outdated
static bool is_ignored(const std::string &report)
{
lazyinit_ignorevec();
for (auto ignore : ignorevec) {
Copy link
Owner

Choose a reason for hiding this comment

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

clang-tidy complains about this.

/home/honggyu/work/heaptrace/src/stacktrace.cc:61:12: warning: loop variable is copied but only used as const reference; consider making it a const reference [performance-for-range-copy]
        for (auto ignore : ignorevec) {
                  ^
             const  &

@Bojun-Seo Bojun-Seo mentioned this pull request Nov 27, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@honggyukim honggyukim honggyukim approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Bojun-Seo @honggyukim