Refactoring of job dispatch in HostResolverImpl in preparation for DnsClient.

net/base/prioritized_dispatcher.cc

Index: net/base/prioritized_dispatcher.cc
diff --git a/net/base/prioritized_dispatcher.cc b/net/base/prioritized_dispatcher.cc
new file mode 100644
index 0000000000000000000000000000000000000000..1e479ad71801559941b0d6be7b00159271f9844d
--- /dev/null
+++ b/net/base/prioritized_dispatcher.cc
@@ -0,0 +1,105 @@
+// Copyright (c) 2011 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.
+
+#include "net/base/prioritized_dispatcher.h"
+
+#include "base/logging.h"
+
+namespace net {
+
+PrioritizedDispatcher::PrioritizedDispatcher(size_t num_priorities,
+                                             const Limits& limits)
+    : queue_(num_priorities),
+      max_running_jobs_(num_priorities),
+      num_running_jobs_(0) {
+  // No reserved slots beyond num_priorities are allowed.
+  for (size_t i = num_priorities; i < limits.reserved_slots.size(); ++i) {

[cbentzel, 2012/01/05 18:32:20]

Why aren't reserved_slots.size() and num_priorities required to be the same? 

[szym, 2012/01/05 19:00:29]

This would require callers of HostResolverImpl() to know NUM_PRIORITIES.

+    DCHECK_EQ(0u, limits.reserved_slots[i]);
+  }
+  size_t total = 0;
+  for (size_t i = std::min(num_priorities, limits.reserved_slots.size());
+      i > 0; --i) {
+    total += limits.reserved_slots[i - 1];
+    max_running_jobs_[i - 1] = total;
+  }
+  // Unreserved slots are available for all priorities.
+  DCHECK_LE(total, limits.total_jobs) << "sum(reserved_slots) <= total_jobs";
+  size_t spare = limits.total_jobs - total;
+  for (size_t i = 0; i < num_priorities; ++i) {
+    max_running_jobs_[i] += spare;
+  }
+}
+
+PrioritizedDispatcher::~PrioritizedDispatcher() {}
+
+PrioritizedDispatcher::Handle PrioritizedDispatcher::Add(
+    Job* job, size_t priority) {
+  DCHECK(job);
+  DCHECK_LT(priority, num_priorities());
+  if (num_running_jobs_ < max_running_jobs_[priority]) {
+    ++num_running_jobs_;
+    job->Start();

[cbentzel, 2012/01/05 18:32:20]

It looks like there are no re-entrancy problems if OnJobFinished is called
within Start (both here, and inside of DispatchJob). Perhaps worth mentioning.

[szym, 2012/01/05 19:00:29]

Already mentioned.

+    return Handle();
+  }
+  return queue_.Insert(job, priority);
+}
+
+void PrioritizedDispatcher::Cancel(const Handle& handle) {
+  queue_.Erase(handle);
+}
+
+PrioritizedDispatcher::Job* PrioritizedDispatcher::EvictOldestLowest() {
+  Handle handle = queue_.FirstMax();
+  if (handle.is_null())
+    return NULL;
+  Job* job = handle.value();
+  Cancel(handle);
+  return job;
+}
+
+PrioritizedDispatcher::Handle PrioritizedDispatcher::Update(
+    const Handle& handle, size_t priority) {
+  DCHECK(!handle.is_null());
+  DCHECK_LT(priority, num_priorities());
+  DCHECK_GE(num_running_jobs_, max_running_jobs_[handle.priority()]) <<
+      "Job should not be in queue when limits permit it to start.";
+  DCHECK_LE(queue_.size(), max_queued_jobs_);
+
+  if (handle.priority() == priority)
+    return handle;
+
+  if (DispatchJob(handle, priority))
+    return Handle();
+  Job* job = handle.value();
+  queue_.Erase(handle);
+  return queue_.Insert(job, priority);
+}
+
+void PrioritizedDispatcher::OnJobFinished() {
+  DCHECK_GT(num_running_jobs_, 0u);
+  --num_running_jobs_;
+  Handle handle = queue_.FirstMin();
+  if (handle.is_null()) {
+    DCHECK_EQ(0u, queue_.size());
+    return;
+  }
+  DispatchJob(handle, handle.priority());
+}
+
+bool PrioritizedDispatcher::DispatchJob(const Handle& handle,
+                                        size_t job_priority) {
+  DCHECK_LT(job_priority, num_priorities());
+  if (num_running_jobs_ >= max_running_jobs_[job_priority])
+    return false;
+  Job* job = handle.value();
+  queue_.Erase(handle);
+  ++num_running_jobs_;
+  DCHECK(job);

[cbentzel, 2012/01/05 18:32:20]

This DCHECK isn't needed - you check it at Add time which is more appropriate.

+  job->Start();
+  return true;
+}
+
+}  // namespace net
+
+