[PATCH] D124715: Use QoS class Utility for ThreadPriority::Low on Mac

stefanhaller updated this revision to Diff 427577. stefanhaller added a comment. - Added a comment to setThreadBackgroundPriority - Improved comments for ThreadPriority enum - Add FIXME comments for Windows and Linux Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D124715: Use QoS class Utility for ThreadPriority::Low on Mac

sammccall accepted this revision. sammccall added a comment. Thanks a lot, this version looks good to me and I'll follow up with a flag for clangd. In D124715#3493493 , @stefanhaller wrote: > In D124715#3491985

[PATCH] D124715: Use QoS class Utility for ThreadPriority::Low on Mac

stefanhaller updated this revision to Diff 427567. stefanhaller added a comment. Updated the patch to have both Background and Low As discussed, we provide both Background and Low now, with Low being the default; Linux and Windows still implement both the same (as Background), this could be

[PATCH] D124715: Use QoS class Utility for ThreadPriority::Low on Mac

kadircet added a comment. I agree that clangd's indexing should probably be higher priority than whatever FS indexing is going on (because clangd's indexing only start when user actually starts working on a project, hence this indicates some level of "interactiveness"). so I feel like the

[PATCH] D124715: Use QoS class Utility for ThreadPriority::Low on Mac

stefanhaller added a comment. In D124715#3491985 , @sammccall wrote: > I'm still concerned some users will find this a large regression and we won't > have a good workaround: > > - it'll use a lot more battery than before On Intel Macs, I'm not sure

[PATCH] D124715: Use QoS class Utility for ThreadPriority::Low on Mac

sammccall added a comment. OK, so wanting this to be utility makes sense, and maybe/probably it should be the default. I'm still concerned some users will find this a large regression and we won't have a good workaround: - it'll use a lot more battery than before - not everyone will see the

[PATCH] D124715: Use QoS class Utility for ThreadPriority::Low on Mac

stefanhaller added a comment. In D124715#3491782 , @akyrtzi wrote: > That said, you may want to consider dynamically switching to background if > running on laptop with battery, or other heuristics, My take on this is that it would be Apple's

[PATCH] D124715: Use QoS class Utility for ThreadPriority::Low on Mac

akyrtzi added a comment. My recommendation is that indexing belongs to 'utility', as @stefanhaller mentioned the user is actively depending on functionality coming from the index. That said, you may want to consider dynamically switching to background if running on laptop with battery, or other

[PATCH] D124715: Use QoS class Utility for ThreadPriority::Low on Mac

dgoldman added a comment. In D124715#3490364 , @stefanhaller wrote: > In D124715#3488447 , @sammccall > wrote: > >> (How) does this interact with battery vs mains power on laptops? >> It seems like there's a

[PATCH] D124715: Use QoS class Utility for ThreadPriority::Low on Mac

stefanhaller added a comment. In D124715#3488447 , @sammccall wrote: > (How) does this interact with battery vs mains power on laptops? > It seems like there's a common scenario where: > > - the user is on a relatively slow laptop, running off battery >

[PATCH] D124715: Use QoS class Utility for ThreadPriority::Low on Mac

sammccall added a comment. > With this priority, the thread is > confined to efficiency cores only, which makes background indexing take > forever (How) does this interact with battery vs mains power on laptops? It seems like there's a common scenario where: - the user is on a relatively slow

[PATCH] D124715: Use QoS class Utility for ThreadPriority::Low on Mac

dgoldman accepted this revision. dgoldman added a comment. This revision is now accepted and ready to land. In D124715#3484058 , @stefanhaller wrote: > Fix documentation of ThreadPriority::Low (was: Background), and move it up > from >

[PATCH] D124715: Use QoS class Utility for ThreadPriority::Low on Mac

stefanhaller updated this revision to Diff 426233. stefanhaller added a comment. Fix documentation of ThreadPriority::Low (was: Background), and move it up from set_thread_priority to ThreadPriority. (Is it ok that there's no documentation left for set_thread_priority? I couldn't come up with

[PATCH] D124715: Use QoS class Utility for ThreadPriority::Low on Mac

dgoldman added inline comments. Comment at: llvm/include/llvm/Support/Threading.h:239 }; /// If priority is Background tries to lower current threads priority such /// that it does not affect foreground tasks significantly. Can be used for Should be

[PATCH] D124715: Use QoS class Utility for ThreadPriority::Low on Mac

stefanhaller created this revision. Herald added subscribers: dexonsmith, usaxena95, kadircet, arphaman, hiraditya. Herald added a project: All. stefanhaller requested review of this revision. Herald added subscribers: cfe-commits, llvm-commits, ilya-biryukov. Herald added projects: clang, LLVM,