nodejs.org icon indicating copy to clipboard operation
nodejs.org copied to clipboard

Fix sed command line in "Diagnostics - Flame Graphs" guide

Open kazarmy opened this issue 2 years ago • 2 comments

This pr fixes the sed command line used for perf file cleanup by enabling Extended Regular Expressions (ERE) syntax, so that the first sed script (that deletes lines containing " __libc_start", "LazyCompile" etc.) works. The alternative to enabling ERE syntax is to escape (, |, ) and ? which is less elegant.

kazarmy avatar Jun 04 '22 09:06 kazarmy

Ping @nodejs/diagnostics

nschonni avatar Jun 04 '22 17:06 nschonni

Hi, it's been more than a week. You can verify this fix yourself using diff -u on before/after perf files to see whether lines containing __libc_start| LazyCompile | v8::internal::| Builtin:| Stub:| LoadIC:|\[unknown\]| LoadPolymorphicIC: are actually deleted.

kazarmy avatar Jun 14 '22 11:06 kazarmy

I can confirm that this works as intended. @kazarmy sorry for the extremely slow reply, feel free to rebase this PR and I would more than happy approve and merge it :)

But yet, @nodejs/diagnostics if y'all want to have a word here

ovflowd avatar Mar 14 '23 09:03 ovflowd

@kazarmy sorry for the extremely slow reply

No problem ... better late than never :)

feel free to rebase this PR

Ok done

kazarmy avatar Mar 14 '23 11:03 kazarmy

I didn't have time to test it at the moment. I'll try to do it this week.

tony-go avatar Mar 14 '23 13:03 tony-go