Task manager should support Idle Wakeups on Windows

chrome/browser/task_management/sampling/shared_sampler.h

Index: chrome/browser/task_management/sampling/shared_sampler.h
diff --git a/chrome/browser/task_management/sampling/shared_sampler.h b/chrome/browser/task_management/sampling/shared_sampler.h
new file mode 100644
index 0000000000000000000000000000000000000000..623c0ec0fd0b3c6a5148bd8fb93bafbb0c123102
--- /dev/null
+++ b/chrome/browser/task_management/sampling/shared_sampler.h
@@ -0,0 +1,133 @@
+// Copyright 2016 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.
+
+#ifndef CHROME_BROWSER_TASK_MANAGEMENT_SAMPLING_SHARED_SAMPLER_H_
+#define CHROME_BROWSER_TASK_MANAGEMENT_SAMPLING_SHARED_SAMPLER_H_
+
+#include <map>
+#include <memory>
+#include <utility>
+#include <vector>
+
+#include "base/callback.h"
+#include "base/files/file_path.h"
+#include "base/macros.h"
+#include "base/memory/ref_counted.h"
+#include "base/process/process_handle.h"
+#include "base/sequence_checker.h"
+#include "base/sequenced_task_runner.h"
+#include "build/build_config.h"
+
+namespace task_management {
+
+struct ProcessDataSnapshot;
+
+// Defines sampler that will calculate resources for all processes all at once,
+// on the worker thread. Created by TaskManagerImpl the UI thread, but used
+// mainly on a blocking pool thread.

[ncarter (slow), 2016/07/28 21:37:26]

PrivateWorkingSetSnapshot had the following comment; we should do something
similar here to explain why we need SharedSampler:

// This
// exists because on Windows it is much faster to collect a group of private
// working sets all at once using PdhOpenQuery than to calculate the private
// working sets for each process individually.

[stanisc, 2016/07/28 22:38:43]

Good idea!

[stanisc, 2016/08/01 22:34:26]

Done.

+class SharedSampler : public base::RefCountedThreadSafe<SharedSampler> {
+ public:
+  SharedSampler(

[ncarter (slow), 2016/07/28 21:37:26]

single-arg ctor should be explicit

[stanisc, 2016/08/01 22:34:26]

Done.

+      const scoped_refptr<base::SequencedTaskRunner>& blocking_pool_runner);
+
+  // The below are the types of the callbacks to the UI thread upon their
+  // corresponding refresh are done on the worker thread.
+  using OnIdleWakeupsCallback = base::Callback<void(int)>;
+
+  // Returns a combination of refresh flags supported by the shared sampler.
+  int64_t SupportsFlags() const;
+
+  // Registers task group specific callbacks.
+  void RegisterCallbacks(base::ProcessId process_id,
+                         const OnIdleWakeupsCallback& on_idle_wakeups);
+
+  // Unregisters task group specific callbacks.
+  void UnregisterCallbacks(base::ProcessId process_id);
+
+  // Refreshes the expensive process' stats (for now only idle wakeups per
+  // second) on the worker thread.
+  void Refresh(base::ProcessId process_id, int64_t refresh_flags);
+
+ private:
+  friend class base::RefCountedThreadSafe<SharedSampler>;
+  ~SharedSampler();
+
+#if defined(OS_WIN)
+  // The UI-thread callbacks in TaskGroup registered with RegisterCallbacks and
+  // to be called when refresh on the worker thread is done.
+  struct Callbacks {
+    Callbacks();
+    Callbacks(Callbacks&& other);

[ncarter (slow), 2016/07/28 21:37:26]

could this be = default?

That saves us from having to remember to update the move ctor body as new fields
are added.

[stanisc, 2016/07/28 22:38:43]

I tried '= default' but that still generated an error on win_clang.

[stanisc, 2016/08/01 22:34:26]

See win_clang error in patchset #5:
https://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds...

[ncarter (slow), 2016/08/03 22:28:55]

OK. Thanks for trying.

+    ~Callbacks();
+
+    OnIdleWakeupsCallback on_idle_wakeups;
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(Callbacks);
+  };
+
+  typedef std::map<base::ProcessId, Callbacks> CallbacksMap;
+
+  // Contains all results of refresh for a single process.
+  struct RefreshResult {
+    int idle_wakeups_per_second;
+  };
+
+  typedef std::map<base::ProcessId, RefreshResult> RefreshResultMap;

[ncarter (slow), 2016/07/28 21:37:26]

A vector would probably be faster.

[stanisc, 2016/07/28 22:38:43]

I thought about that but then I'd have to deal with merging two vectors in
OnRefreshDone - this one and the vector of per-process refresh flags.

Actually, according to your comment below per-process flags aren't necessary so
I could now change this into a vector.

[stanisc, 2016/08/01 22:34:26]

Done.

+
+  // Posted on the worker thread to do the actual refresh.
+  std::unique_ptr<RefreshResultMap> RefreshOnWorkerThread();
+
+  // Called on UI thread when the refresh is done.
+  void OnRefreshDone(std::unique_ptr<RefreshResultMap> refresh_results);
+
+  // Used to filter process information.
+  bool IsChromeImageName(const wchar_t* image_name) const;
+
+  // Captures a snapshot of data for all chrome processes.
+  // Runs on the worker thread.
+  std::unique_ptr<ProcessDataSnapshot> CaptureSnapshot();
+
+  // Produce refresh results by diffing two snapshots.
+  static void MakeResultsFromTwoSnapshots(
+      const ProcessDataSnapshot& prev_snapshot,
+      const ProcessDataSnapshot& snapshot,
+      RefreshResultMap* results);
+
+  // Produce refresh results from one snapshot.
+  // This is used only the first time when only one snapshot is available.
+  static void MakeResultsFromSnapshot(
+      const ProcessDataSnapshot& snapshot,
+      RefreshResultMap* results);
+
+  // Accumulates callbacks passed from TaskGroup objects passed via
+  // RegisterCallbacks calls.
+  CallbacksMap callbacks_map_;
+
+  // Accumulates per-process refresh flags passed via Refresh calls.
+  std::vector<std::pair<base::ProcessId, int64_t>> refresh_flags_;

[ncarter (slow), 2016/07/28 21:37:26]

I don't think we really need to track the refresh flags on a per-process basis.
The refresh flags are the union of the needs of all task manager observers at
the moment, but every observer is listening for info about all processes.

[stanisc, 2016/07/28 22:38:43]

I see. I was wondering about that. So basically all consecutive Refresh calls
should be made with the same flags. The implementation could simply DCHECK that.

[stanisc, 2016/08/01 22:34:26]

Done.

+
+  // Snapshot of previously captured resources used to calculate the delta.
+  std::unique_ptr<ProcessDataSnapshot> previous_snapshot_;
+
+  // Size of the buffer previously used to query system information.
+  uint32_t previous_buffer_size_;
+
+  // The current process image name for process filtering purposes.
+  const base::FilePath current_process_image_name_;

[ncarter (slow), 2016/07/28 21:37:26]

maybe this should be a vector of FilePaths, which contains both
current_process_image_name and also nacl64?

[stanisc, 2016/07/28 22:38:43]

Hmm... The implementation would have to iterate over an array which would be
slower than the current hardcoded version. But I have no problem doing this if
you think that is a good idea.

[stanisc, 2016/08/01 22:34:26]

Done.

+
+  // The specific blocking pool SequencedTaskRunner that will be used to post
+  // the refresh tasks onto serially.
+  scoped_refptr<base::SequencedTaskRunner> blocking_pool_runner_;
+
+  // To assert we're running on the correct thread.
+  base::SequenceChecker worker_pool_sequenced_checker_;
+#endif  // defined(OS_WIN)
+
+  DISALLOW_COPY_AND_ASSIGN(SharedSampler);
+};
+
+}  // namespace task_management
+
+#endif  // CHROME_BROWSER_TASK_MANAGEMENT_SAMPLING_SHARED_SAMPLER_H_