godotengine / godot-cpp

C++ bindings for the Godot script API
MIT License
1.74k stars 574 forks source link

Compute profiling signatures at compile-time #378

Open Zylann opened 4 years ago

Zylann commented 4 years ago

While I skimmed randomly this repo I found out this file: https://github.com/GodotNativeTools/godot-cpp/blob/master/include/core/GodotProfiling.hpp It's late here but I have a suggestion I thought noting down.

I believe one key of profiling is to be as lightweight as possible, especially with languages that run really fast like C++. Here, snprintf is used, and this may induce overhead (especially in nested cases), to print constant data at runtime into a static buffer which may be either too large in majority of cases, or too small in worst cases.

I believe snprintf can be removed. Instead, macros can do all that work, at compilation time. Then the only thing FunctionProfiling needs is a pointer to the compile-time string, and no buffer. Example of how I did to obtain that string with my own small profiler: https://github.com/Zylann/godot_voxel/blob/9f7a081dd409e703dff6bb9318cef99cb297b5f3/util/zprofiling.h#L14

Zylann commented 4 years ago

After doing some tests, I came to the conclusion that this cannot be done by the preprocessor, at least not as long as gdnative_profiling_add_data requires the whole signature as a single const char*. The reason is, __FUNCTION__ is not a macro, it's known by the compiler, not the preprocessor. We could do it fine in a simple way with the __LINE__ macro instead, but having a function name is nicer.

Still, allocating a large buffer on the stack and formatting with snsprintf every single time surely can be avoided at compile time... https://stackoverflow.com/questions/20965003/how-to-concatenate-a-const-char-in-compile-time This is giving me some ideas, but I need to investigate more. Notably, this could be of help https://akrzemi1.wordpress.com/2017/06/28/compile-time-string-concatenation/

Alternatively, it would be really nice if Godot's API could allow us to specify a const char* AND a line number as an integer, it would simplify the problem, but for that to be worth it, Godot should not concatenate it anyways as soon as it receives it. The point is to profile, not to introduce bunch of overhead...