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

Forceinline improves performance significantly with MSVC #108

Closed
wants to merge 1 commit into from

Conversation

mscppppppppp
Copy link

@mscppppppppp mscppppppppp commented Jan 8, 2024

I have proposed a pull request inspired by a strategy discussed in an issue from another repository boostorg/unordered#168. My aim is to enhance the performance of unordered_dense.h through strategic inlining. To this end, I've introduced the ANKERL_UNORDERED_DENSE_FORCEINLINE macro to various functions within the file.

I've attached benchmark results from MSVC for your review:
ankerl_unordered_dense.md
ankerl_unordered_dense_forceinline.md
While I believe these changes will benefit the performance based on the benchmarks, I understand that method selection and the use of forceinline might not align with all coding practices or performance considerations. If you find that my approach is not suitable, or if there are specific modifications you'd prefer, please feel free to close this PR. I'm open to any feedback or alternate suggestions.

Additionally, if it's more appropriate, I can open an issue to discuss the potential for these kinds of optimizations more broadly before proceeding with code changes. Your guidance on the best approach to contribute to this project is greatly appreciated.

Thank you for considering my contribution. I look forward to your feedback.

add ANKERL_UNORDERED_DENSE_FORCEINLINE
@martinus
Copy link
Owner

martinus commented Oct 5, 2024

Thanks, but I've played a bit with your changes, and for clang and gcc they seem to make things a bit slower mostly on my machine.

I think forcing inline is the wrong approach, modern compilers usually know best. If not, I'd use PGO.

@martinus martinus closed this Oct 5, 2024
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