Support -finstrument-functions

[rokups]

GCC supports -finstrument-functions option which makes compiler insert following function calls at the start and end of the function:

void __cyg_profile_func_enter (void *this_fn, void *call_site);
void __cyg_profile_func_exit  (void *this_fn, void *call_site);

dladdr can be then used to retrieve function name.

Merely enabling said option could allow easy_profiler automatically track all function calls.

[cas4ey]

Hi @rokups! Won't it be a huge performance hit to use dladdr on every function call? Or you suggesting to use it later in the ui? Getting names from hash map for every function call would be expensive too, I think. But it is interesting and needs to be tested. Have you experimented with it already?

[rokups]

I haven't tried yet. Maybe it could be optimized somewhat? Maybe caching names in a hashmap?

[cas4ey]

Already said about hash map :) (I was editing comment when you were typing reply) Anyway it needs to be tested. Could you perform some experiments? :)

[rokups]

This is on my TODO list so yeah, eventually :)

[rokups]

@cas4ey i need your advice. In order to use this function obvious candidate is ::profiler::beginNonScopedBlock(). However the way it is designed now mentioned function expects BaseBlockDescriptor*. Problem here is that we can not have BaseBlockDescriptor* created as static variable as EASY_NONSCOPED_BLOCK() macro does, because same code in __cyg_profile_func_enter() will profile many functions. I actually had very same problem when hiding easy_profiler API and making it scripting-friendly, this was my solution:

void Profiler::BeginBlock(const char* name, const char* file, int line, unsigned int argb, unsigned char status) __attribute__((no_instrument_function))
{
	// Line used as starting hash value for efficiency.
	// This is likely to not play well with hot code reload.
	unsigned hash = StringHash::Calculate(file, (unsigned)line);	// TODO: calculate hash at compile time
	HashMap<unsigned, ::profiler::BaseBlockDescriptor*>::Iterator it = blockDescriptorCache_.Find(hash);
	const ::profiler::BaseBlockDescriptor* desc = 0;
	if (it == blockDescriptorCache_.End())
	{
		String uniqueName = ToString("%s at %s:%d", name, file, line);
		desc = ::profiler::registerDescription((::profiler::EasyBlockStatus)status, uniqueName.CString(), name, file,
											   line, ::profiler::BLOCK_TYPE_BLOCK, argb, true);
	}
	else
		desc = it->second_;
	::profiler::beginNonScopedBlock(desc, name);
}

It sucks. And it is not viable with __cyg_profile_func_enter() because now i need extensive list of opt-out functions. If __cyg_profile_func_enter() calls anything that calls __cyg_profile_func_enter() we end up with infinite recursion.

I was wondering.. Could we maybe somehow have a version of ::profiler::beginNonScopedBlock() that does not require initializing and caching BaseBlockDescriptor*?

P.S. embeddedprofiler is making use of these functions so they probably are viable thing for profiling: https://embeddedprofiler.com/User_Manual#_gcc_and_mingw_compilers

[cas4ey]

@rokups , can __cyg_profile_func_... functions be declared inside profiler_manager.cpp? If yes, then you don't need any of profiler::beginBlock functions at all - you can access ProfilerManager directly. You can use a hash map with a function pointer as a key, no need to calculate hash from filename and function name, I think.

embeddedprofiler looks not so viable nowadays... :) p.s. As I see it used a separate symbols file for -finstrument-functions mode.

[cas4ey]

P.S. isn't it better just to use a sampling profiler? Less accurate but giving a big picture. I think collecting all function calls would give us a really huge output file which would be hard to load and to display in the UI.

[rokups]

can _cyg_profile_func... functions be declared inside profiler_manager.cpp?

Yes they can, and they probably should be anyway. I will investigate that direction.

P.S. isn't it better just to use a sampling profiler? Less accurate but giving a big picture. I think collecting all function calls would give us a really huge output file which would be hard to load and to display in the UI.

Yes this is a tough nut to crack. Sampling profilers are cool due to zero setup requirements but they fail miserably when trying to diagnose micro stutters. Explicit profilers are awesome because we are given exact information about every tiny performance drop. They also are annoying to use due to extensive setup requirements through all of the codebase.. I started looking into __cyg_profile_* funcs to ease the life for the lazy (like me).

Could maybe UI be reworked somewhat to only work on hierarchy of nodes to display when they are accessed? That would help in this case and also be a great improvement for easy_profiler.

[cas4ey]

Explicit profilers are awesome because we are given exact information about every tiny performance drop.

Don't forget about performance impact... When you will add additional 15-20ns to the every function you have, including every operator (), every ptr() or get() and so on (which invoked billion of times per second), - you can get a serious slowdown which would hide those micro stutters from you. Explicit profilers are good with their explicitness :) But maybe I'm wrong.

By the way, you can test it right now by adding EASY_NONSCOPED_BLOCK to the __cyg_profile_func_enter() and EASY_END_BLOCK; to the __cyg_profile_func_exit.

Could maybe UI be reworked somewhat to only work on hierarchy of nodes to display when they are accessed? That would help in this case and also be a great improvement for easy_profiler.

I'm not sure if I can handle it in a decent time... :)