Index: chrome/browser/net/dns_master.h |
diff --git a/chrome/browser/net/dns_master.h b/chrome/browser/net/dns_master.h |
index 72116cdac48b7de71dfa0ae269b677839f12d250..8018e26857dba21685ca9b063775493778e800f2 100644 |
--- a/chrome/browser/net/dns_master.h |
+++ b/chrome/browser/net/dns_master.h |
@@ -3,59 +3,45 @@ |
// found in the LICENSE file. |
// A DnsMaster object is instantiated once in the browser |
-// process, and delivers DNS prefetch assignments (hostnames) |
-// to any of several DnsSlave objects. |
+// process, and manages asynchronous resolution of DNS hostnames. |
// Most hostname lists are sent out by renderer processes, and |
// involve lists of hostnames that *might* be used in the near |
// future by the browsing user. The goal of this class is to |
// cause the underlying DNS structure to lookup a hostname before |
// it is really needed, and hence reduce latency in the standard |
-// lookup paths. Since some DNS lookups may take a LONG time, we |
-// use several DnsSlave threads to concurrently perform the |
-// lookups. |
+// lookup paths. |
#ifndef CHROME_BROWSER_NET_DNS_MASTER_H_ |
#define CHROME_BROWSER_NET_DNS_MASTER_H_ |
#include <map> |
#include <queue> |
+#include <set> |
#include <string> |
-#include "base/condition_variable.h" |
-#include "base/scoped_ptr.h" |
-#include "base/values.h" |
+#include "base/lock.h" |
#include "chrome/browser/net/dns_host_info.h" |
#include "chrome/browser/net/referrer.h" |
#include "chrome/common/net/dns.h" |
#include "googleurl/src/url_canon.h" |
+#include "testing/gtest/include/gtest/gtest_prod.h" |
namespace chrome_browser_net { |
-class DnsSlave; |
- |
typedef chrome_common_net::NameList NameList; |
typedef std::map<std::string, DnsHostInfo> Results; |
class DnsMaster { |
public: |
- // The number of slave processes that will do DNS prefetching |
- static const size_t kSlaveCountMax = 8; |
- |
- explicit DnsMaster(base::TimeDelta shutdown_wait_time); |
+ // Too many concurrent lookups negate benefits of prefetching |
+ // by trashing the OS cache. |
+ static const size_t kMaxConcurrentLookups; |
- ~DnsMaster() { |
- if (!shutdown_) |
- ShutdownSlaves(); // Ensure we did our cleanup. |
- } |
+ DnsMaster(); |
+ ~DnsMaster(); |
- // ShutdownSlaves() gets all spawned threads to terminate, closes |
- // their handles, and deletes their DnsSlave instances. |
- // Return value of true means all operations succeeded. |
- // Return value of false means that the threads wouldn't terminate, |
- // and that resources may leak. If this returns false, it is best |
- // to NOT delete this DnsMaster, as slave threads may still call into |
- // this object. |
- bool ShutdownSlaves(); |
+ // Cancel pending requests and prevent new ones from being made. |
+ void Shutdown(); |
// In some circumstances, for privacy reasons, all results should be |
// discarded. This method gracefully handles that activity. |
@@ -64,7 +50,7 @@ class DnsMaster { |
// (cache hits etc.). |
void DiscardAllResults(); |
- // Add hostname(s) to the queue for processing by slaves |
+ // Add hostname(s) to the queue for processing. |
void ResolveList(const NameList& hostnames, |
DnsHostInfo::ResolutionMotivation motivation); |
void Resolve(const std::string& hostname, |
@@ -89,36 +75,6 @@ class DnsMaster { |
// domains. |
void GetHtmlInfo(std::string* output); |
- // For testing only... |
- // Currently testing only provides a crude measure of success. |
- bool WasFound(const std::string& hostname) { |
- AutoLock auto_lock(lock_); |
- return (results_.find(hostname) != results_.end()) && |
- results_[hostname].was_found(); |
- } |
- |
- // Accessor methods, used mostly for testing. |
- // Both functions return DnsHostInfo::kNullDuration if name was not yet |
- // processed enough. |
- base::TimeDelta GetResolutionDuration(const std::string hostname) { |
- AutoLock auto_lock(lock_); |
- if (results_.find(hostname) == results_.end()) |
- return DnsHostInfo::kNullDuration; |
- return results_[hostname].resolve_duration(); |
- } |
- |
- base::TimeDelta GetQueueDuration(const std::string hostname) { |
- AutoLock auto_lock(lock_); |
- if (results_.find(hostname) == results_.end()) |
- return DnsHostInfo::kNullDuration; |
- return results_[hostname].queue_duration(); |
- } |
- |
- size_t running_slave_count() { |
- AutoLock auto_lock(lock_); |
- return running_slave_count_; |
- } |
- |
// Discard any referrer for which all the suggested host names are currently |
// annotated with no user latency reduction. Also scale down (diminish) the |
// total benefit of those that did help, so that their reported contribution |
@@ -135,42 +91,58 @@ class DnsMaster { |
// values into the current referrer list. |
void DeserializeReferrers(const ListValue& referral_list); |
- //---------------------------------------------------------------------------- |
- // Methods below this line should only be called by slave processes. |
- |
- // GetNextAssignment() gets the next hostname from queue for processing |
- // It is not meant to be public, and should only be used by the slave. |
- // GetNextAssignment() waits on a condition variable if there are no more |
- // names in queue. |
- // Return false if slave thread should terminate. |
- // Return true if slave thread should process the value. |
- bool GetNextAssignment(std::string* hostname); |
- |
- // Access methods for use by slave threads to callback with state updates. |
- void SetFoundState(const std::string hostname); |
- void SetNoSuchNameState(const std::string hostname); |
+ private: |
+ FRIEND_TEST(DnsMasterTest, BenefitLookupTest); |
+ FRIEND_TEST(DnsMasterTest, ShutdownWhenResolutionIsPendingTest); |
+ FRIEND_TEST(DnsMasterTest, SingleLookupTest); |
+ FRIEND_TEST(DnsMasterTest, ConcurrentLookupTest); |
+ FRIEND_TEST(DnsMasterTest, MassiveConcurrentLookupTest); |
+ friend class WaitForResolutionHelper; // For testing. |
- // Notification during ShutdownSlaves. |
- void SetSlaveHasTerminated(int slave_index); |
+ class LookupRequest; |
- private: |
// A map that is keyed with the hostnames that we've learned were the cause |
// of loading additional hostnames. The list of additional hostnames in held |
// in a Referrer instance, which is found in this type. |
typedef std::map<std::string, Referrer> Referrers; |
+ // Only for testing. Returns true if hostname has been successfully resolved |
+ // (name found). |
+ bool WasFound(const std::string& hostname) { |
+ AutoLock auto_lock(lock_); |
+ return (results_.find(hostname) != results_.end()) && |
+ results_[hostname].was_found(); |
+ } |
+ |
+ // Only for testing. Return how long was the resolution |
+ // or DnsHostInfo::kNullDuration if it hasn't been resolved yet. |
+ base::TimeDelta GetResolutionDuration(const std::string& hostname) { |
+ AutoLock auto_lock(lock_); |
+ if (results_.find(hostname) == results_.end()) |
+ return DnsHostInfo::kNullDuration; |
+ return results_[hostname].resolve_duration(); |
+ } |
+ |
+ // Only for testing; |
+ size_t peak_pending_lookups() const { return peak_pending_lookups_; } |
+ |
+ // Access method for use by lookup request to pass resolution result. |
+ void OnLookupFinished(LookupRequest* request, |
+ const std::string& hostname, bool found); |
+ |
// "PreLocked" means that the caller has already Acquired lock_ in the |
// following method names. |
// Queue hostname for resolution. If queueing was done, return the pointer |
// to the queued instance, otherwise return NULL. |
DnsHostInfo* PreLockedResolve(const std::string& hostname, |
DnsHostInfo::ResolutionMotivation motivation); |
- bool PreLockedCreateNewSlaveIfNeeded(); // Lazy slave processes creation. |
- // Number of slave processes started early (to help with startup prefetch). |
- static const size_t kSlaveCountMin = 4; |
+ // Take lookup requests from name_buffer_ and tell HostResolver |
+ // to look them up asynchronously, provided we don't exceed |
+ // concurrent resolution limit. |
+ void PreLockedScheduleLookups(); |
- // Synchronize access to results_, referrers_, and slave control data. |
+ // Synchronize access to variables listed below. |
darin (slow to review)
2009/02/18 21:49:17
looks like a good comment to me.
|
Lock lock_; |
// name_buffer_ holds a list of names we need to look up. |
@@ -184,30 +156,13 @@ class DnsMaster { |
// pre-resolve when there is a navigation to the orginial hostname. |
Referrers referrers_; |
- // Signaling slaves to process elements in the queue, or to terminate, |
- // is done using ConditionVariables. |
- ConditionVariable slaves_have_work_; |
- |
- size_t slave_count_; // Count of slave processes started. |
- size_t running_slave_count_; // Count of slaves process still running. |
- |
- // TODO(jrg): wait for CL 15076 from _ph to come in which resolves |
- // this. In the short term this file is hacked to be happy when |
- // included in render_process.h. |
-#if defined(OS_WIN) |
- // The following arrays are only initialized as |
- // slave_count_ grows (up to the indicated max). |
- DWORD thread_ids_[kSlaveCountMax]; |
- HANDLE thread_handles_[kSlaveCountMax]; |
- DnsSlave* slaves_[kSlaveCountMax]; |
-#endif |
- |
- // shutdown_ is set to tell the slaves to terminate. |
- bool shutdown_; |
+ std::set<LookupRequest*> pending_lookups_; |
- // The following is the maximum time the ShutdownSlaves method |
- // will wait for all the slave processes to terminate. |
- const base::TimeDelta kShutdownWaitTime_; |
+ // For testing, to verify that we don't exceed the limit. |
+ size_t peak_pending_lookups_; |
+ |
+ // When true, we don't make new lookup requests. |
+ bool shutdown_; |
// A list of successful events resulting from pre-fetching. |
DnsHostInfo::DnsInfoTable cache_hits_; |