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

Type::GetByName(): cache results for faster lookup #9553

Closed
wants to merge 1 commit into from

Conversation

Al2Klimov
Copy link
Member

Especially since #9543

"Blocked by" (not really, but preferably)

@Al2Klimov Al2Klimov self-assigned this Oct 28, 2022
@cla-bot cla-bot bot added the cla/signed label Oct 28, 2022
@icinga-probot icinga-probot bot added area/configuration DSL, parser, compiler, error handling core/quality Improve code, libraries, algorithms, inline docs ref/IP labels Oct 28, 2022
@Al2Klimov Al2Klimov marked this pull request as ready for review October 28, 2022 09:28
@julianbrost
Copy link
Contributor

I just had a really quick look at this and find it strange doing it this way around.

How much work would it be to make the primary source of types a std::unordered_map<String, Type::Ptr> and make globals.Types a view into this?

@Al2Klimov
Copy link
Member Author

Too much for what this change shall improve.

@Al2Klimov
Copy link
Member Author

Also globals.System.Types is initialised very early and never changes.

@Al2Klimov
Copy link
Member Author

Before/after, time icinga2 daemon -C

f59f361
real 36m38.277s
user 196m31.058s
sys 71m49.837s

real 35m14.356s
user 190m16.353s
sys 68m23.075s

real 35m35.668s
user 192m36.516s
sys 68m44.489s

979a918
real 33m37.158s
user 185m54.210s
sys 61m20.227s

real 32m31.482s
user 182m6.965s
sys 58m28.920s

real 32m7.106s
user 176m20.685s
sys 60m13.250s

In short, I'm proud of this bridge technology.

@Al2Klimov
Copy link
Member Author

Would you prefer if the register macros also fill a second, non-DSL, map (which is preferably accessed unlocked)?

@Al2Klimov
Copy link
Member Author

In short, I'm proud of this bridge technology.

We have caches also e.g. in #9408 after all.

@Al2Klimov
Copy link
Member Author

  • 56 cores/threads (IDK)
  • 256G RAM
  • Config of our customer 22470
  • time icinga2 daemon -C

master before #9627

real 54m20,817s
user 337m24,969s
sys 2459m21,103s

master before #9627 + #9553

real 49m3,936s
user 345m6,072s
sys 2208m49,892s

master with #9627

real 23m15,077s
user 1039m6,028s
sys 198m59,482s

master with #9627 + #9553

real 21m52,338s
user 963m19,549s
sys 203m45,879s

@Al2Klimov Al2Klimov marked this pull request as draft January 24, 2023 10:11
@icinga-probot icinga-probot bot deleted the Type-GetByName branch January 26, 2023 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/configuration DSL, parser, compiler, error handling cla/signed core/quality Improve code, libraries, algorithms, inline docs ref/IP
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants