diff options
| -rw-r--r-- | net/base/host_resolver.h | 10 | ||||
| -rw-r--r-- | net/base/host_resolver_impl.cc | 298 | ||||
| -rw-r--r-- | net/base/host_resolver_impl.h | 65 | ||||
| -rw-r--r-- | net/base/host_resolver_impl_unittest.cc | 277 | ||||
| -rw-r--r-- | net/base/mock_host_resolver.cc | 2 | ||||
| -rw-r--r-- | net/base/net_error_list.h | 4 | ||||
| -rw-r--r-- | net/base/request_priority.h | 3 | ||||
| -rw-r--r-- | net/http/http_network_transaction.cc | 3 | ||||
| -rw-r--r-- | net/tools/hresolv/hresolv.cc | 2 |
9 files changed, 634 insertions, 30 deletions
diff --git a/net/base/host_resolver.h b/net/base/host_resolver.h index 8a88402..0bcbbe8 100644 --- a/net/base/host_resolver.h +++ b/net/base/host_resolver.h @@ -11,6 +11,7 @@ #include "googleurl/src/gurl.h" #include "net/base/address_family.h" #include "net/base/completion_callback.h" +#include "net/base/request_priority.h" class MessageLoop; @@ -40,7 +41,8 @@ class HostResolver : public base::RefCountedThreadSafe<HostResolver> { address_family_(ADDRESS_FAMILY_UNSPECIFIED), port_(port), allow_cached_response_(true), - is_speculative_(false) {} + is_speculative_(false), + priority_(MEDIUM) {} const int port() const { return port_; } const std::string& hostname() const { return hostname_; } @@ -56,6 +58,9 @@ class HostResolver : public base::RefCountedThreadSafe<HostResolver> { bool is_speculative() const { return is_speculative_; } void set_is_speculative(bool b) { is_speculative_ = b; } + RequestPriority priority() const { return priority_; } + void set_priority(RequestPriority priority) { priority_ = priority; } + const GURL& referrer() const { return referrer_; } void set_referrer(const GURL& referrer) { referrer_ = referrer; } @@ -75,6 +80,9 @@ class HostResolver : public base::RefCountedThreadSafe<HostResolver> { // Whether this request was started by the DNS prefetcher. bool is_speculative_; + // The priority for the request. + RequestPriority priority_; + // Optional data for consumption by observers. This is the URL of the // page that lead us to the navigation, for DNS prefetcher's benefit. GURL referrer_; diff --git a/net/base/host_resolver_impl.cc b/net/base/host_resolver_impl.cc index 5644223..b81b4cb 100644 --- a/net/base/host_resolver_impl.cc +++ b/net/base/host_resolver_impl.cc @@ -4,6 +4,10 @@ #include "net/base/host_resolver_impl.h" +#include <cmath> +#include <deque> + +#include "base/basictypes.h" #include "base/compiler_specific.h" #include "base/debug_util.h" #include "base/message_loop.h" @@ -39,8 +43,15 @@ HostCache* CreateDefaultCache() { } // anonymous namespace HostResolver* CreateSystemHostResolver() { + // Maximum of 50 concurrent threads. + // TODO(eroman): Adjust this, do some A/B experiments. + static const size_t kMaxJobs = 50u; + // TODO(willchan): Pass in the NetworkChangeNotifier. - return new HostResolverImpl(NULL, CreateDefaultCache(), NULL); + HostResolverImpl* resolver = new HostResolverImpl( + NULL, CreateDefaultCache(), NULL, kMaxJobs); + + return resolver; } static int ResolveAddrInfo(HostResolverProc* resolver_proc, @@ -212,6 +223,13 @@ class HostResolverImpl::Job return requests_; } + // Returns the first request attached to the job. + const Request* initial_request() const { + DCHECK_EQ(origin_loop_, MessageLoop::current()); + DCHECK(!requests_.empty()); + return requests_[0]; + } + private: friend class base::RefCountedThreadSafe<HostResolverImpl::Job>; @@ -289,16 +307,172 @@ class HostResolverImpl::Job //----------------------------------------------------------------------------- +// We rely on the priority enum values being sequential having starting at 0, +// and increasing for lower priorities. +COMPILE_ASSERT(HIGHEST == 0u && + LOWEST > HIGHEST && + NUM_PRIORITIES > LOWEST, + priority_indexes_incompatible); + +// JobPool contains all the information relating to queued requests, including +// the limits on how many jobs are allowed to be used for this category of +// requests. +class HostResolverImpl::JobPool { + public: + JobPool(size_t max_outstanding_jobs, size_t max_pending_requests) + : num_outstanding_jobs_(0u) { + SetConstraints(max_outstanding_jobs, max_pending_requests); + } + + ~JobPool() { + // Free the pending requests. + for (size_t i = 0; i < arraysize(pending_requests_); ++i) + STLDeleteElements(&pending_requests_[i]); + } + + // Sets the constraints for this pool. See SetPoolConstraints() for the + // specific meaning of these parameters. + void SetConstraints(size_t max_outstanding_jobs, + size_t max_pending_requests) { + CHECK(max_outstanding_jobs != 0u); + max_outstanding_jobs_ = max_outstanding_jobs; + max_pending_requests_ = max_pending_requests; + } + + // Returns the number of pending requests enqueued to this pool. + // A pending request is one waiting to be attached to a job. + size_t GetNumPendingRequests() const { + size_t total = 0u; + for (size_t i = 0u; i < arraysize(pending_requests_); ++i) + total += pending_requests_[i].size(); + return total; + } + + bool HasPendingRequests() const { + return GetNumPendingRequests() > 0u; + } + + // Enqueues a request to this pool. As a result of enqueing this request, + // the queue may have reached its maximum size. In this case, a request is + // evicted from the queue, and returned. Otherwise returns NULL. The caller + // is responsible for freeing the evicted request. + Request* InsertPendingRequest(Request* req) { + PendingRequestsQueue& q = pending_requests_[req->info().priority()]; + q.push_back(req); + + // If the queue is too big, kick out the lowest priority oldest request. + if (GetNumPendingRequests() > max_pending_requests_) { + // Iterate over the queues from lowest priority to highest priority. + for (int i = static_cast<int>(arraysize(pending_requests_)) - 1; + i >= 0; --i) { + PendingRequestsQueue& q = pending_requests_[i]; + if (!q.empty()) { + Request* req = q.front(); + q.pop_front(); + return req; + } + } + } + + return NULL; + } + + // Erases |req| from this container. Caller is responsible for freeing + // |req| afterwards. + void RemovePendingRequest(Request* req) { + PendingRequestsQueue& q = pending_requests_[req->info().priority()]; + PendingRequestsQueue::iterator it = std::find(q.begin(), q.end(), req); + DCHECK(it != q.end()); + q.erase(it); + } + + // Removes and returns the highest priority pending request. + Request* RemoveTopPendingRequest() { + DCHECK(HasPendingRequests()); + + for (size_t i = 0u; i < arraysize(pending_requests_); ++i) { + PendingRequestsQueue& q = pending_requests_[i]; + if (!q.empty()) { + Request* req = q.front(); + q.pop_front(); + return req; + } + } + + NOTREACHED(); + return NULL; + } + + // Keeps track of a job that was just added/removed, and belongs to this pool. + void AdjustNumOutstandingJobs(int offset) { + DCHECK(offset == 1 || (offset == -1 && num_outstanding_jobs_ > 0u)); + num_outstanding_jobs_ += offset; + } + + // Returns true if a new job can be created for this pool. + bool CanCreateJob() const { + return num_outstanding_jobs_ + 1u <= max_outstanding_jobs_; + } + + // Removes any pending requests from the queue which are for the + // same hostname/address-family as |job|, and attaches them to |job|. + void MoveRequestsToJob(Job* job) { + for (size_t i = 0u; i < arraysize(pending_requests_); ++i) { + PendingRequestsQueue& q = pending_requests_[i]; + PendingRequestsQueue::iterator req_it = q.begin(); + while (req_it != q.end()) { + Request* req = *req_it; + Key req_key(req->info().hostname(), req->info().address_family()); + if (req_key == job->key()) { + // Job takes ownership of |req|. + job->AddRequest(req); + req_it = q.erase(req_it); + } else { + ++req_it; + } + } + } + } + + private: + typedef std::deque<Request*> PendingRequestsQueue; + + // Maximum number of concurrent jobs allowed to be started for requests + // belonging to this pool. + size_t max_outstanding_jobs_; + + // The current number of running jobs that were started for requests + // belonging to this pool. + size_t num_outstanding_jobs_; + + // The maximum number of requests we allow to be waiting on a job, + // for this pool. + size_t max_pending_requests_; + + // The requests which are waiting to be started for this pool. + PendingRequestsQueue pending_requests_[NUM_PRIORITIES]; +}; + +//----------------------------------------------------------------------------- + HostResolverImpl::HostResolverImpl( HostResolverProc* resolver_proc, HostCache* cache, - const scoped_refptr<NetworkChangeNotifier>& network_change_notifier) + const scoped_refptr<NetworkChangeNotifier>& network_change_notifier, + size_t max_jobs) : cache_(cache), + max_jobs_(max_jobs), next_request_id_(0), resolver_proc_(resolver_proc), default_address_family_(ADDRESS_FAMILY_UNSPECIFIED), shutdown_(false), network_change_notifier_(network_change_notifier) { + DCHECK_GT(max_jobs, 0u); + + // It is cumbersome to expose all of the constraints in the constructor, + // so we choose some defaults, which users can override later. + job_pools_[POOL_NORMAL] = new JobPool(max_jobs, 100u * max_jobs); + #if defined(OS_WIN) EnsureWinsockInit(); #endif @@ -318,6 +492,10 @@ HostResolverImpl::~HostResolverImpl() { if (network_change_notifier_) network_change_notifier_->RemoveObserver(this); + + // Delete the job pools. + for (size_t i = 0u; i < arraysize(job_pools_); ++i) + delete job_pools_[i]; } // TODO(eroman): Don't create cache entries for hostnames which are simply IP @@ -394,13 +572,12 @@ int HostResolverImpl::Resolve(const RequestInfo& info, if (job) { job->AddRequest(req); } else { - // Create a new job for this request. - job = new Job(this, key); - job->AddRequest(req); - AddOutstandingJob(job); - // TODO(eroman): Bound the total number of concurrent jobs. - // http://crbug.com/9598 - job->Start(); + JobPool* pool = GetPoolForRequest(req); + if (CanCreateJobForPool(*pool)) { + CreateAndStartJob(req); + } else { + return EnqueueRequest(pool, req); + } } // Completion happens during OnJobComplete(Job*). @@ -420,7 +597,19 @@ void HostResolverImpl::CancelRequest(RequestHandle req_handle) { } Request* req = reinterpret_cast<Request*>(req_handle); DCHECK(req); - DCHECK(req->job()); + + scoped_ptr<Request> request_deleter; // Frees at end of function. + + if (!req->job()) { + // If the request was not attached to a job yet, it must have been + // enqueued into a pool. Remove it from that pool's queue. + // Otherwise if it was attached to a job, the job is responsible for + // deleting it. + JobPool* pool = GetPoolForRequest(req); + pool->RemovePendingRequest(req); + request_deleter.reset(req); + } + // NULL out the fields of req, to mark it as cancelled. req->MarkAsCancelled(); OnCancelRequest(req->load_log(), req->id(), req->info()); @@ -453,10 +642,23 @@ void HostResolverImpl::Shutdown() { jobs_.clear(); } +void HostResolverImpl::SetPoolConstraints(JobPoolIndex pool_index, + size_t max_outstanding_jobs, + size_t max_pending_requests) { + CHECK(pool_index >= 0); + CHECK(pool_index < POOL_COUNT); + CHECK(jobs_.empty()) << "Can only set constraints during setup"; + JobPool* pool = job_pools_[pool_index]; + pool->SetConstraints(max_outstanding_jobs, max_pending_requests); +} + void HostResolverImpl::AddOutstandingJob(Job* job) { scoped_refptr<Job>& found_job = jobs_[job->key()]; DCHECK(!found_job); found_job = job; + + JobPool* pool = GetPoolForRequest(job->initial_request()); + pool->AdjustNumOutstandingJobs(1); } HostResolverImpl::Job* HostResolverImpl::FindOutstandingJob(const Key& key) { @@ -471,6 +673,9 @@ void HostResolverImpl::RemoveOutstandingJob(Job* job) { DCHECK(it != jobs_.end()); DCHECK_EQ(it->second.get(), job); jobs_.erase(it); + + JobPool* pool = GetPoolForRequest(job->initial_request()); + pool->AdjustNumOutstandingJobs(-1); } void HostResolverImpl::OnJobComplete(Job* job, @@ -487,6 +692,9 @@ void HostResolverImpl::OnJobComplete(Job* job, DCHECK(!cur_completing_job_); cur_completing_job_ = job; + // Try to start any queued requests now that a job-slot has freed up. + ProcessQueuedRequests(); + // Complete all of the requests that were attached to the job. for (RequestsList::const_iterator it = job->requests().begin(); it != job->requests().end(); ++it) { @@ -578,4 +786,74 @@ void HostResolverImpl::OnIPAddressChanged() { cache_->clear(); } +// static +HostResolverImpl::JobPoolIndex HostResolverImpl::GetJobPoolIndexForRequest( + const Request* req) { + return POOL_NORMAL; +} + +bool HostResolverImpl::CanCreateJobForPool(const JobPool& pool) const { + DCHECK_LE(jobs_.size(), max_jobs_); + + // We can't create another job if it would exceed the global total. + if (jobs_.size() + 1 > max_jobs_) + return false; + + // Check whether the pool's constraints are met. + return pool.CanCreateJob(); +} + +void HostResolverImpl::ProcessQueuedRequests() { + // Find the highest priority request that can be scheduled. + Request* top_req = NULL; + for (size_t i = 0; i < arraysize(job_pools_); ++i) { + JobPool* pool = job_pools_[i]; + if (pool->HasPendingRequests() && CanCreateJobForPool(*pool)) { + top_req = pool->RemoveTopPendingRequest(); + break; + } + } + + if (!top_req) + return; + + scoped_refptr<Job> job = CreateAndStartJob(top_req); + + // Search for any other pending request which can piggy-back off this job. + for (size_t pool_i = 0; pool_i < POOL_COUNT; ++pool_i) { + JobPool* pool = job_pools_[pool_i]; + pool->MoveRequestsToJob(job); + } +} + +HostResolverImpl::Job* HostResolverImpl::CreateAndStartJob(Request* req) { + DCHECK(CanCreateJobForPool(*GetPoolForRequest(req))); + Key key(req->info().hostname(), req->info().address_family()); + scoped_refptr<Job> job = new Job(this, key); + job->AddRequest(req); + AddOutstandingJob(job); + job->Start(); + return job.get(); +} + +int HostResolverImpl::EnqueueRequest(JobPool* pool, Request* req) { + scoped_ptr<Request> req_evicted_from_queue( + pool->InsertPendingRequest(req)); + + // If the queue has become too large, we need to kick something out. + if (req_evicted_from_queue.get()) { + Request* r = req_evicted_from_queue.get(); + int error = ERR_HOST_RESOLVER_QUEUE_TOO_LARGE; + + OnFinishRequest(r->load_log(), r->id(), r->info(), error); + + if (r == req) + return error; + + r->OnComplete(error, AddressList()); + } + + return ERR_IO_PENDING; +} + } // namespace net diff --git a/net/base/host_resolver_impl.h b/net/base/host_resolver_impl.h index 6cabcf9..77f9020 100644 --- a/net/base/host_resolver_impl.h +++ b/net/base/host_resolver_impl.h @@ -41,9 +41,24 @@ namespace net { // Thread safety: This class is not threadsafe, and must only be called // from one thread! // +// The HostResolverImpl enforces |max_jobs_| as the maximum number of concurrent +// threads. +// +// Requests are ordered in the queue based on their priority. + class HostResolverImpl : public HostResolver, public NetworkChangeNotifier::Observer { public: + // The index into |job_pools_| for the various job pools. Pools with a higher + // index have lower priority. + // + // Note: This is currently unused, since there is a single pool + // for all requests. + enum JobPoolIndex { + POOL_NORMAL = 0, + POOL_COUNT, + }; + // Creates a HostResolver that first uses the local cache |cache|, and then // falls back to |resolver_proc|. // @@ -54,9 +69,13 @@ class HostResolverImpl : public HostResolver, // thread-safe since it is run from multiple worker threads. If // |resolver_proc| is NULL then the default host resolver procedure is // used (which is SystemHostResolverProc except if overridden). + // + // |max_jobs| specifies the maximum number of threads that the host resolver + // will use. Use SetPoolConstraints() to specify finer-grain settings. HostResolverImpl(HostResolverProc* resolver_proc, HostCache* cache, - const scoped_refptr<NetworkChangeNotifier>& notifier); + const scoped_refptr<NetworkChangeNotifier>& notifier, + size_t max_jobs); // HostResolver methods: virtual int Resolve(const RequestInfo& info, @@ -76,8 +95,24 @@ class HostResolverImpl : public HostResolver, default_address_family_ = address_family; } + // Applies a set of constraints for requests that belong to the specified + // pool. NOTE: Don't call this after requests have been already been started. + // + // |pool_index| -- Specifies which pool these constraints should be applied + // to. + // |max_outstanding_jobs| -- How many concurrent jobs are allowed for this + // pool. + // |max_pending_requests| -- How many requests can be enqueued for this pool + // before we start dropping requests. Dropped + // requests fail with + // ERR_HOST_RESOLVER_QUEUE_TOO_LARGE. + void SetPoolConstraints(JobPoolIndex pool_index, + size_t max_outstanding_jobs, + size_t max_pending_requests); + private: class Job; + class JobPool; class Request; typedef std::vector<Request*> RequestsList; typedef HostCache::Key Key; @@ -126,12 +161,40 @@ class HostResolverImpl : public HostResolver, // NetworkChangeNotifier::Observer methods: virtual void OnIPAddressChanged(); + // Returns true if the constraints for |pool| are met, and a new job can be + // created for this pool. + bool CanCreateJobForPool(const JobPool& pool) const; + + // Returns the index of the pool that request |req| maps to. + static JobPoolIndex GetJobPoolIndexForRequest(const Request* req); + + JobPool* GetPoolForRequest(const Request* req) { + return job_pools_[GetJobPoolIndexForRequest(req)]; + } + + // Starts up to 1 job given the current pool constraints. This job + // may have multiple requests attached to it. + void ProcessQueuedRequests(); + + // Attaches |req| to a new job, and starts it. Returns that job. + Job* CreateAndStartJob(Request* req); + + // Adds a pending request |req| to |pool|. + int EnqueueRequest(JobPool* pool, Request* req); + // Cache of host resolution results. scoped_ptr<HostCache> cache_; // Map from hostname to outstanding job. JobMap jobs_; + // Maximum number of concurrent jobs allowed, across all pools. + size_t max_jobs_; + + // The information to track pending requests for a JobPool, as well as + // how many outstanding jobs the pool already has, and its constraints. + JobPool* job_pools_[POOL_COUNT]; + // The job that OnJobComplete() is currently processing (needed in case // HostResolver gets deleted from within the callback). scoped_refptr<Job> cur_completing_job_; diff --git a/net/base/host_resolver_impl_unittest.cc b/net/base/host_resolver_impl_unittest.cc index c2cd1b5..f3d350c 100644 --- a/net/base/host_resolver_impl_unittest.cc +++ b/net/base/host_resolver_impl_unittest.cc @@ -24,6 +24,7 @@ // cache while an async is already pending). namespace net { + namespace { HostCache* CreateDefaultCache() { @@ -33,6 +34,25 @@ HostCache* CreateDefaultCache() { base::TimeDelta::FromSeconds(0)); } +static const size_t kMaxJobs = 10u; + +HostResolverImpl* CreateHostResolverImpl(HostResolverProc* resolver_proc) { + return new HostResolverImpl( + resolver_proc, + CreateDefaultCache(), + NULL, // network_change_notifier + kMaxJobs); +} + +// Helper to create a HostResolver::RequestInfo. +HostResolver::RequestInfo CreateResolverRequest( + const std::string& hostname, + RequestPriority priority) { + HostResolver::RequestInfo info(hostname, 80); + info.set_priority(priority); + return info; +} + // A variant of WaitingHostResolverProc that pushes each host mapped into a // list. // (and uses a manual-reset event rather than auto-reset). @@ -186,7 +206,7 @@ TEST_F(HostResolverImplTest, SynchronousLookup) { resolver_proc->AddRule("just.testing", "192.168.1.42"); scoped_refptr<HostResolver> host_resolver( - new HostResolverImpl(resolver_proc, CreateDefaultCache(), NULL)); + CreateHostResolverImpl(resolver_proc)); HostResolver::RequestInfo info("just.testing", kPortnum); scoped_refptr<LoadLog> log(new LoadLog(LoadLog::kUnbounded)); @@ -216,7 +236,7 @@ TEST_F(HostResolverImplTest, AsynchronousLookup) { resolver_proc->AddRule("just.testing", "192.168.1.42"); scoped_refptr<HostResolver> host_resolver( - new HostResolverImpl(resolver_proc, CreateDefaultCache(), NULL)); + CreateHostResolverImpl(resolver_proc)); HostResolver::RequestInfo info("just.testing", kPortnum); scoped_refptr<LoadLog> log(new LoadLog(LoadLog::kUnbounded)); @@ -251,7 +271,7 @@ TEST_F(HostResolverImplTest, CanceledAsynchronousLookup) { scoped_refptr<LoadLog> log(new LoadLog(LoadLog::kUnbounded)); { scoped_refptr<HostResolver> host_resolver( - new HostResolverImpl(resolver_proc, CreateDefaultCache(), NULL)); + CreateHostResolverImpl(resolver_proc)); AddressList adrlist; const int kPortnum = 80; @@ -285,7 +305,7 @@ TEST_F(HostResolverImplTest, NumericIPv4Address) { resolver_proc->AllowDirectLookup("*"); scoped_refptr<HostResolver> host_resolver( - new HostResolverImpl(resolver_proc, CreateDefaultCache(), NULL)); + CreateHostResolverImpl(resolver_proc)); AddressList adrlist; const int kPortnum = 5555; HostResolver::RequestInfo info("127.1.2.3", kPortnum); @@ -310,7 +330,7 @@ TEST_F(HostResolverImplTest, NumericIPv6Address) { // Resolve a plain IPv6 address. Don't worry about [brackets], because // the caller should have removed them. scoped_refptr<HostResolver> host_resolver( - new HostResolverImpl(resolver_proc, CreateDefaultCache(), NULL)); + CreateHostResolverImpl(resolver_proc)); AddressList adrlist; const int kPortnum = 5555; HostResolver::RequestInfo info("2001:db8::1", kPortnum); @@ -345,7 +365,7 @@ TEST_F(HostResolverImplTest, EmptyHost) { resolver_proc->AllowDirectLookup("*"); scoped_refptr<HostResolver> host_resolver( - new HostResolverImpl(resolver_proc, CreateDefaultCache(), NULL)); + CreateHostResolverImpl(resolver_proc)); AddressList adrlist; const int kPortnum = 5555; HostResolver::RequestInfo info("", kPortnum); @@ -406,7 +426,7 @@ TEST_F(HostResolverImplTest, DeDupeRequests) { new CapturingHostResolverProc(NULL); scoped_refptr<HostResolver> host_resolver( - new HostResolverImpl(resolver_proc, CreateDefaultCache(), NULL)); + CreateHostResolverImpl(resolver_proc)); // The class will receive callbacks for when each resolve completes. It // checks that the right things happened. @@ -457,7 +477,7 @@ TEST_F(HostResolverImplTest, CancelMultipleRequests) { new CapturingHostResolverProc(NULL); scoped_refptr<HostResolver> host_resolver( - new HostResolverImpl(resolver_proc, CreateDefaultCache(), NULL)); + CreateHostResolverImpl(resolver_proc)); // The class will receive callbacks for when each resolve completes. It // checks that the right things happened. @@ -544,7 +564,7 @@ TEST_F(HostResolverImplTest, CancelWithinCallback) { new CapturingHostResolverProc(NULL); scoped_refptr<HostResolver> host_resolver( - new HostResolverImpl(resolver_proc, CreateDefaultCache(), NULL)); + CreateHostResolverImpl(resolver_proc)); // The class will receive callbacks for when each resolve completes. It // checks that the right things happened. @@ -605,7 +625,7 @@ TEST_F(HostResolverImplTest, DeleteWithinCallback) { // checks that the right things happened. Note that the verifier holds the // only reference to |host_resolver|, so it can delete it within callback. HostResolver* host_resolver = - new HostResolverImpl(resolver_proc, CreateDefaultCache(), NULL); + CreateHostResolverImpl(resolver_proc); DeleteWithinCallbackVerifier verifier(host_resolver); // Start 4 requests, duplicating hosts "a". Since the resolver_proc is @@ -658,7 +678,7 @@ TEST_F(HostResolverImplTest, StartWithinCallback) { // Turn off caching for this host resolver. scoped_refptr<HostResolver> host_resolver( - new HostResolverImpl(resolver_proc, NULL, NULL)); + new HostResolverImpl(resolver_proc, NULL, NULL, kMaxJobs)); // The class will receive callbacks for when each resolve completes. It // checks that the right things happened. @@ -723,7 +743,7 @@ class BypassCacheVerifier : public ResolveRequest::Delegate { TEST_F(HostResolverImplTest, BypassCache) { scoped_refptr<HostResolver> host_resolver( - new HostResolverImpl(NULL, CreateDefaultCache(), NULL)); + CreateHostResolverImpl(NULL)); // The class will receive callbacks for when each resolve completes. It // checks that the right things happened. @@ -741,6 +761,7 @@ bool operator==(const HostResolver::RequestInfo& a, return a.hostname() == b.hostname() && a.port() == b.port() && a.allow_cached_response() == b.allow_cached_response() && + a.priority() == b.priority() && a.is_speculative() == b.is_speculative() && a.referrer() == b.referrer(); } @@ -807,7 +828,7 @@ class CapturingObserver : public HostResolver::Observer { // synchronous. TEST_F(HostResolverImplTest, Observers) { scoped_refptr<HostResolver> host_resolver( - new HostResolverImpl(NULL, CreateDefaultCache(), NULL)); + CreateHostResolverImpl(NULL)); CapturingObserver observer; @@ -892,7 +913,7 @@ TEST_F(HostResolverImplTest, CancellationObserver) { { // Create a host resolver and attach an observer. scoped_refptr<HostResolver> host_resolver( - new HostResolverImpl(NULL, CreateDefaultCache(), NULL)); + CreateHostResolverImpl(NULL)); host_resolver->AddObserver(&observer); TestCompletionCallback callback; @@ -961,7 +982,8 @@ TEST_F(HostResolverImplTest, FlushCacheOnIPAddressChange) { new MockNetworkChangeNotifier); scoped_refptr<HostResolver> host_resolver( new HostResolverImpl(NULL, CreateDefaultCache(), - mock_network_change_notifier)); + mock_network_change_notifier, + kMaxJobs)); AddressList addrlist; @@ -987,5 +1009,230 @@ TEST_F(HostResolverImplTest, FlushCacheOnIPAddressChange) { EXPECT_EQ(OK, callback.WaitForResult()); } +// Tests that when the maximum threads is set to 1, requests are dequeued +// in order of priority. +TEST_F(HostResolverImplTest, HigherPriorityRequestsStartedFirst) { + scoped_refptr<CapturingHostResolverProc> resolver_proc = + new CapturingHostResolverProc(NULL); + + // This HostResolverImpl will only allow 1 outstanding resolve at a time. + size_t kMaxJobs = 1u; + scoped_refptr<HostResolver> host_resolver( + new HostResolverImpl(resolver_proc, CreateDefaultCache(), + NULL, kMaxJobs)); + + CapturingObserver observer; + host_resolver->AddObserver(&observer); + + // Note that at this point the CapturingHostResolverProc is blocked, so any + // requests we make will not complete. + + HostResolver::RequestInfo req[] = { + CreateResolverRequest("req0", LOW), + CreateResolverRequest("req1", MEDIUM), + CreateResolverRequest("req2", MEDIUM), + CreateResolverRequest("req3", LOW), + CreateResolverRequest("req4", HIGHEST), + CreateResolverRequest("req5", LOW), + CreateResolverRequest("req6", LOW), + CreateResolverRequest("req5", HIGHEST), + }; + + TestCompletionCallback callback[arraysize(req)]; + AddressList addrlist[arraysize(req)]; + + // Start all of the requests. + for (size_t i = 0; i < arraysize(req); ++i) { + int rv = host_resolver->Resolve(req[i], &addrlist[i], + &callback[i], NULL, NULL); + EXPECT_EQ(ERR_IO_PENDING, rv); + } + + // Unblock the resolver thread so the requests can run. + resolver_proc->Signal(); + + // Wait for all the requests to complete succesfully. + for (size_t i = 0; i < arraysize(req); ++i) { + EXPECT_EQ(OK, callback[i].WaitForResult()) << "i=" << i; + } + + host_resolver->RemoveObserver(&observer); + + // Since we have restricted to a single concurrent thread in the jobpool, + // the requests should complete in order of priority (with the exception + // of the first request, which gets started right away, since there is + // nothing outstanding). + std::vector<std::string> capture_list = resolver_proc->GetCaptureList(); + ASSERT_EQ(7u, capture_list.size()); + + EXPECT_EQ("req0", capture_list[0]); + EXPECT_EQ("req4", capture_list[1]); + EXPECT_EQ("req5", capture_list[2]); + EXPECT_EQ("req1", capture_list[3]); + EXPECT_EQ("req2", capture_list[4]); + EXPECT_EQ("req3", capture_list[5]); + EXPECT_EQ("req6", capture_list[6]); + + // Also check using the observer's trace. + EXPECT_EQ(8U, observer.start_log.size()); + EXPECT_EQ(8U, observer.finish_log.size()); + EXPECT_EQ(0U, observer.cancel_log.size()); + + EXPECT_EQ("req0", observer.finish_log[0].info.hostname()); + EXPECT_EQ("req4", observer.finish_log[1].info.hostname()); + + // There were two requests for "req5". The highest priority + // one should have been dispatched earlier. + EXPECT_EQ("req5", observer.finish_log[2].info.hostname()); + EXPECT_EQ("req5", observer.finish_log[3].info.hostname()); + EXPECT_EQ(HIGHEST, observer.finish_log[2].info.priority()); + EXPECT_EQ(LOW, observer.finish_log[3].info.priority()); + + EXPECT_EQ("req1", observer.finish_log[4].info.hostname()); + EXPECT_EQ("req2", observer.finish_log[5].info.hostname()); + EXPECT_EQ("req3", observer.finish_log[6].info.hostname()); + EXPECT_EQ("req6", observer.finish_log[7].info.hostname()); +} + +// Try cancelling a request which has not been attached to a job yet. +TEST_F(HostResolverImplTest, CancelPendingRequest) { + scoped_refptr<CapturingHostResolverProc> resolver_proc = + new CapturingHostResolverProc(NULL); + + // This HostResolverImpl will only allow 1 outstanding resolve at a time. + const size_t kMaxJobs = 1u; + scoped_refptr<HostResolver> host_resolver( + new HostResolverImpl(resolver_proc, CreateDefaultCache(), + NULL, kMaxJobs)); + + // Note that at this point the CapturingHostResolverProc is blocked, so any + // requests we make will not complete. + + HostResolver::RequestInfo req[] = { + CreateResolverRequest("req0", LOWEST), + CreateResolverRequest("req1", HIGHEST), // Will cancel. + CreateResolverRequest("req2", MEDIUM), + CreateResolverRequest("req3", LOW), + CreateResolverRequest("req4", HIGHEST), // Will cancel. + CreateResolverRequest("req5", LOWEST), // Will cancel. + CreateResolverRequest("req6", MEDIUM), + }; + + TestCompletionCallback callback[arraysize(req)]; + AddressList addrlist[arraysize(req)]; + HostResolver::RequestHandle handle[arraysize(req)]; + + // Start all of the requests. + for (size_t i = 0; i < arraysize(req); ++i) { + int rv = host_resolver->Resolve(req[i], &addrlist[i], + &callback[i], &handle[i], NULL); + EXPECT_EQ(ERR_IO_PENDING, rv); + } + + // Cancel some requests + host_resolver->CancelRequest(handle[1]); + host_resolver->CancelRequest(handle[4]); + host_resolver->CancelRequest(handle[5]); + handle[1] = handle[4] = handle[5] = NULL; + + // Unblock the resolver thread so the requests can run. + resolver_proc->Signal(); + + // Wait for all the requests to complete succesfully. + for (size_t i = 0; i < arraysize(req); ++i) { + if (!handle[i]) + continue; // Don't wait for the requests we cancelled. + EXPECT_EQ(OK, callback[i].WaitForResult()); + } + + // Verify that they called out the the resolver proc (which runs on the + // resolver thread) in the expected order. + std::vector<std::string> capture_list = resolver_proc->GetCaptureList(); + ASSERT_EQ(4u, capture_list.size()); + + EXPECT_EQ("req0", capture_list[0]); + EXPECT_EQ("req2", capture_list[1]); + EXPECT_EQ("req6", capture_list[2]); + EXPECT_EQ("req3", capture_list[3]); +} + +// Test that when too many requests are enqueued, old ones start to be aborted. +TEST_F(HostResolverImplTest, QueueOverflow) { + scoped_refptr<CapturingHostResolverProc> resolver_proc = + new CapturingHostResolverProc(NULL); + + // This HostResolverImpl will only allow 1 outstanding resolve at a time. + const size_t kMaxOutstandingJobs = 1u; + scoped_refptr<HostResolverImpl> host_resolver( + new HostResolverImpl(resolver_proc, CreateDefaultCache(), + NULL, kMaxOutstandingJobs)); + + // Only allow up to 3 requests to be enqueued at a time. + const size_t kMaxPendingRequests = 3u; + host_resolver->SetPoolConstraints(HostResolverImpl::POOL_NORMAL, + kMaxOutstandingJobs, + kMaxPendingRequests); + + // Note that at this point the CapturingHostResolverProc is blocked, so any + // requests we make will not complete. + + HostResolver::RequestInfo req[] = { + CreateResolverRequest("req0", LOWEST), + CreateResolverRequest("req1", HIGHEST), + CreateResolverRequest("req2", MEDIUM), + CreateResolverRequest("req3", MEDIUM), + + // At this point, there are 3 enqueued requests. + // Insertion of subsequent requests will cause evictions + // based on priority. + + CreateResolverRequest("req4", LOW), // Evicts itself! + CreateResolverRequest("req5", MEDIUM), // Evicts req3 + CreateResolverRequest("req6", HIGHEST), // Evicts req5. + CreateResolverRequest("req7", MEDIUM), // Evicts req2. + }; + + TestCompletionCallback callback[arraysize(req)]; + AddressList addrlist[arraysize(req)]; + HostResolver::RequestHandle handle[arraysize(req)]; + + // Start all of the requests. + for (size_t i = 0; i < arraysize(req); ++i) { + int rv = host_resolver->Resolve(req[i], &addrlist[i], + &callback[i], &handle[i], NULL); + if (i == 4u) + EXPECT_EQ(ERR_HOST_RESOLVER_QUEUE_TOO_LARGE, rv); + else + EXPECT_EQ(ERR_IO_PENDING, rv) << i; + } + + // Unblock the resolver thread so the requests can run. + resolver_proc->Signal(); + + // Requests 3, 5, 2 will have been evicted due to queue overflow. + size_t reqs_expected_to_fail[] = { 2, 3, 5 }; + for (size_t i = 0; i < arraysize(reqs_expected_to_fail); ++i) { + EXPECT_EQ(ERR_HOST_RESOLVER_QUEUE_TOO_LARGE, + callback[reqs_expected_to_fail[i]].WaitForResult()); + } + + // The rest should succeed. + size_t reqs_expected_to_succeed[] = { 0, 1, 6, 7 }; + for (size_t i = 0; i < arraysize(reqs_expected_to_succeed); ++i) { + EXPECT_EQ(OK, callback[reqs_expected_to_succeed[i]].WaitForResult()); + } + + // Verify that they called out the the resolver proc (which runs on the + // resolver thread) in the expected order. + std::vector<std::string> capture_list = resolver_proc->GetCaptureList(); + ASSERT_EQ(4u, capture_list.size()); + + EXPECT_EQ("req0", capture_list[0]); + EXPECT_EQ("req1", capture_list[1]); + EXPECT_EQ("req6", capture_list[2]); + EXPECT_EQ("req7", capture_list[3]); +} + } // namespace + } // namespace net diff --git a/net/base/mock_host_resolver.cc b/net/base/mock_host_resolver.cc index 0bac1ff..6f555d3 100644 --- a/net/base/mock_host_resolver.cc +++ b/net/base/mock_host_resolver.cc @@ -102,7 +102,7 @@ void MockHostResolverBase::Reset(HostResolverProc* interceptor) { base::TimeDelta::FromSeconds(0)); } - impl_ = new HostResolverImpl(proc, cache, NULL); + impl_ = new HostResolverImpl(proc, cache, NULL, 50u); } //----------------------------------------------------------------------------- diff --git a/net/base/net_error_list.h b/net/base/net_error_list.h index 81ca271..bff55d5 100644 --- a/net/base/net_error_list.h +++ b/net/base/net_error_list.h @@ -121,6 +121,10 @@ NET_ERROR(BAD_SSL_CLIENT_AUTH_CERT, -117) // A connection attempt timed out. NET_ERROR(CONNECTION_TIMED_OUT, -118) +// There are too many pending DNS resolves, so a request in the queue was +// aborted. +NET_ERROR(HOST_RESOLVER_QUEUE_TOO_LARGE, -119) + // Certificate error codes // // The values of certificate error codes must be consecutive. diff --git a/net/base/request_priority.h b/net/base/request_priority.h index b618ef2..1347001 100644 --- a/net/base/request_priority.h +++ b/net/base/request_priority.h @@ -13,7 +13,8 @@ enum RequestPriority { HIGHEST = 0, // 0 must be the highest priority. MEDIUM, LOW, - LOWEST + LOWEST, + NUM_PRIORITIES, }; } // namespace net diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc index 8bbc883..4be555f 100644 --- a/net/http/http_network_transaction.cc +++ b/net/http/http_network_transaction.cc @@ -677,6 +677,7 @@ int HttpNetworkTransaction::DoInitConnection() { DCHECK(!connection_group.empty()); HostResolver::RequestInfo resolve_info(host, port); + resolve_info.set_priority(request_->priority); // The referrer is used by the DNS prefetch system to correlate resolutions // with the page that triggered them. It doesn't impact the actual addresses @@ -751,6 +752,7 @@ int HttpNetworkTransaction::DoSOCKSConnect() { HostResolver::RequestInfo req_info(request_->url.HostNoBrackets(), request_->url.EffectiveIntPort()); req_info.set_referrer(request_->referrer); + req_info.set_priority(request_->priority); if (proxy_info_.proxy_server().scheme() == ProxyServer::SCHEME_SOCKS5) s = new SOCKS5ClientSocket(s, req_info); @@ -1134,6 +1136,7 @@ int HttpNetworkTransaction::DoSpdySendRequest() { HostResolver::RequestInfo req_info(request_->url.HostNoBrackets(), request_->url.EffectiveIntPort()); + req_info.set_priority(request_->priority); const scoped_refptr<FlipSessionPool> spdy_pool = session_->flip_session_pool(); scoped_refptr<FlipSession> spdy_session; diff --git a/net/tools/hresolv/hresolv.cc b/net/tools/hresolv/hresolv.cc index f865a53..f092703 100644 --- a/net/tools/hresolv/hresolv.cc +++ b/net/tools/hresolv/hresolv.cc @@ -446,7 +446,7 @@ int main(int argc, char** argv) { base::TimeDelta::FromSeconds(0)); scoped_refptr<net::HostResolver> host_resolver( - new net::HostResolverImpl(NULL, cache, NULL)); + new net::HostResolverImpl(NULL, cache, NULL, 100u)); ResolverInvoker invoker(host_resolver.get()); invoker.ResolveAll(hosts_and_times, options.async); |