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

Evaluate memory overhead of icinga::Locked<...> in object attributes #10113

Open
julianbrost opened this issue Aug 8, 2024 · 4 comments
Open
Labels
core/evaluate Analyse/Evaluate features and problems

Comments

@julianbrost
Copy link
Contributor

julianbrost commented Aug 8, 2024

As a countermeasure for race conditions, #9364 added a mutex for every object attribute with a type that's incompatible with std::atomic. At the moment, that's implemented using a dedicated std::mutex for every attribute for every single object. On my machine, sizeof(std::mutex) = 40, and if I compare the sizeof(icinga::Host) with and without these mutexes, that's a 70% increase. However, that won't result in a 70% increase in memory usage of Icinga 2 as a whole (for example, all strings like object names are dynamically allocated and thus not part of icinga::Host itself and aren't affected by this increase.

Tasks

  1. Figure out how much of an effect this has on the total memory use of Icinga 2.

  2. Improve this. One idea would be to take some inspiration from how something like atomic_load(const std::shared_ptr<T>*) is/can be implemented:

    These functions are typically implemented using mutexes, stored in a global hash table where the pointer value is used as the key.

    Note that if using only part of the address as the key, i.e. sharing the mutex between objects, this would reduce the memory requirements.

@julianbrost julianbrost changed the title Evaluate costs of #9364 Evaluate memory overhead of icinga::Locked<...> in object attributes Aug 8, 2024
@julianbrost julianbrost added the core/evaluate Analyse/Evaluate features and problems label Aug 8, 2024
@Al2Klimov Al2Klimov self-assigned this Aug 20, 2024
@Al2Klimov
Copy link
Member

Size in bytes (pointers) M3 Mac NixOS x64 laptop
sizeof(std::mutex) 64 (8) 40 (5)
alignof(std::mutex) 8 (1) 8 (1)

.ti attributes of a Service: about 90. Ex. numbers and bools: about 30. (icinga2 console > Service(), then grep -v | wc -l)

So, every Service consumes +1.2KB (30 x 40) with Locked<>.

@julianbrost
Copy link
Contributor Author

That gives no estimate on the big scale of things, i.e. how this affects the overall memory usage. There are more object types than just Host and Service affected by this.

@Al2Klimov Al2Klimov removed their assignment Aug 26, 2024
@yhabteab
Copy link
Member

yhabteab commented Sep 6, 2024

  • Figure out how much of an effect this has on the total memory use of Icinga 2.

I've been testing this for the entire week now and couldn't find a way to exactly determine the differences with and without this mutex. Attaching GDB to the running icinga2 process and calling malloc_stats() was promising, but then we found out that the output is next to useless as it only shows the virtual memory usage and not the actual physically allocated ones. So I just did it with the plain simple htop command and here is the result:

Setup (Debian 12 (Icinga 2 linked to jemalloc)):

  • 48070 Services
  • 4006 Hosts
  • 6 Dependencies
  • 249 CheckCommands
  • 24 HostGroups and 25 ServiceGroups
  • And at least 1 object for each of the remaining object types, e.g 1 IcingaDB, 2 Endpoints, 16 Notifications etc.
Main Process Memory Master w/ mutex Master w/o mutex
Resident (RES) 830M 690M
Virtual (VIRT) 3560M 1595M

@Al2Klimov
Copy link
Member

I see two different construction areas:

While the first one must be addressed ASAP, otherwise even 1 TB RAM wouldn't(?) help, the second one is, without doubt, much less problematic. At best no one even notices, at worst(!) people already have fixed it with one more RAM stick. And I even believe that, as so often, the truth is somewhere in the middle.

Hence, I opt for commenting-out this discussion from the upcoming bug fix release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core/evaluate Analyse/Evaluate features and problems
Projects
None yet
Development

No branches or pull requests

3 participants