-
Notifications
You must be signed in to change notification settings - Fork 95
Add Support HTTP Multiplexing #3101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -217,6 +217,10 @@ class HttpRequest | |
| /// Per-class only | ||
| PO_PIPELINING_DEPTH, | ||
|
|
||
| /// If it is 1, enable HTTP Multiplexing. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. More explanation about this API. The 'public' API presented here is not meant to be a facade for libcurl. At the time this was written and also possibly for future purposes, libcurl might be replaced by another library. (For Sansar, we had our own low-level libcurl-like library which could have slotted in.) The 'public' API avoided using libcurl definitions. Instead, this API is meant to describe how the viewer wants to use HTTP requests and how HTTP traffic should be managed and shaped. The intent is to have this description be as minimal as possible. If a concept isn't used by the viewer, it won't appear here. Internal APIs, such as _httplibcurl.*, are the translation layer between the public API and whatever is going on below. An adapter layer. There's only one concrete adapter at this point but that is okay. And so we have the comment above:
That is still correct. The public API describes an abstract usage and a pipelining depth of 2 or more enables pipelining. And so the code above in _httplibcurl is no longer correct. What should this API be in the http/2 world? Well, we don't know yet. We haven't stood up the infrastructure to support it. But we would probably want to modify llcorehttp differently. PO_PIPELINING_DEPTH might instead be changed to PO_REQUEST_CONCURRENCY covering both the http/1 (pipelining) and http/2 (multiplexing) cases with added explicit enables/disables for pipelining and http/2. However, we may find that we need independent http/2 parameters for the policy classes. (There's another weak reason to keep existing API definitions as-is: we use a copy of this code in server-side binaries and subtle changes may not be caught. This isn't meant to stop changes and progress in the viewer. But when all other considerations are equal, keeping an API spec has a benefit to us.) |
||
| /// Per-class only | ||
| PO_MULTIPLEXING_MODE, | ||
|
|
||
| /// Controls whether client-side throttling should be | ||
| /// performed on this policy class. Positive values | ||
| /// enable throttling and specify the request rate | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -122,7 +122,8 @@ static void ssl_verification_changed(); | |
| LLAppCoreHttp::HttpClass::HttpClass() | ||
| : mPolicy(LLCore::HttpRequest::DEFAULT_POLICY_ID), | ||
| mConnLimit(0U), | ||
| mPipelined(false) | ||
| mPipelined(false), | ||
| mMultiplexing(false) | ||
| {} | ||
|
|
||
|
|
||
|
|
@@ -131,7 +132,8 @@ LLAppCoreHttp::LLAppCoreHttp() | |
| mStopHandle(LLCORE_HTTP_HANDLE_INVALID), | ||
| mStopRequested(0.0), | ||
| mStopped(false), | ||
| mPipelined(true) | ||
| mPipelined(true), | ||
| mMultiplexing(false) | ||
| {} | ||
|
|
||
|
|
||
|
|
@@ -281,6 +283,15 @@ void LLAppCoreHttp::init() | |
| LL_INFOS("Init") << "HTTP Pipelining " << (mPipelined ? "enabled" : "disabled") << "!" << LL_ENDL; | ||
| } | ||
|
|
||
| // Global multiplexing setting | ||
| static const std::string http_multiplexing("UseHTTP2Multiplexing"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personally I would have merged "HttpPipelining" and "UseHTTP2Multiplexing" into some kind of 'mode switch' since they are sort of exclusive. |
||
| if (gSavedSettings.controlExists(http_multiplexing)) | ||
| { | ||
| // Default to true (in ctor) if absent. | ||
| mMultiplexing = gSavedSettings.getBOOL(http_multiplexing); | ||
| LL_INFOS("Init") << "HTTP Multiplexing " << (mMultiplexing ? "enabled" : "disabled") << "!" << LL_ENDL; | ||
| } | ||
|
|
||
| // Register signals for settings and state changes | ||
| for (int i(0); i < LL_ARRAY_SIZE(init_data); ++i) | ||
| { | ||
|
|
@@ -451,6 +462,38 @@ void LLAppCoreHttp::refreshSettings(bool initial) | |
| } | ||
| } | ||
|
|
||
| // Multiplexing changes | ||
| if (initial) | ||
| { | ||
| const bool to_multiplex(mMultiplexing); | ||
| if (to_multiplex != mHttpClasses[app_policy].mMultiplexing) | ||
| { | ||
| // Pipeline election changing, set dynamic option via request | ||
|
|
||
| LLCore::HttpHandle handle; | ||
| const long new_mode(to_multiplex ? 1 : 0); | ||
|
|
||
| handle = mRequest->setPolicyOption(LLCore::HttpRequest::PO_MULTIPLEXING_MODE, | ||
| mHttpClasses[app_policy].mPolicy, | ||
| new_mode, | ||
| LLCore::HttpHandler::ptr_t()); | ||
| if (LLCORE_HTTP_HANDLE_INVALID == handle) | ||
| { | ||
| status = mRequest->getStatus(); | ||
| LL_WARNS("Init") << "Unable to set " << init_data[i].mUsage | ||
| << " pipelining. Reason: " << status.toString() | ||
| << LL_ENDL; | ||
| } | ||
| else | ||
| { | ||
| LL_DEBUGS("Init") << "Changed " << init_data[i].mUsage | ||
| << " multiplexing. New mode: " << (new_mode ? "enable" : "disable") | ||
| << LL_ENDL; | ||
| mHttpClasses[app_policy].mMultiplexing = to_multiplex; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Get target connection concurrency value | ||
| U32 setting(init_data[i].mDefault); | ||
| if (! init_data[i].mKey.empty() && gSavedSettings.controlExists(init_data[i].mKey)) | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.