[PATCH] D59130: [llvm][Support] Provide interface to set thread priorities

Tue Mar 12 09:44:14 PDT 2019

jfb added subscribers: jkorous, arphaman.
jfb added a comment.

In D59130#1426086 <https://reviews.llvm.org/D59130#1426086>, @gribozavr wrote:

> In D59130#1424778 <https://reviews.llvm.org/D59130#1424778>, @jfb wrote:
>
> > That doesn't answer my question though: if someone sets priority and on the platform the test witness acceptable forward progress, will it happen that another platform provide no forward progress for the same work. For example, some implementations don't let a background thread perform any I/O. In such a case, a mere logging statement would halt the background thread's progress.
>
>
> Fair enough.  But is this a theoretical concern for some abstract platform that might implement this function in future, or is it a concrete concern for one of the implementations in this review?

You're adding a "portable" abstraction to different platform APIs. The onus here is on you to make sure that the abstraction is portable, and to point at documentation which says that each platform you're adding support for has the same behavior. Ideally you'd also have a test which shows that a background thread won't indefinitely hang when this is used, but that's not really feasible IMO. If there are caveats between different platforms, then your new API should document said caveat (e.g. "if you do I/O in such and such manner, the thread many not get any progress if the rest of the application is doing such and such other thing").

My worry is that your new API gets used, and eventually we get bug reports about things mysteriously hanging because people added features to a thread, not knowing the semantics that the API gave to that thread.

> The desired semantics for our use cases (see below) is to merely lower the CPU and I/O priority, where possible, so that heavy background work does not degrade the interactive user experience serviced by the threads in the same process.
> 
>> What threads does LLVM have that require background priority?
> 
> Source code indexing in libclang and clangd are the two use cases that we are interested in.

This seems like something you'd want to do for an indexing process, not just a thread? Maybe I misunderstand the setup, but if it's for indexing I'd like @arphaman and @jkorous to chime in.

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D59130/new/

https://reviews.llvm.org/D59130