Skip to content

portable sched_getcpu calls#2272

Closed
ajkr wants to merge 3 commits intofacebook:masterfrom
ajkr:fix-sched-getcpu
Closed

portable sched_getcpu calls#2272
ajkr wants to merge 3 commits intofacebook:masterfrom
ajkr:fix-sched-getcpu

Conversation

@ajkr
Copy link
Copy Markdown
Contributor

@ajkr ajkr commented May 10, 2017

  • added a feature test in build_detect_platform to check whether sched_getcpu() is available. glibc offers it only on some platforms (e.g., linux but not mac); this way should be easier than maintaining a list of platforms on which it's available.
  • refactored PhysicalCoreID() to be simpler / less repetitive. ordered the conditional compilation clauses from most-to-least preferred

@ajkr ajkr requested a review from lightmark May 10, 2017 17:57
@ajkr ajkr requested a review from siying May 10, 2017 17:58
}
EOF
if [ "$?" = 0 ]; then
COMMON_FLAGS="$COMMON_FLAGS -DROCKSDB_SCHED_GETCPU_PRESENT"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would you want to set it in build_tools/fbcode_config.sh and build_tools/fbcode_config4.8.1.sh too?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

do we still use those? I noticed our TARGETS file manually sets some of these like -DROCKSDB_FALLOCATE_PRESENT. Is adding it to TARGETS sufficient?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

oh never mind, I see why they're needed now.

Copy link
Copy Markdown
Contributor

@siying siying May 10, 2017

Choose a reason for hiding this comment

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

And you have a good point. You need to add it to TARGETS file too. I think we may also have missed several other flags there and TARGETS files.

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@ajkr updated the pull request - view changes

@facebook-github-bot
Copy link
Copy Markdown
Contributor

@ajkr updated the pull request - view changes

@facebook-github-bot
Copy link
Copy Markdown
Contributor

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

@@ -401,6 +401,17 @@ EOF
if [ "$?" = 0 ]; then
COMMON_FLAGS="$COMMON_FLAGS -DROCKSDB_RANGESYNC_PRESENT"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Damn, we need to add this one too before the next release.

@ajkr ajkr mentioned this pull request May 10, 2017
facebook-github-bot pushed a commit that referenced this pull request May 11, 2017
Summary:
address siying's comment in #2272.
Closes #2274

Differential Revision: D5039489

Pulled By: ajkr

fbshipit-source-id: 3e2d957d3469c13d0e33ededa59320c4c3f24ef6
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