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.

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

Merged
merged 3 commits into from
Feb 3, 2025
Merged

Use inline #3936

merged 3 commits into from
Feb 3, 2025

Conversation

rettinghaus
Copy link
Contributor

This PR combines the PercentToStr method for other percentage values, and marks several forwards as inline to reduce function calls.

@rettinghaus rettinghaus requested a review from brdvd February 3, 2025 10:50
@lpugin lpugin merged commit fd96914 into rism-digital:develop Feb 3, 2025
5 checks passed
@rettinghaus rettinghaus deleted the develop-inline branch February 3, 2025 13:54
@craigsapp
Copy link
Contributor

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:

What is the advantage/disadvantage of using inline keyword in C++?

The inline keyword in C++ serves two main purposes, each with its own advantages and disadvantages:

Advantages

  1. Potential Performance Improvement:
  • Reduced Function Call Overhead:
    When you mark a function as inline, you are suggesting to the compiler that it should replace the function call with the actual function code. This can eliminate the overhead associated with a function call (such as setting up a stack frame), which can be beneficial especially for very small, frequently called functions.
  1. Header File Definitions:
  • Avoiding Multiple Definitions:
    In C++, if you define a function in a header file that is included in multiple translation units, you risk violating the One Definition Rule (ODR). Declaring the function as inline tells the linker that multiple identical definitions across different translation units are acceptable. This makes it safe to define functions in headers.

Disadvantages

  1. Code Bloat:
  • Increased Binary Size:
    If an inline function is large or used in many places, substituting the function code at every call site can lead to a larger binary. This code bloat can negatively impact performance, for example by increasing the pressure on the instruction cache.
  1. Limited Compiler Control:
  • A Hint, Not a Command:
    The inline keyword is merely a suggestion to the compiler. Modern compilers perform their own optimizations and may choose to inline functions that aren’t marked inline or ignore the inline suggestion if the function is too complex. Thus, using the keyword does not guarantee that inlining will occur.
  1. Debugging Difficulties:
  • Harder to Set Breakpoints:
    When code is inlined, there isn’t a single, distinct function call to break on during debugging. This can make stepping through the code or diagnosing issues more challenging.

Summary

  • When to Use inline:
  • It is most useful for very small functions where the overhead of a function call is significant compared to the function's work.
    It is essential when you want to define a function in a header file that will be included in multiple translation units, thus avoiding multiple definition errors.
  • When to Avoid or Be Cautious with inline:
  • For large functions or functions that are called infrequently, inlining might increase code size without providing a noticeable performance benefit.
  • Relying too much on inline can sometimes hinder the compiler's ability to perform more effective optimizations.

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 inline keyword.

@rettinghaus
Copy link
Contributor Author

@craigsapp I'm not completely sure what you want to tell.

  • Binary size before this PR: 13611448
  • Binary size after this PR: 13611192

So it's smaller and faster.
It would be good if you could share those compiler optimization flags.

@lpugin
Copy link
Contributor

lpugin commented Feb 3, 2025

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.

@brdvd
Copy link
Contributor

brdvd commented Feb 3, 2025

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 inline for functions like Att:: MeasurementunsignedToStr.

It is a different story if the function does not belong to a class. Without inline it would violate the One Definition Rule and you get a linker error there. One example is DurationMin/Max in vrv.h.

I think so far the approach has been to add inline defensively, i.e. only in those cases where it is required for successful compilation.

@craigsapp
Copy link
Contributor

So it's smaller and faster.

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 inline is a suggestion that now-a-days is typically left to the compiler when optimizing code.

It is a different story if the function does not belong to a class. Without inline it would violate the One Definition Rule and you get a linker error there. One example is DurationMin/Max in vrv.h.

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 :-).

@lpugin
Copy link
Contributor

lpugin commented Feb 3, 2025

OK. I removed them 4528635

@rettinghaus
Copy link
Contributor Author

Thanks for the discussion!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants