-
-
Notifications
You must be signed in to change notification settings - Fork 338
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
Add ankerl::unordered_dense containers #1881
Conversation
Bloaty Results 🐋Compared to main
Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-1881-compared-to-main.txtCompared to d387090 (legacy)
Full report: https://maplibre-native.s3.eu-central-1.amazonaws.com/bloaty-results/pr-1881-compared-to-legacy.txt |
@ntadej @louwers I think its time to retire GCC 8. Ran into what appears to be a defect in No other compilers tested (MSVC 17 2022, Clang 11, 14, GCC 10, 12) show the same defect. I've temporarily forked this library to not try and include PMR if RTTI is off but before this is merged I want my temporary fork reverted. |
Which platform is that? |
Looks like Ubuntu. |
Can you just revert the fork then? I'll have a look and probably then we can either try to update the compiler or disable the test. |
I've reverted the fork modification that addressed GCC 8. |
Can you please try updating |
Changed the image but no dice. I think as long as GCC 8 is trying to build this the workflow will fail. |
Hmm, I thought this image has a newer compiler. Let me update it tomorrow. |
You can rebase now. |
Awesome! Thanks for taking a look. |
|
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Add
ankerl::unordered_dense
containers as a default build option, replacing std containers in various hot-paths. This yields a notable performance improvement on Android and Windows platforms.