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

[Dmitri Gribenko via Phabricator via llvm-commits]

gribozavr accepted this revision.
gribozavr added a comment.
This revision is now accepted and ready to land.

In D59130#1426403 <https://reviews.llvm.org/D59130#1426403>, @jfb wrote:

> 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


There is a reasonable limit to portability.  For example, someone might come along and object to filesystem APIs that traverse directories on the basis that there can be a system that does not support nested directories, or uses a content-addressable filesystem.  (Both of those exist in a real world BTW).  However, that is not a valid basis for not adding such APIs.

In D59130#1426403 <https://reviews.llvm.org/D59130#1426403>, @jfb wrote:

> This seems like something you'd want to do for an indexing process, not just a thread?


It depends on the indexing architecture.  libclang and clangd's background indexing run indexing in process of the language server, while Apple's (to-be-upstreamed) index-while-building uses indexing in-process of the compiler, out-of-process of the language server.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59130