[Performance] Change skill_cap from vector to map

[xackery]

Description

Prior CPU load was high on skill caps

Fixes # N/A

Type of change

Changes a vector to a map

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)

Testing

before this fix:

after this fix:

message details seen here: https://discord.com/channels/212663220849213441/557677602706423982/1228873494671130684

Clients tested: RoF2, client shouldn't be applicable, it's a server side fix

Checklist

• I have tested my changes
• I have performed a self-review of my code. Ensuring variables, functions and methods are named in a human-readable way, comments are added only where naming of variables, functions and methods can't give enough context.
• I own the changes of my code and take responsibility for the potential issues that occur

[Kinglykrab]

👍

[Akkadius]

Thanks for pushing this up. I think the change to a map makes sense especially with the dataset depth of 58k+ entries. Vectors are incredibly fast but with this much data essentially being accessed via key, maps are better suited.

I love that you attached flame graphs, but your flame graphs before and after are not representative of the same things.

I ended up throwing together a quick benchmark for myself because I wanted to know how terrible the performance currently is.

Here's my test benchmark, all it does is do some semi-expensive work in a loop regarding skill caps to stress lookups.

BenchTimer benchmark;

for (int i = 0; i < 100000; i++) {
	for (const auto &s: EQ::skills::GetSkillTypeMap()) {
		auto current_skill_value = (
			EQ::skills::IsSpecializedSkill(s.first) ?
				MAX_SPECIALIZED_SKILL :
				skill_caps.GetSkillCap(GetClass(), s.first, GetLevel()).cap
		);
	}
}

LogInfo("MaxSkills took [{}]", benchmark.elapsed());

Vector Lookups

MaxSkills took [143.216751001]
MaxSkills took [120.783758598]

Map Lookups - String Key

MaxSkills took [1.808443233]
MaxSkills took [2.982354806]
MaxSkills took [2.284587722]
MaxSkills took [2.15480665]

72x speedup

I further refined this by replacing the map an integer based key.

Map Lookups - Integer Key

MaxSkills took [0.116583367]
MaxSkills took [0.117441757]
MaxSkills took [0.11748233]
MaxSkills took [0.117634232]
MaxSkills took [0.114459948]
MaxSkills took [0.113323209]

1243x speedup

I'll push up the integer based key changes and lets get this into master and on a release