base: Support setting thread priorities generically.

base/threading/platform_thread.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // WARNING: You should *NOT* be using this class directly.  PlatformThread is
 // the low-level platform-specific abstraction to the OS's threading interface.
 // You should instead be using a message-loop driven Thread, see thread.h.
 
 #ifndef BASE_THREADING_PLATFORM_THREAD_H_
 #define BASE_THREADING_PLATFORM_THREAD_H_
@@ skipping 25 matching lines @@
 const PlatformThreadHandle kNullThreadHandle = 0;
 typedef pid_t PlatformThreadId;
 #endif
 
 const PlatformThreadId kInvalidThreadId = 0;
 
 // Valid values for SetThreadPriority()
 enum ThreadPriority{
   kThreadPriority_Normal,
   // Suitable for low-latency, glitch-resistant audio.
-  kThreadPriority_RealtimeAudio
+  kThreadPriority_RealtimeAudio,
+  kThreadPriority_Display,

[jar (doing other things), 2013/04/24 01:04:57]

Although I can guess what Normal means, vs Background, I don't know how to fit
in Display and RealtimeAudio.

Can you at a minimum add comments, and probably better would be to also lengthen
the name so that its significance would be clear at point of use (without
reading the comments).

[epenner, 2013/04/30 04:37:08]

Done.

+  kThreadPriority_Background
 };
 
 // A namespace for low-level thread functions.
 class BASE_EXPORT PlatformThread {
  public:
   // Implement this interface to run code on a background thread.  Your
   // ThreadMain method will be called on the newly created thread.
   class BASE_EXPORT Delegate {
    public:
     virtual void ThreadMain() = 0;
@@ skipping 42 matching lines @@
   // CreateNonJoinable() does the same thing as Create() except the thread
   // cannot be Join()'d.  Therefore, it also does not output a
   // PlatformThreadHandle.
   static bool CreateNonJoinable(size_t stack_size, Delegate* delegate);
 
   // Joins with a thread created via the Create function.  This function blocks
   // the caller until the designated thread exits.  This will invalidate
   // |thread_handle|.
   static void Join(PlatformThreadHandle thread_handle);
 
-  // Sets the priority of the thread specified in |handle| to |priority|.
-  // This does not work on Linux, use CreateWithPriority() instead.
+  // Only one of the handle/id is required depending on the
+  // platform. However, for cross platform code, pass both.
   static void SetThreadPriority(PlatformThreadHandle handle,
+                                PlatformThreadId id,

[jar (doing other things), 2013/04/24 01:04:57]

Rather than having both, can you unify this in a class some how?  What is the
significance of listing both in call??

[epenner, 2013/04/30 04:37:08]

See also my previous general comment on the patch.

The closest I think would be trying to store the ID inside the Handle itself, at
thread creation time. Perhaps the ID could then be removed entirely from the
PlatformThread abstraction. The trouble is that this imposes limits on when
priorities can be set on certain platforms.

Specifically, I think you would need to have created the thread yourself in
order to set it's priority (on all platforms), even though this is only a real
limitation on windows (IIUC).

                                 ThreadPriority priority);
-

[jar (doing other things), 2013/04/24 01:04:57]

nit: preserve the white-space line before the private keyword.  I couldn't find
explicit support in the style guide, but *all* the examples they provide had a
blank line before it.... so you should probably at least keep it here.

[epenner, 2013/04/30 04:37:08]

Done.

  private:
   DISALLOW_IMPLICIT_CONSTRUCTORS(PlatformThread);
 };
 
 }  // namespace base
 
 #endif  // BASE_THREADING_PLATFORM_THREAD_H_