From 5cca78e718f15522cc3db9aec76aa910dd696aa8 Mon Sep 17 00:00:00 2001 From: Monty Brandenberg Date: Mon, 23 Jun 2014 14:23:33 -0400 Subject: First HTTP pipelining viewer. Enable pipelining for GetTexture and GetMesh2 at a pipeline depth of 5. Create global debug option, HttpPipelining, to enable and disable HTTP pipelining (defaults to true). Tweak texture and mesh low- and high-water request levels based on pipelining status and depth. Fixup texture console which was damaged in a recent release. Split logging of the no-request HTTP error case into two cases: one for missing URL in HTTP request, one for HTTP request not created. A refactor in llcorehttp is coming: I will be moving all libcurl- using code into libcurl-specific modules. --- indra/llcorehttp/_httplibcurl.cpp | 69 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 67 insertions(+), 2 deletions(-) (limited to 'indra/llcorehttp/_httplibcurl.cpp') diff --git a/indra/llcorehttp/_httplibcurl.cpp b/indra/llcorehttp/_httplibcurl.cpp index e56bc84174..fb907f6318 100755 --- a/indra/llcorehttp/_httplibcurl.cpp +++ b/indra/llcorehttp/_httplibcurl.cpp @@ -4,7 +4,7 @@ * * $LicenseInfo:firstyear=2012&license=viewerlgpl$ * Second Life Viewer Source Code - * Copyright (C) 2012-2013, Linden Research, Inc. + * Copyright (C) 2012-2014, Linden Research, Inc. * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -33,6 +33,15 @@ #include "llhttpconstants.h" +namespace +{ + +// Error testing and reporting for libcurl status codes +void check_curl_multi_code(CURLMcode code); +void check_curl_multi_code(CURLMcode code, int curl_setopt_option); + +} // end anonymous namespace + namespace LLCore { @@ -92,14 +101,44 @@ void HttpLibcurl::start(int policy_count) llassert_always(policy_count <= HTTP_POLICY_CLASS_LIMIT); llassert_always(! mMultiHandles); // One-time call only + HttpPolicy & policy(mService->getPolicy()); mPolicyCount = policy_count; mMultiHandles = new CURLM * [mPolicyCount]; mActiveHandles = new int [mPolicyCount]; for (int policy_class(0); policy_class < mPolicyCount; ++policy_class) { - mMultiHandles[policy_class] = curl_multi_init(); + HttpPolicyClass & options(policy.getClassOptions(policy_class)); + mActiveHandles[policy_class] = 0; + if (NULL == (mMultiHandles[policy_class] = curl_multi_init())) + { + LL_ERRS("CoreHttp") << "Failed to allocate multi handle in libcurl." + << LL_ENDL; + } + + if (options.mPipelining > 1) + { + CURLMcode code; + + // We'll try to do pipelining on this multihandle + code = curl_multi_setopt(mMultiHandles[policy_class], + CURLMOPT_PIPELINING, + 1L); + check_curl_multi_code(code, CURLMOPT_PIPELINING); + code = curl_multi_setopt(mMultiHandles[policy_class], + CURLMOPT_MAX_PIPELINE_LENGTH, + long(options.mPipelining)); + check_curl_multi_code(code, CURLMOPT_MAX_PIPELINE_LENGTH); + code = curl_multi_setopt(mMultiHandles[policy_class], + CURLMOPT_MAX_HOST_CONNECTIONS, + long(options.mPerHostConnectionLimit)); + check_curl_multi_code(code, CURLMOPT_MAX_HOST_CONNECTIONS); + code = curl_multi_setopt(mMultiHandles[policy_class], + CURLMOPT_MAX_TOTAL_CONNECTIONS, + long(options.mConnectionLimit)); + check_curl_multi_code(code, CURLMOPT_MAX_TOTAL_CONNECTIONS); + } } } @@ -376,3 +415,29 @@ struct curl_slist * append_headers_to_slist(const HttpHeaders * headers, struct } // end namespace LLCore + + +namespace +{ + +void check_curl_multi_code(CURLMcode code, int curl_setopt_option) +{ + if (CURLM_OK != code) + { + LL_WARNS("CoreHttp") << "libcurl multi error detected: " << curl_multi_strerror(code) + << ", curl_multi_setopt option: " << curl_setopt_option + << LL_ENDL; + } +} + + +void check_curl_multi_code(CURLMcode code) +{ + if (CURLM_OK != code) + { + LL_WARNS("CoreHttp") << "libcurl multi error detected: " << curl_multi_strerror(code) + << LL_ENDL; + } +} + +} // end anonymous namespace -- cgit v1.2.3 From e79a88c8ccfadcd260892000d4dec2ae921b26de Mon Sep 17 00:00:00 2001 From: Monty Brandenberg Date: Tue, 12 Aug 2014 18:21:26 -0400 Subject: Better support for dynamic option changes in llcorehttp. Libcurl has some problems disabling pipelining on a multi handle with outstanding requests so build a more conservative system that allows requests to drain before setting curl multi options. Would rather not have this but it is significantly safer. "HttpPipelining" debug setting is now fully dynamic. Connection limits can also be made dynamic in the near future. Upped the default connection count back to 8 for now but will revisit this in the tuning phase. It might be time to combine mesh and textures into a single asset class. For normal server operations that would be a clear path, but for server under load, the current scheme may be better. Minor cleanup in logging to elminate some redundant strings. Might add some more tracing to the stall logic 'just in case'. --- indra/llcorehttp/_httplibcurl.cpp | 198 ++++++++++++++++++++++++++------------ 1 file changed, 137 insertions(+), 61 deletions(-) (limited to 'indra/llcorehttp/_httplibcurl.cpp') diff --git a/indra/llcorehttp/_httplibcurl.cpp b/indra/llcorehttp/_httplibcurl.cpp index fb907f6318..b46833a1f3 100755 --- a/indra/llcorehttp/_httplibcurl.cpp +++ b/indra/llcorehttp/_httplibcurl.cpp @@ -40,6 +40,8 @@ namespace void check_curl_multi_code(CURLMcode code); void check_curl_multi_code(CURLMcode code, int curl_setopt_option); +static const char * const LOG_CORE("CoreHttp"); + } // end anonymous namespace @@ -51,7 +53,8 @@ HttpLibcurl::HttpLibcurl(HttpService * service) : mService(service), mPolicyCount(0), mMultiHandles(NULL), - mActiveHandles(NULL) + mActiveHandles(NULL), + mDirtyPolicy(NULL) {} @@ -90,6 +93,9 @@ void HttpLibcurl::shutdown() delete [] mActiveHandles; mActiveHandles = NULL; + + delete [] mDirtyPolicy; + mDirtyPolicy = NULL; } mPolicyCount = 0; @@ -101,44 +107,21 @@ void HttpLibcurl::start(int policy_count) llassert_always(policy_count <= HTTP_POLICY_CLASS_LIMIT); llassert_always(! mMultiHandles); // One-time call only - HttpPolicy & policy(mService->getPolicy()); mPolicyCount = policy_count; mMultiHandles = new CURLM * [mPolicyCount]; mActiveHandles = new int [mPolicyCount]; + mDirtyPolicy = new bool [mPolicyCount]; for (int policy_class(0); policy_class < mPolicyCount; ++policy_class) { - HttpPolicyClass & options(policy.getClassOptions(policy_class)); - - mActiveHandles[policy_class] = 0; if (NULL == (mMultiHandles[policy_class] = curl_multi_init())) { - LL_ERRS("CoreHttp") << "Failed to allocate multi handle in libcurl." - << LL_ENDL; - } - - if (options.mPipelining > 1) - { - CURLMcode code; - - // We'll try to do pipelining on this multihandle - code = curl_multi_setopt(mMultiHandles[policy_class], - CURLMOPT_PIPELINING, - 1L); - check_curl_multi_code(code, CURLMOPT_PIPELINING); - code = curl_multi_setopt(mMultiHandles[policy_class], - CURLMOPT_MAX_PIPELINE_LENGTH, - long(options.mPipelining)); - check_curl_multi_code(code, CURLMOPT_MAX_PIPELINE_LENGTH); - code = curl_multi_setopt(mMultiHandles[policy_class], - CURLMOPT_MAX_HOST_CONNECTIONS, - long(options.mPerHostConnectionLimit)); - check_curl_multi_code(code, CURLMOPT_MAX_HOST_CONNECTIONS); - code = curl_multi_setopt(mMultiHandles[policy_class], - CURLMOPT_MAX_TOTAL_CONNECTIONS, - long(options.mConnectionLimit)); - check_curl_multi_code(code, CURLMOPT_MAX_TOTAL_CONNECTIONS); + LL_ERRS(LOG_CORE) << "Failed to allocate multi handle in libcurl." + << LL_ENDL; } + mActiveHandles[policy_class] = 0; + mDirtyPolicy[policy_class] = false; + policyUpdated(policy_class); } } @@ -156,8 +139,19 @@ HttpService::ELoopSpeed HttpLibcurl::processTransport() // Give libcurl some cycles to do I/O & callbacks for (int policy_class(0); policy_class < mPolicyCount; ++policy_class) { - if (! mActiveHandles[policy_class] || ! mMultiHandles[policy_class]) + if (! mMultiHandles[policy_class]) { + // No handle, nothing to do. + continue; + } + if (! mActiveHandles[policy_class]) + { + // If we've gone quiet and there's a dirty update, apply it, + // otherwise we're done. + if (mDirtyPolicy[policy_class]) + { + policyUpdated(policy_class); + } continue; } @@ -192,9 +186,9 @@ HttpService::ELoopSpeed HttpLibcurl::processTransport() } else { - LL_WARNS_ONCE("CoreHttp") << "Unexpected message from libcurl. Msg code: " - << msg->msg - << LL_ENDL; + LL_WARNS_ONCE(LOG_CORE) << "Unexpected message from libcurl. Msg code: " + << msg->msg + << LL_ENDL; } msgs_in_queue = 0; } @@ -230,11 +224,11 @@ void HttpLibcurl::addOp(HttpOpRequest * op) { HttpPolicy & policy(mService->getPolicy()); - LL_INFOS("CoreHttp") << "TRACE, ToActiveQueue, Handle: " - << static_cast(op) - << ", Actives: " << mActiveOps.size() - << ", Readies: " << policy.getReadyCount(op->mReqPolicy) - << LL_ENDL; + LL_INFOS(LOG_CORE) << "TRACE, ToActiveQueue, Handle: " + << static_cast(op) + << ", Actives: " << mActiveOps.size() + << ", Readies: " << policy.getReadyCount(op->mReqPolicy) + << LL_ENDL; } // On success, make operation active @@ -286,10 +280,10 @@ void HttpLibcurl::cancelRequest(HttpOpRequest * op) // Tracing if (op->mTracing > HTTP_TRACE_OFF) { - LL_INFOS("CoreHttp") << "TRACE, RequestCanceled, Handle: " - << static_cast(op) - << ", Status: " << op->mStatus.toTerseString() - << LL_ENDL; + LL_INFOS(LOG_CORE) << "TRACE, RequestCanceled, Handle: " + << static_cast(op) + << ", Status: " << op->mStatus.toTerseString() + << LL_ENDL; } // Cancel op and deliver for notification @@ -306,18 +300,18 @@ bool HttpLibcurl::completeRequest(CURLM * multi_handle, CURL * handle, CURLcode if (handle != op->mCurlHandle || ! op->mCurlActive) { - LL_WARNS("CoreHttp") << "libcurl handle and HttpOpRequest handle in disagreement or inactive request." - << " Handle: " << static_cast(handle) - << LL_ENDL; + LL_WARNS(LOG_CORE) << "libcurl handle and HttpOpRequest handle in disagreement or inactive request." + << " Handle: " << static_cast(handle) + << LL_ENDL; return false; } active_set_t::iterator it(mActiveOps.find(op)); if (mActiveOps.end() == it) { - LL_WARNS("CoreHttp") << "libcurl completion for request not on active list. Continuing." - << " Handle: " << static_cast(handle) - << LL_ENDL; + LL_WARNS(LOG_CORE) << "libcurl completion for request not on active list. Continuing." + << " Handle: " << static_cast(handle) + << LL_ENDL; return false; } @@ -348,9 +342,9 @@ bool HttpLibcurl::completeRequest(CURLM * multi_handle, CURL * handle, CURLcode } else { - LL_WARNS("CoreHttp") << "Invalid HTTP response code (" - << http_status << ") received from server." - << LL_ENDL; + LL_WARNS(LOG_CORE) << "Invalid HTTP response code (" + << http_status << ") received from server." + << LL_ENDL; op->mStatus = HttpStatus(HttpStatus::LLCORE, HE_INVALID_HTTP_STATUS); } } @@ -363,10 +357,10 @@ bool HttpLibcurl::completeRequest(CURLM * multi_handle, CURL * handle, CURLcode // Tracing if (op->mTracing > HTTP_TRACE_OFF) { - LL_INFOS("CoreHttp") << "TRACE, RequestComplete, Handle: " - << static_cast(op) - << ", Status: " << op->mStatus.toTerseString() - << LL_ENDL; + LL_INFOS(LOG_CORE) << "TRACE, RequestComplete, Handle: " + << static_cast(op) + << ", Status: " << op->mStatus.toTerseString() + << LL_ENDL; } // Dispatch to next stage @@ -390,6 +384,88 @@ int HttpLibcurl::getActiveCountInClass(int policy_class) const return mActiveHandles ? mActiveHandles[policy_class] : 0; } +void HttpLibcurl::policyUpdated(int policy_class) +{ + if (policy_class < 0 || policy_class >= mPolicyCount || ! mMultiHandles) + { + return; + } + + HttpPolicy & policy(mService->getPolicy()); + + if (! mActiveHandles[policy_class]) + { + // Clear to set options. As of libcurl 7.37.0, if a pipelining + // multi handle has active requests and you try to set the + // multi handle to non-pipelining, the library gets very angry + // and goes off the rails corrupting memory. A clue that you're + // about to crash is that you'll get a missing server response + // error (curl code 9). So, if options are to be set, we let + // the multi handle run out of requests, then set options, and + // re-enable request processing. + // + // All of this stall mechanism exists for this reason. If + // libcurl becomes more resilient later, it should be possible + // to remove all of this. The connection limit settings are fine, + // it's just that pipelined-to-non-pipelined transition that + // is fatal at the moment. + + HttpPolicyClass & options(policy.getClassOptions(policy_class)); + CURLM * multi_handle(mMultiHandles[policy_class]); + CURLMcode code; + + // Enable policy if stalled + policy.stallPolicy(policy_class, false); + mDirtyPolicy[policy_class] = false; + + if (options.mPipelining > 1) + { + // We'll try to do pipelining on this multihandle + code = curl_multi_setopt(multi_handle, + CURLMOPT_PIPELINING, + 1L); + check_curl_multi_code(code, CURLMOPT_PIPELINING); + code = curl_multi_setopt(multi_handle, + CURLMOPT_MAX_PIPELINE_LENGTH, + long(options.mPipelining)); + check_curl_multi_code(code, CURLMOPT_MAX_PIPELINE_LENGTH); + code = curl_multi_setopt(multi_handle, + CURLMOPT_MAX_HOST_CONNECTIONS, + long(options.mPerHostConnectionLimit)); + check_curl_multi_code(code, CURLMOPT_MAX_HOST_CONNECTIONS); + code = curl_multi_setopt(multi_handle, + CURLMOPT_MAX_TOTAL_CONNECTIONS, + long(options.mConnectionLimit)); + check_curl_multi_code(code, CURLMOPT_MAX_TOTAL_CONNECTIONS); + } + else + { + code = curl_multi_setopt(multi_handle, + CURLMOPT_PIPELINING, + 0L); + check_curl_multi_code(code, CURLMOPT_PIPELINING); + code = curl_multi_setopt(multi_handle, + CURLMOPT_MAX_HOST_CONNECTIONS, + 0L); + check_curl_multi_code(code, CURLMOPT_MAX_HOST_CONNECTIONS); + code = curl_multi_setopt(multi_handle, + CURLMOPT_MAX_TOTAL_CONNECTIONS, + long(options.mConnectionLimit)); + check_curl_multi_code(code, CURLMOPT_MAX_TOTAL_CONNECTIONS); + } + } + else if (! mDirtyPolicy[policy_class]) + { + // Mark policy dirty and request a stall in the policy. + // When policy goes idle, we'll re-invoke this method + // and perform the change. Don't allow this thread to + // sleep while we're waiting for quiescence, we'll just + // stop processing. + mDirtyPolicy[policy_class] = true; + policy.stallPolicy(policy_class, true); + } +} + // --------------------------------------- // Free functions @@ -424,9 +500,9 @@ void check_curl_multi_code(CURLMcode code, int curl_setopt_option) { if (CURLM_OK != code) { - LL_WARNS("CoreHttp") << "libcurl multi error detected: " << curl_multi_strerror(code) - << ", curl_multi_setopt option: " << curl_setopt_option - << LL_ENDL; + LL_WARNS(LOG_CORE) << "libcurl multi error detected: " << curl_multi_strerror(code) + << ", curl_multi_setopt option: " << curl_setopt_option + << LL_ENDL; } } @@ -435,8 +511,8 @@ void check_curl_multi_code(CURLMcode code) { if (CURLM_OK != code) { - LL_WARNS("CoreHttp") << "libcurl multi error detected: " << curl_multi_strerror(code) - << LL_ENDL; + LL_WARNS(LOG_CORE) << "libcurl multi error detected: " << curl_multi_strerror(code) + << LL_ENDL; } } -- cgit v1.2.3 From 0c20beda6800149ee71a307ca4e943b5bba56908 Mon Sep 17 00:00:00 2001 From: Monty Brandenberg Date: Thu, 4 Sep 2014 16:57:44 -0400 Subject: Pipelining work. Extend transfer timeout by the pipeline depth as transfers can appear delayed with deep pipelining and more requests in the pool. Added bad HTTP status error (typically getting a 0 back as HTTP status from libcurl) to the list of retryable errors. There's a response stream problem with libcurl and pipelining that induces this problem. Retrying helps but may not be entirely safe. Watch bug 1420 on the libcurl sourceforge bug tracker. Extend options of test/example program to include un-ranged requests. Document the excessive data transfer induced when ranged requests are disabled. This is an abnormal mode for very rare users so we'll just eat that for now. --- indra/llcorehttp/_httplibcurl.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'indra/llcorehttp/_httplibcurl.cpp') diff --git a/indra/llcorehttp/_httplibcurl.cpp b/indra/llcorehttp/_httplibcurl.cpp index b46833a1f3..cfbe0fd2bb 100755 --- a/indra/llcorehttp/_httplibcurl.cpp +++ b/indra/llcorehttp/_httplibcurl.cpp @@ -217,8 +217,17 @@ void HttpLibcurl::addOp(HttpOpRequest * op) } // Make the request live - curl_multi_add_handle(mMultiHandles[op->mReqPolicy], op->mCurlHandle); + CURLMcode code; + code = curl_multi_add_handle(mMultiHandles[op->mReqPolicy], op->mCurlHandle); + if (CURLM_OK != code) + { + // *TODO: Better cleanup and recovery but not much we can do here. + check_curl_multi_code(code); + return; + } op->mCurlActive = true; + mActiveOps.insert(op); + ++mActiveHandles[op->mReqPolicy]; if (op->mTracing > HTTP_TRACE_OFF) { @@ -230,10 +239,6 @@ void HttpLibcurl::addOp(HttpOpRequest * op) << ", Readies: " << policy.getReadyCount(op->mReqPolicy) << LL_ENDL; } - - // On success, make operation active - mActiveOps.insert(op); - ++mActiveHandles[op->mReqPolicy]; } -- cgit v1.2.3 From 79ab7c20703c092a4416a4f9a885e0246fc17ee0 Mon Sep 17 00:00:00 2001 From: Monty Brandenberg Date: Fri, 19 Sep 2014 15:34:09 -0400 Subject: Introduce libcurl handle cache. Create a private cache of used handles and a fast handle factory that's thread- correct. --- indra/llcorehttp/_httplibcurl.cpp | 83 +++++++++++++++++++++++++++++++++++++-- 1 file changed, 80 insertions(+), 3 deletions(-) (limited to 'indra/llcorehttp/_httplibcurl.cpp') diff --git a/indra/llcorehttp/_httplibcurl.cpp b/indra/llcorehttp/_httplibcurl.cpp index cfbe0fd2bb..81b44ab90b 100755 --- a/indra/llcorehttp/_httplibcurl.cpp +++ b/indra/llcorehttp/_httplibcurl.cpp @@ -51,6 +51,7 @@ namespace LLCore HttpLibcurl::HttpLibcurl(HttpService * service) : mService(service), + mHandleCache(), mPolicyCount(0), mMultiHandles(NULL), mActiveHandles(NULL), @@ -61,7 +62,7 @@ HttpLibcurl::HttpLibcurl(HttpService * service) HttpLibcurl::~HttpLibcurl() { shutdown(); - + mService = NULL; } @@ -279,7 +280,7 @@ void HttpLibcurl::cancelRequest(HttpOpRequest * op) // Detach from multi and recycle handle curl_multi_remove_handle(mMultiHandles[op->mReqPolicy], op->mCurlHandle); - curl_easy_cleanup(op->mCurlHandle); + mHandleCache.freeHandle(op->mCurlHandle); op->mCurlHandle = NULL; // Tracing @@ -356,7 +357,7 @@ bool HttpLibcurl::completeRequest(CURLM * multi_handle, CURL * handle, CURLcode // Detach from multi and recycle handle curl_multi_remove_handle(multi_handle, handle); - curl_easy_cleanup(handle); + mHandleCache.freeHandle(op->mCurlHandle); op->mCurlHandle = NULL; // Tracing @@ -471,6 +472,82 @@ void HttpLibcurl::policyUpdated(int policy_class) } } +// --------------------------------------- +// HttpLibcurl::HandleCache +// --------------------------------------- + +HttpLibcurl::HandleCache::HandleCache() + : mHandleTemplate(NULL) +{ + mCache.reserve(50); +} + + +HttpLibcurl::HandleCache::~HandleCache() +{ + if (mHandleTemplate) + { + curl_easy_cleanup(mHandleTemplate); + mHandleTemplate = NULL; + } + + for (handle_cache_t::iterator it(mCache.begin()); mCache.end() != it; ++it) + { + curl_easy_cleanup(*it); + } + mCache.clear(); +} + + +CURL * HttpLibcurl::HandleCache::getHandle() +{ + CURL * ret(NULL); + + if (! mCache.empty()) + { + // Fastest path to handle + ret = mCache.back(); + mCache.pop_back(); + } + else if (mHandleTemplate) + { + // Still fast path + ret = curl_easy_duphandle(mHandleTemplate); + } + else + { + // When all else fails + ret = curl_easy_init(); + } + + return ret; +} + + +void HttpLibcurl::HandleCache::freeHandle(CURL * handle) +{ + if (! handle) + { + return; + } + + curl_easy_reset(handle); + if (! mHandleTemplate) + { + // Save the first freed handle as a template. + mHandleTemplate = handle; + } + else + { + // Otherwise add it to the cache + if (mCache.size() >= mCache.capacity()) + { + mCache.reserve(mCache.capacity() + 50); + } + mCache.push_back(handle); + } +} + // --------------------------------------- // Free functions -- cgit v1.2.3