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

improve: AreaCombat::areas to array #1653

Merged
merged 2 commits into from
Oct 5, 2023
Merged

improve: AreaCombat::areas to array #1653

merged 2 commits into from
Oct 5, 2023

Conversation

mehah
Copy link
Contributor

@mehah mehah commented Sep 30, 2023

Accessing the array index is almost ~2x faster than using a std::map. There would only be advantages to using std::map if there were a large number of records, which is not the case.

Memory consumption may be a little higher with an array, as it will allocate memory to directions that may not be used, but it is irrelevant compared to the performance gain.

Benchmark:
image

struct Teste {
	bool any = false;
};

std::array<std::unique_ptr<Teste>, 8> arrayTest;
std::map<int, std::unique_ptr<Teste>> mapTest;
arrayTest[3] = std::make_unique<Teste>();
arrayTest[4] = std::make_unique<Teste>();
arrayTest[5] = std::make_unique<Teste>();
arrayTest[6] = std::make_unique<Teste>();

Benchmark bm;
for (size_t i = 0; i < 999999999; ++i) {
	const auto &v = arrayTest[3 + (rand() % 6)];
}
g_logger().info("Array: " + std::to_string(bm.duration()) + "ms");

bm.reset();
bm.start();
for (size_t i = 0; i < 999999999; ++i) {
	const auto &v = mapTest[3 + (rand() % 6)];
}
g_logger().info("Map: " + std::to_string(bm.duration()) + "ms");

@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 5, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@luan luan merged commit 4863c1d into main Oct 5, 2023
@luan luan deleted the area-map-to-array branch October 5, 2023 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants