summaryrefslogtreecommitdiffstats
path: root/chrome/browser/net/dns_master.h
diff options
context:
space:
mode:
authorphajdan.jr@chromium.org <phajdan.jr@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2009-02-19 18:23:25 +0000
committerphajdan.jr@chromium.org <phajdan.jr@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2009-02-19 18:23:25 +0000
commit1933eb200c4bb16e4df535d779c98561f85c1e25 (patch)
tree0ec1fe2dc176b2159abf8e33c53eaf940fb310c4 /chrome/browser/net/dns_master.h
parentfd599eeeddf2a57a2b2d70e63725106ce93c4591 (diff)
downloadchromium_src-1933eb200c4bb16e4df535d779c98561f85c1e25.zip
chromium_src-1933eb200c4bb16e4df535d779c98561f85c1e25.tar.gz
chromium_src-1933eb200c4bb16e4df535d779c98561f85c1e25.tar.bz2
Clean up dns prefetch code, and also port it.
- remove slave threads and use HostResolver in asynchronous mode instead (while still limiting number of concurrent lookups) - make the implementation portable and make DnsMaster unit test compile and pass on Linux - add more tests to DnsMaster unit test to simulate various shutdown scenarios, concurrent lookups, and to verify that we don't exceed our limit of concurrent lookup requests) - remove some tests which relied on specifics of slaves' inner working - adjust initialization and shutdown of dns prefetching (now it relies on the IO message loop being present) Bonus: shutdown is almost instant now, no need to have a timeout. BUG=5687, 6683 Review URL: http://codereview.chromium.org/15076 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@10021 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome/browser/net/dns_master.h')
-rw-r--r--chrome/browser/net/dns_master.h157
1 files changed, 56 insertions, 101 deletions
diff --git a/chrome/browser/net/dns_master.h b/chrome/browser/net/dns_master.h
index 72116cd..8018e26 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.
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_;
generated by cgit v1.1 at 2025-03-05 19:07:13 +0000