Mozart: Improve tracing and backpressure.

services/gfx/compositor/backend/gpu_output.h

Index: services/gfx/compositor/backend/gpu_output.h
diff --git a/services/gfx/compositor/backend/gpu_output.h b/services/gfx/compositor/backend/gpu_output.h
index 8b0aa8782e281cb14c56aa297c394852fb5a9fb8..4b63b08cc02582869c855d25285a675279611a5f 100644
--- a/services/gfx/compositor/backend/gpu_output.h
+++ b/services/gfx/compositor/backend/gpu_output.h
@@ -12,19 +12,18 @@
 #include "base/callback.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
-#include "base/memory/scoped_ptr.h"
+#include "base/synchronization/waitable_event.h"
 #include "base/task_runner.h"
 #include "base/threading/thread.h"
 #include "mojo/services/gpu/interfaces/context_provider.mojom.h"
+#include "services/gfx/compositor/backend/gpu_rasterizer.h"
 #include "services/gfx/compositor/backend/output.h"
 #include "services/gfx/compositor/backend/vsync_scheduler.h"
 
 namespace compositor {
 
-class GpuRasterizer;
-
 // Renderer backed by a ContextProvider.
-class GpuOutput : public Output {
+class GpuOutput : public Output, private GpuRasterizer::Callbacks {
  public:
   GpuOutput(mojo::InterfaceHandle<mojo::ContextProvider> context_provider,
             const SchedulerCallbacks& scheduler_callbacks,
@@ -35,55 +34,70 @@ class GpuOutput : public Output {
   void SubmitFrame(const scoped_refptr<RenderFrame>& frame) override;
 
  private:
-  // Wrapper around state which is made available to the rasterizer thread.
-  class RasterizerDelegate {
-   public:
-    RasterizerDelegate();
-    ~RasterizerDelegate();
-
-    void PostInitialize(
-        mojo::InterfaceHandle<mojo::ContextProvider> context_provider,
-        const scoped_refptr<VsyncScheduler>& scheduler,
-        const scoped_refptr<base::TaskRunner>& task_runner,
-        const base::Closure& error_callback);
+  struct FrameData : public base::RefCountedThreadSafe<FrameData> {
+    FrameData(const scoped_refptr<RenderFrame>& frame, int64_t submit_time);
 
-    void PostDestroy(scoped_ptr<RasterizerDelegate> self);
+    void Recycle();
 
-    void PostFrame(const scoped_refptr<RenderFrame>& frame);
+    const scoped_refptr<RenderFrame> frame;
+    const int64_t submit_time;
+    int64_t draw_time;
+    int64_t wait_time;

[abarth, 2016/05/19 00:49:56]

Should these be base::TimeTicks?

[jeffbrown, 2016/05/20 01:09:47]

Perhaps.  Technically they are MojoTimeTicks but in the mojom files for
FrameInfo they appear as int64.  It was getting confusing to intermix these
types but perhaps I should be more rigorous.  I'll open an issue to look into it
later.

 
    private:
-    void PostSubmit();
+    friend class base::RefCountedThreadSafe<FrameData>;
 
-    // Called on rasterizer thread.
-    void InitializeTask(
-        mojo::InterfaceHandle<mojo::ContextProvider> context_provider,
-        const scoped_refptr<VsyncScheduler>& scheduler,
-        const scoped_refptr<base::TaskRunner>& task_runner,
-        const base::Closure& error_callback);
+    ~FrameData();
 
-    // Called on rasterizer thread.
-    void SubmitTask();
+    DISALLOW_COPY_AND_ASSIGN(FrameData);
+  };
 
-    // Called on rasterizer thread.
-    void OnFrameSubmitted(int64_t frame_time,
-                          int64_t presentation_time,
-                          int64_t submit_time,
-                          bool presented);
+  // |GpuRasterizer::Callbacks|:
+  void OnRasterizerReady(int64_t vsync_timebase,
+                         int64_t vsync_interval) override;
+  void OnRasterizerSuspended() override;
+  void OnRasterizerFinishedDraw(bool presented) override;
+  void OnRasterizerError() override;
 
-    std::unique_ptr<base::Thread> thread_;
-    scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
-    std::unique_ptr<GpuRasterizer> rasterizer_;
+  void ScheduleDrawLocked();
+  void OnDraw();
 
-    std::mutex mutex_;
-    std::queue<scoped_refptr<RenderFrame>> frames_;  // guarded by |mutex_|
+  void InitializeRasterizer(
+      mojo::InterfaceHandle<mojo::ContextProvider> context_provider);
+  void DestroyRasterizer();
+  void PostErrorCallback();
 
-    DISALLOW_COPY_AND_ASSIGN(RasterizerDelegate);
-  };
+  scoped_refptr<base::SingleThreadTaskRunner> compositor_task_runner_;

[mikejurka, 2016/05/19 23:26:19]

should we be using scoped_refptr or std::unique_ptr across mojo? they seem
equivalent and it seems like we should stick to one or the other, at least
within modules.

[jeffbrown, 2016/05/20 01:09:47]

A better substitute for scoped_refptr is std::shared_ptr though the C++
committee made some unfortunate design decisions that make it a bit less
attractive.  That said, we do prefer using std::unique_ptr over scoped_ptr (non
ref-counted) although there are some idiosyncratic cases where scoped_ptr must
be used for interoperation with other things in //base.

+  scoped_refptr<VsyncScheduler> vsync_scheduler_;
+  base::Closure error_callback_;
+
+  // Maximum number of frames to hold in the drawing pipeline.
+  // Any more than this and we start dropping them.
+  uint32_t pipeline_depth_;
+
+  // The rasterizer itself runs on its own thread.
+  std::unique_ptr<base::Thread> rasterizer_thread_;
+  scoped_refptr<base::SingleThreadTaskRunner> rasterizer_task_runner_;
+  base::WaitableEvent rasterizer_initialized_;
+  std::unique_ptr<GpuRasterizer> rasterizer_;
+
+  // Holds state shared between the compositor and rasterizer threads.
+  struct {
+    std::mutex mutex;  // guards all shared state
+
+    // The most recently submitted frame.
+    // Only null until the first frame has been submitted.
+    scoped_refptr<FrameData> current_frame_data;

[mikejurka, 2016/05/19 23:26:18]

If FrameData already inherits from RefCountedThreadSafe, do we need to wrap it
in yet another scoped_refptr?

[jeffbrown, 2016/05/20 01:09:47]

scoped_refptr is a smart pointer which holds a ref counted pointer so the two go
hand in hand.

+
+    // Frames drawn and awaiting completion by the rasterizer.
+    std::queue<scoped_refptr<FrameData>> drawn_frames_awaiting_finish;
+
+    // Set to true when the rasterizer is ready to draw.
+    bool rasterizer_ready = false;
 
-  scoped_refptr<VsyncScheduler> scheduler_;
-  scoped_ptr<RasterizerDelegate> rasterizer_delegate_;  // can't use unique_ptr
-                                                        // here due to
-                                                        // base::Bind (sadness)
+    // Set to true when a request to draw has been scheduled.
+    bool draw_scheduled = false;
+  } shared_state_;
 
   DISALLOW_COPY_AND_ASSIGN(GpuOutput);
 };