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

LRUCacheShard cache line size alignment #2620

Closed
wants to merge 5 commits into from
Closed

LRUCacheShard cache line size alignment #2620

wants to merge 5 commits into from

Conversation

grooverdan
Copy link
Contributor

combining #2568 and #2612.

@grooverdan
Copy link
Contributor Author

mingw32 failure - https://gcc.gnu.org/bugzilla/show_bug.cgi?id=52991

@facebook-github-bot
Copy link
Contributor

@IslamAbdelRahman has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@IslamAbdelRahman
Copy link
Contributor

Thanks @grooverdan, How can we suppress the MinGW failure .. we don't want our travis failure to consistently fail

@grooverdan
Copy link
Contributor Author

I tried just adding -mno-ms-bitfields on the mingw32 however this wasn't sufficient https://travis-ci.org/grooverdan/rocksdb/jobs/255882007

So as per next commit just disabling alignment for mingw32

@facebook-github-bot
Copy link
Contributor

@grooverdan updated the pull request - view changes - changes since last import

@facebook-github-bot
Copy link
Contributor

@IslamAbdelRahman has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Contributor

@IslamAbdelRahman IslamAbdelRahman left a comment

Choose a reason for hiding this comment

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

Thanks!

@grooverdan
Copy link
Contributor Author

Thanks for the initial work and merging.

@yiwu-arbug
Copy link
Contributor

Seems this still doesn't properly align LRUCacheShard, as shown by running tests with UBSAN: https://gist.github.com/yiwu-arbug/401217b8868be158a545f85220785e17

I guess it is because we allocate shard_ as an array, which short-circuit the overloaded new operator.

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.

4 participants