-
Notifications
You must be signed in to change notification settings - Fork 187
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.
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
Use inline #3936
Use inline #3936
Conversation
def98f6
to
84b36e7
Compare
84b36e7
to
64d68e5
Compare
In general for modern compilers you should not use the inline keyword, and instead rely on compiler optimization flags to decide whether the function should be inlined or not automatically. In particular inlining will increase the size of the executable since the function's code is inserted where there would otherwise be a shared function (so very small functions are not a problem, but these could be determined automatically when optimizing the code). Code size versus speed should be considered in particular for the Javascript toolkit version for download over the internet, where size is more important than otherwise for a C++ library version of verovio running on a local computer. Asking the new ChatGPT 3o model ("Great at coding and logic") about this:
The Advantages
Disadvantages
Summary
In modern C++ development, the compiler's optimization capabilities are quite advanced. Often, it is best to write clear and maintainable code and let the compiler decide which functions to inline based on its optimization heuristics, rather than relying too heavily on the |
@craigsapp I'm not completely sure what you want to tell.
So it's smaller and faster. |
It does not seem to make a very significant difference size-wise. Probably not more speed-wise. You can see https://emscripten.org/docs/tools_reference/settings_reference.html#inlining-limit for the emscripten compiler. |
If a function is defined inside a class declaration, it is automatically considered inline by the compiler. So it should not be necessary to add It is a different story if the function does not belong to a class. Without I think so far the approach has been to add |
It is smaller, which is interesting, but you did not provide proof of speed (but I would expect inline code to be faster since it does not need to jump to a new location in memory). Probably there are no local variables in the function, so the space saved is based on the lack of a pointer to a separate function. It is fine with me, but note that
That is a good point, although I avoid defining functions in header files, so I have not been aware of that (probably because I got duplicate function compiling errors in the past and instinctively avoid doing that :-). |
OK. I removed them 4528635 |
Thanks for the discussion! |
This PR combines the
PercentToStr
method for other percentage values, and marks several forwards asinline
to reduce function calls.