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

Further optimizations for province automapping #62

Merged
merged 8 commits into from
Dec 10, 2024
Merged

Conversation

IhateTrains
Copy link
Member

@IhateTrains IhateTrains commented Dec 9, 2024

Notable changes:

  • Drastically reduced the number of isProvinceMapped calls (which turned out to be very expensive).
  • Reduced the number of locks when registering source-target province matches, thus increasing the gains from parallelism.
  • Included https://github.com/martinus/robin-hood-hashing for its faster (about 2x in my test runs) implementation of unordered_map. EDIT: replaced with https://github.com/greg7mdp/gtl
  • Made Province struct a class because its instances are quite big, and added some encapsulation.
  • Moved some automapping-related code from ImageFrame to Automapper, which seems a better place for it.
  • Some micro-optimizations like making classes final.
  • A source map pixel is no longer skipped if (sourcePixel.x + sourcePixel.y) % 2 == 0. The code is about 32x faster, despite checking all source map pixels instead of only half of them, so it's no longer needed to reduce the mapping accuracy.

Logged time measurements on Ryzen 7 7700, with I:R Terra Indomita to CK3 Rajas of Asia mappings being generated.

before (measured on an older version of Rajas of Asia, but shouldn't matter much):

Elapsed time (sec): 34.02
Elapsed time (sec): 76.0615
Elapsed time (sec): 70.8512
Elapsed time (sec): 71.7781
Elapsed time (sec): 69.4187

after (see new updated in comment below):

Elapsed time (sec): 9.58116
Elapsed time (sec): 2.19897
Elapsed time (sec): 2.26537
Elapsed time (sec): 2.25273
Elapsed time (sec): 2.23148

The first run always takes a longer time, because a point-province map is being initialized. It's reused in subsequent runs.

@IhateTrains IhateTrains marked this pull request as ready for review December 9, 2024 04:00
@IhateTrains IhateTrains requested a review from Zemurin December 9, 2024 04:00
@IhateTrains IhateTrains changed the title Further optimize province automapping Further optimizations for province automapping Dec 9, 2024
@IhateTrains IhateTrains marked this pull request as draft December 9, 2024 14:31
@IhateTrains
Copy link
Member Author

IhateTrains commented Dec 9, 2024

Made some changes because I realized that already mapped provinces can't be excluded from tgtPointToWaterProvinceMap and tgtPointToLandProvinceMap if the maps are to be used in subsequent runs, in which the previously mapped provinces can be unmapped.

Also replaced https://github.com/martinus/robin-hood-hashing with https://github.com/greg7mdp/gtl. The previous library was archived, and the new one also provides a custom hashset implementation.

New time measurements:

Elapsed time (sec): 9.60111
Elapsed time (sec): 2.93702
Elapsed time (sec): 2.92947
Elapsed time (sec): 2.90784
Elapsed time (sec): 2.99318

The PR's ready for review again.

@IhateTrains IhateTrains marked this pull request as ready for review December 9, 2024 19:17
Copy link
Member

@Zemurin Zemurin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It certainly appears ok, didn't find anything dubious. Hope everything else works as well. :)

@IhateTrains IhateTrains merged commit 2360603 into master Dec 10, 2024
3 checks passed
@IhateTrains IhateTrains deleted the optimizations branch December 14, 2024 00:16
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.

2 participants