This is an archive of the discontinued LLVM Phabricator instance.

[clangd] Add thread priority lowering for MacOS as well
ClosedPublic

Authored by kadircet on Feb 21 2019, 1:31 AM.

Diff Detail

Event Timeline

kadircet created this revision.Feb 21 2019, 1:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 21 2019, 1:31 AM
ilya-biryukov added a subscriber: gribozavr.

Not an expert on MacOS threading APIs, only checked this builds on Mac.
@gribozavr could you confirm using these APIs is the right thing to do on Mac?

clangd/Threading.cpp
128

Could you add a corresponding #include directive on Mac (#include <sys/resource.h>)?

ilya-biryukov added inline comments.Feb 21 2019, 4:48 AM
clangd/Threading.cpp
127

This points into iPhoneOS docs, might be confusing.
Is there a link for the desktop version of the OS somewhere?
Could also leave it out, I'm sure setpriority is easy to google.

gribozavr added a comment.EditedFeb 21 2019, 9:55 AM

macOS supports pthread_setschedparam() that we call on Linux, why not reuse the same code path?

https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/pthread_setschedparam.3.html

This one was used in a few different places in clang, for ex: https://github.com/llvm-mirror/clang/blob/master/tools/libclang/CIndex.cpp#L8713. Therefore I've used the same funcitonality.

Regarding pthread_setschedparam does setting prioirty to PTHREAD_MIN_PRIORITY on either SCHED_FIFO or SCHED_RR policy corresponds to SCHED_IDLE in linux(http://man7.org/linux/man-pages/man7/sched.7.html)?

kadircet updated this revision to Diff 187908.Feb 22 2019, 1:31 AM
kadircet marked an inline comment as done.
  • Add required header
kadircet marked an inline comment as done.Feb 22 2019, 1:32 AM
kadircet added inline comments.
clangd/Threading.cpp
127

It still says This document is a Mac OS X manual page. on top of the page, couldn't find any url that contains something like MacOS instead of iPhoneOS

gribozavr accepted this revision.Feb 22 2019, 4:33 AM

I see, there's no SCHED_IDLE in the macOS SDK. OK then, I trust that people who wrote CIndex.cpp support for macOS probably have got it correctly working for macOS, so it makes sense to do the same in clangd. Please consider factoring a function in libSupport though.

This revision is now accepted and ready to land.Feb 22 2019, 4:33 AM
This revision was automatically updated to reflect the committed changes.