From d238341afaecedfe227141126c4c35dcde4a0671 Mon Sep 17 00:00:00 2001
From: Monty Brandenberg <monty@lindenlab.com>
Date: Mon, 16 Jul 2012 11:53:04 -0400
Subject: [PATCH] SH-3189 Remove/improve naive data structures When releasing
HTTP waiters, avoid unnecessary sort activity. For Content-Type in responses,
let libcurl do the work and removed my parsing of headers. Drop
Content-Encoding as libcurl will deal with that. If anyone is interested,
they can parse.
---
indra/llcorehttp/_httplibcurl.cpp | 18 +++++++-
indra/llcorehttp/_httpoprequest.cpp | 48 ++-------------------
indra/llcorehttp/_httpoprequest.h | 2 -
indra/llcorehttp/httpcommon.cpp | 3 +-
indra/llcorehttp/httpcommon.h | 5 ++-
indra/llcorehttp/httpresponse.h | 9 ++--
indra/llcorehttp/tests/test_httprequest.hpp | 3 +-
indra/newview/lltexturefetch.cpp | 22 ++++++++--
8 files changed, 48 insertions(+), 62 deletions(-)
diff --git a/indra/llcorehttp/_httplibcurl.cpp b/indra/llcorehttp/_httplibcurl.cpp
index 3c69ae1c969..e031efbc91e 100644
--- a/indra/llcorehttp/_httplibcurl.cpp
+++ b/indra/llcorehttp/_httplibcurl.cpp
@@ -280,7 +280,23 @@ bool HttpLibcurl::completeRequest(CURLM * multi_handle, CURL * handle, CURLcode
int http_status(HTTP_OK);
curl_easy_getinfo(handle, CURLINFO_RESPONSE_CODE, &http_status);
- op->mStatus = LLCore::HttpStatus(http_status);
+ if (http_status >= 100 && http_status <= 999)
+ {
+ char * cont_type(NULL);
+ curl_easy_getinfo(handle, CURLINFO_CONTENT_TYPE, &cont_type);
+ if (cont_type)
+ {
+ op->mReplyConType = cont_type;
+ }
+ op->mStatus = HttpStatus(http_status);
+ }
+ else
+ {
+ LL_WARNS("CoreHttp") << "Invalid HTTP response code ("
+ << http_status << ") received from server."
+ << LL_ENDL;
+ op->mStatus = HttpStatus(HttpStatus::LLCORE, HE_INVALID_HTTP_STATUS);
+ }
}
// Detach from multi and recycle handle
diff --git a/indra/llcorehttp/_httpoprequest.cpp b/indra/llcorehttp/_httpoprequest.cpp
index 1854d7ada43..1a770f67be5 100644
--- a/indra/llcorehttp/_httpoprequest.cpp
+++ b/indra/llcorehttp/_httpoprequest.cpp
@@ -228,7 +228,7 @@ void HttpOpRequest::visitNotifier(HttpRequest * request)
// Got an explicit offset/length in response
response->setRange(mReplyOffset, mReplyLength, mReplyFullLength);
}
- response->setContent(mReplyConType, mReplyConEncode);
+ response->setContentType(mReplyConType);
mUserHandler->onCompleted(static_cast<HttpHandle>(this), response);
@@ -316,7 +316,7 @@ void HttpOpRequest::setupCommon(HttpRequest::policy_t policy_id,
HttpOptions * options,
HttpHeaders * headers)
{
- mProcFlags = PF_SCAN_CONTENT_HEADERS; // Always scan for content headers
+ mProcFlags = 0U;
mReqPolicy = policy_id;
mReqPriority = priority;
mReqURL = url;
@@ -379,7 +379,6 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service)
mReplyHeaders = NULL;
}
mReplyConType.clear();
- mReplyConEncode.clear();
// *FIXME: better error handling later
HttpStatus status;
@@ -542,7 +541,7 @@ HttpStatus HttpOpRequest::prepareRequest(HttpService * service)
}
curl_easy_setopt(mCurlHandle, CURLOPT_HTTPHEADER, mCurlHeaders);
- if (mProcFlags & (PF_SCAN_RANGE_HEADER | PF_SAVE_HEADERS | PF_SCAN_CONTENT_HEADERS))
+ if (mProcFlags & (PF_SCAN_RANGE_HEADER | PF_SAVE_HEADERS))
{
curl_easy_setopt(mCurlHandle, CURLOPT_HEADERFUNCTION, headerCallback);
curl_easy_setopt(mCurlHandle, CURLOPT_HEADERDATA, this);
@@ -620,8 +619,6 @@ size_t HttpOpRequest::headerCallback(void * data, size_t size, size_t nmemb, voi
op->mReplyOffset = 0;
op->mReplyLength = 0;
op->mReplyFullLength = 0;
- op->mReplyConType.clear();
- op->mReplyConEncode.clear();
op->mStatus = HttpStatus();
if (op->mReplyHeaders)
{
@@ -688,45 +685,6 @@ size_t HttpOpRequest::headerCallback(void * data, size_t size, size_t nmemb, voi
}
}
- // Detect and parse 'Content-Type' and 'Content-Encoding' headers
- if (op->mProcFlags & PF_SCAN_CONTENT_HEADERS)
- {
- if (wanted_hdr_size > con_type_line_len &&
- ! os_strncasecmp(hdr_data, con_type_line, con_type_line_len))
- {
- // Found 'Content-Type:', extract single-token value
- std::string rhs(hdr_data + con_type_line_len, wanted_hdr_size - con_type_line_len);
- std::string::size_type begin(0), end(rhs.size()), pos;
-
- if ((pos = rhs.find_first_not_of(hdr_whitespace)) != std::string::npos)
- {
- begin = pos;
- }
- if ((pos = rhs.find_first_of(hdr_whitespace, begin)) != std::string::npos)
- {
- end = pos;
- }
- op->mReplyConType.assign(rhs, begin, end - begin);
- }
- else if (wanted_hdr_size > con_enc_line_len &&
- ! os_strncasecmp(hdr_data, con_enc_line, con_enc_line_len))
- {
- // Found 'Content-Encoding:', extract single-token value
- std::string rhs(hdr_data + con_enc_line_len, wanted_hdr_size - con_enc_line_len);
- std::string::size_type begin(0), end(rhs.size()), pos;
-
- if ((pos = rhs.find_first_not_of(hdr_whitespace)) != std::string::npos)
- {
- begin = pos;
- }
- if ((pos = rhs.find_first_of(hdr_whitespace, begin)) != std::string::npos)
- {
- end = pos;
- }
- op->mReplyConEncode.assign(rhs, begin, end - begin);
- }
- }
-
return hdr_size;
}
diff --git a/indra/llcorehttp/_httpoprequest.h b/indra/llcorehttp/_httpoprequest.h
index 200b925c4ec..36dc5dc8769 100644
--- a/indra/llcorehttp/_httpoprequest.h
+++ b/indra/llcorehttp/_httpoprequest.h
@@ -138,7 +138,6 @@ class HttpOpRequest : public HttpOperation
unsigned int mProcFlags;
static const unsigned int PF_SCAN_RANGE_HEADER = 0x00000001U;
static const unsigned int PF_SAVE_HEADERS = 0x00000002U;
- static const unsigned int PF_SCAN_CONTENT_HEADERS = 0x00000004U;
public:
// Request data
@@ -165,7 +164,6 @@ class HttpOpRequest : public HttpOperation
size_t mReplyFullLength;
HttpHeaders * mReplyHeaders;
std::string mReplyConType;
- std::string mReplyConEncode;
// Policy data
int mPolicyRetries;
diff --git a/indra/llcorehttp/httpcommon.cpp b/indra/llcorehttp/httpcommon.cpp
index 1b189763599..f2fcbf77a3b 100644
--- a/indra/llcorehttp/httpcommon.cpp
+++ b/indra/llcorehttp/httpcommon.cpp
@@ -69,7 +69,8 @@ std::string HttpStatus::toString() const
"Request handle not found",
"Invalid datatype for argument or option",
"Option has not been explicitly set",
- "Option is not dynamic and must be set early"
+ "Option is not dynamic and must be set early",
+ "Invalid HTTP status code received from server"
};
static const int llcore_errors_count(sizeof(llcore_errors) / sizeof(llcore_errors[0]));
diff --git a/indra/llcorehttp/httpcommon.h b/indra/llcorehttp/httpcommon.h
index 576a113e549..dd5798edf9d 100644
--- a/indra/llcorehttp/httpcommon.h
+++ b/indra/llcorehttp/httpcommon.h
@@ -149,7 +149,10 @@ enum HttpError
HE_OPT_NOT_SET = 7,
// Option not dynamic, must be set during init phase
- HE_OPT_NOT_DYNAMIC = 8
+ HE_OPT_NOT_DYNAMIC = 8,
+
+ // Invalid HTTP status code returned by server
+ HE_INVALID_HTTP_STATUS = 9
}; // end enum HttpError
diff --git a/indra/llcorehttp/httpresponse.h b/indra/llcorehttp/httpresponse.h
index 3d35050c178..4a481db6ac5 100644
--- a/indra/llcorehttp/httpresponse.h
+++ b/indra/llcorehttp/httpresponse.h
@@ -134,16 +134,14 @@ class HttpResponse : public LLCoreInt::RefCounted
}
///
- void getContent(std::string & con_type, std::string & con_encode) const
+ const std::string & getContentType() const
{
- con_type = mContentType;
- con_encode = mContentEncoding;
+ return mContentType;
}
- void setContent(const std::string & con_type, const std::string & con_encode)
+ void setContentType(const std::string & con_type)
{
mContentType = con_type;
- mContentEncoding = con_encode;
}
protected:
@@ -155,7 +153,6 @@ class HttpResponse : public LLCoreInt::RefCounted
BufferArray * mBufferArray;
HttpHeaders * mHeaders;
std::string mContentType;
- std::string mContentEncoding;
};
diff --git a/indra/llcorehttp/tests/test_httprequest.hpp b/indra/llcorehttp/tests/test_httprequest.hpp
index 0f9eb939969..1acf4f9d4b1 100644
--- a/indra/llcorehttp/tests/test_httprequest.hpp
+++ b/indra/llcorehttp/tests/test_httprequest.hpp
@@ -147,8 +147,7 @@ public:
if (! mCheckContentType.empty())
{
ensure("Response required with content type check", response != NULL);
- std::string con_type, con_enc;
- response->getContent(con_type, con_enc);
+ std::string con_type(response->getContentType());
ensure("Content-Type as expected (" + mCheckContentType + ")",
mCheckContentType == con_type);
}
diff --git a/indra/newview/lltexturefetch.cpp b/indra/newview/lltexturefetch.cpp
index 8314031f145..225ea46558c 100755
--- a/indra/newview/lltexturefetch.cpp
+++ b/indra/newview/lltexturefetch.cpp
@@ -3342,6 +3342,17 @@ void LLTextureFetch::removeHttpWaiter(const LLUUID & tid)
mNetworkQueueMutex.unlock(); // -Mfnq
}
+// Release as many requests as permitted from the WAIT_HTTP_RESOURCE2
+// state to the SEND_HTTP_REQ state based on their current priority.
+//
+// This data structures and code associated with this looks a bit
+// indirect and naive but it's done in the name of safety. An
+// ordered container may become invalid from time to time due to
+// priority changes caused by actions in other threads. State itself
+// could also suffer the same fate with canceled operations. Even
+// done this way, I'm not fully trusting we're truly safe. This
+// module is due for a major refactoring and we'll deal with it then.
+//
// Threads: Ttf
// Locks: -Mw (must not hold any worker when called)
void LLTextureFetch::releaseHttpWaiters()
@@ -3384,12 +3395,15 @@ void LLTextureFetch::releaseHttpWaiters()
tids2.push_back(worker);
}
}
-
- // Sort into priority order
- LLTextureFetchWorker::Compare compare;
- std::sort(tids2.begin(), tids2.end(), compare);
tids.clear();
+ // Sort into priority order, if necessary and only as much as needed
+ if (tids2.size() > mHttpSemaphore)
+ {
+ LLTextureFetchWorker::Compare compare;
+ std::partial_sort(tids2.begin(), tids2.begin() + mHttpSemaphore, tids2.end(), compare);
+ }
+
// Release workers up to the high water mark. Since we aren't
// holding any locks at this point, we can be in competition
// with other callers. Do defensive things like getting
--
GitLab