tei/MapLibreParticleLab
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[core] Remove internal threading from OnlineFileRequest

Browse Source 
There's no need to do the work that OnlineFileRequest does on a separate thread from the DefaultFileSource thread, and having AsyncTasks proxy to other tasks across a thread boundary adds needless complexity.
This commit is contained in:
John Firebaugh
2016-03-24 11:33:09 -07:00
parent 7bb74ac8db
commit 6d88a23a60
3 changed files with 64 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
include/mbgl/storage/online_file_source.hpp
View File

@@ -5,10 +5,6 @@
namespace mbgl {
namespace util {
template <typename T> class Thread;
} // namespace util
class OnlineFileSource : public FileSource {
public:
OnlineFileSource();
@@ -20,10 +16,10 @@ public:
std::unique_ptr<AsyncRequest> request(const Resource&, Callback) override;
private:
friend class OnlineFileRequestImpl;
friend class OnlineFileRequest;
class Impl;
const std::unique_ptr<util::Thread<Impl>> thread;
const std::unique_ptr<Impl> impl;
std::string accessToken;
};

129
platform/default/online_file_source.cpp
View File

@@ -22,18 +22,18 @@
namespace mbgl {
class OnlineFileRequestImpl : public util::noncopyable {
class OnlineFileRequest : public AsyncRequest {
public:
using Callback = std::function<void (Response)>;
OnlineFileRequestImpl(AsyncRequest*, const Resource&, Callback, OnlineFileSource::Impl&);
~OnlineFileRequestImpl();
OnlineFileRequest(const Resource&, Callback, OnlineFileSource::Impl&);
~OnlineFileRequest();
void networkIsReachableAgain(OnlineFileSource::Impl&);
void schedule(OnlineFileSource::Impl&, optional<SystemTimePoint> expires);
void completed(OnlineFileSource::Impl&, Response);
void networkIsReachableAgain();
void schedule(optional<SystemTimePoint> expires);
void completed(Response);
AsyncRequest* key;
OnlineFileSource::Impl& impl;
Resource resource;
HTTPRequestBase* request = nullptr;
util::Timer timer;
@@ -52,8 +52,7 @@ public:
class OnlineFileSource::Impl {
public:
// Dummy parameter is a workaround for a gcc 4.9 bug.
Impl(int) {
Impl() {
NetworkStatus::Subscribe(&reachability);
}
@@ -61,16 +60,16 @@ public:
NetworkStatus::Unsubscribe(&reachability);
}
void request(AsyncRequest* key, Resource resource, Callback callback) {
allRequests[key] = std::make_unique<OnlineFileRequestImpl>(key, resource, callback, *this);
void add(OnlineFileRequest* request) {
allRequests.insert(request);
}
void cancel(AsyncRequest* key) {
allRequests.erase(key);
if (activeRequests.erase(key)) {
void remove(OnlineFileRequest* request) {
allRequests.erase(request);
if (activeRequests.erase(request)) {
activatePendingRequest();
} else {
auto it = pendingRequestsMap.find(key);
auto it = pendingRequestsMap.find(request);
if (it != pendingRequestsMap.end()) {
pendingRequestsList.erase(it->second);
pendingRequestsMap.erase(it);
@@ -78,30 +77,30 @@ public:
}
}
void activateOrQueueRequest(OnlineFileRequestImpl* impl) {
assert(allRequests.find(impl->key) != allRequests.end());
assert(activeRequests.find(impl->key) == activeRequests.end());
assert(!impl->request);
void activateOrQueueRequest(OnlineFileRequest* request) {
assert(allRequests.find(request) != allRequests.end());
assert(activeRequests.find(request) == activeRequests.end());
assert(!request->request);
if (activeRequests.size() >= HTTPContextBase::maximumConcurrentRequests()) {
queueRequest(impl);
queueRequest(request);
} else {
activateRequest(impl);
activateRequest(request);
}
}
void queueRequest(OnlineFileRequestImpl* impl) {
auto it = pendingRequestsList.insert(pendingRequestsList.end(), impl->key);
pendingRequestsMap.emplace(impl->key, std::move(it));
void queueRequest(OnlineFileRequest* request) {
auto it = pendingRequestsList.insert(pendingRequestsList.end(), request);
pendingRequestsMap.emplace(request, std::move(it));
}
void activateRequest(OnlineFileRequestImpl* impl) {
activeRequests.insert(impl->key);
impl->request = httpContext->createRequest(impl->resource, [=] (Response response) {
impl->request = nullptr;
activeRequests.erase(impl->key);
void activateRequest(OnlineFileRequest* request) {
activeRequests.insert(request);
request->request = httpContext->createRequest(request->resource, [=] (Response response) {
activeRequests.erase(request);
activatePendingRequest();
impl->completed(*this, response);
request->request = nullptr;
request->completed(response);
});
}
@@ -110,20 +109,18 @@ public:
return;
}
AsyncRequest* key = pendingRequestsList.front();
OnlineFileRequest* request = pendingRequestsList.front();
pendingRequestsList.pop_front();
pendingRequestsMap.erase(key);
pendingRequestsMap.erase(request);
auto it = allRequests.find(key);
assert(it != allRequests.end());
activateRequest(it->second.get());
activateRequest(request);
}
private:
void networkIsReachableAgain() {
for (auto& req : allRequests) {
req.second->networkIsReachableAgain(*this);
for (auto& request : allRequests) {
request->networkIsReachableAgain();
}
}
@@ -138,18 +135,17 @@ private:
* Requests in any state are in `allRequests`. Requests in the pending state are in
* `pendingRequests`. Requests in the active state are in `activeRequests`.
*/
std::unordered_map<AsyncRequest*, std::unique_ptr<OnlineFileRequestImpl>> allRequests;
std::list<AsyncRequest*> pendingRequestsList;
std::unordered_map<AsyncRequest*, std::list<AsyncRequest*>::iterator> pendingRequestsMap;
std::unordered_set<AsyncRequest*> activeRequests;
std::unordered_set<OnlineFileRequest*> allRequests;
std::list<OnlineFileRequest*> pendingRequestsList;
std::unordered_map<OnlineFileRequest*, std::list<OnlineFileRequest*>::iterator> pendingRequestsMap;
std::unordered_set<OnlineFileRequest*> activeRequests;
const std::unique_ptr<HTTPContextBase> httpContext { HTTPContextBase::createContext() };
util::AsyncTask reachability { std::bind(&Impl::networkIsReachableAgain, this) };
};
OnlineFileSource::OnlineFileSource()
: thread(std::make_unique<util::Thread<Impl>>(
util::ThreadContext{ "OnlineFileSource", util::ThreadType::Unknown, util::ThreadPriority::Low }, 0)) {
: impl(std::make_unique<Impl>()) {
}
OnlineFileSource::~OnlineFileSource() = default;
@@ -183,37 +179,25 @@ std::unique_ptr<AsyncRequest> OnlineFileSource::request(const Resource& resource
break;
}
class OnlineFileRequest : public AsyncRequest {
public:
OnlineFileRequest(Resource resource_, FileSource::Callback callback_, util::Thread<OnlineFileSource::Impl>& thread_)
: thread(thread_),
workRequest(thread.invokeWithCallback(&OnlineFileSource::Impl::request, callback_, this, resource_)) {
}
~OnlineFileRequest() override {
thread.invoke(&OnlineFileSource::Impl::cancel, this);
}
util::Thread<OnlineFileSource::Impl>& thread;
std::unique_ptr<AsyncRequest> workRequest;
};
return std::make_unique<OnlineFileRequest>(res, callback, *thread);
return std::make_unique<OnlineFileRequest>(res, callback, *impl);
}
OnlineFileRequestImpl::OnlineFileRequestImpl(AsyncRequest* key_, const Resource& resource_, Callback callback_, OnlineFileSource::Impl& impl)
: key(key_),
OnlineFileRequest::OnlineFileRequest(const Resource& resource_, Callback callback_, OnlineFileSource::Impl& impl_)
: impl(impl_),
resource(resource_),
callback(std::move(callback_)) {
impl.add(this);
// Force an immediate first request if we don't have an expiration time.
if (resource.priorExpires) {
schedule(impl, resource.priorExpires);
schedule(resource.priorExpires);
} else {
schedule(impl, SystemClock::now());
schedule(SystemClock::now());
}
}
OnlineFileRequestImpl::~OnlineFileRequestImpl() {
OnlineFileRequest::~OnlineFileRequest() {
impl.remove(this);
if (request) {
request->cancel();
}
@@ -278,7 +262,7 @@ SystemTimePoint interpolateExpiration(const SystemTimePoint& current,
return now + std::max<SystemDuration>(delta, util::CLOCK_SKEW_RETRY_TIMEOUT);
}
void OnlineFileRequestImpl::schedule(OnlineFileSource::Impl& impl, optional<SystemTimePoint> expires) {
void OnlineFileRequest::schedule(optional<SystemTimePoint> expires) {
if (request) {
// There's already a request in progress; don't start another one.
return;
@@ -307,7 +291,7 @@ void OnlineFileRequestImpl::schedule(OnlineFileSource::Impl& impl, optional<Syst
});
}
void OnlineFileRequestImpl::completed(OnlineFileSource::Impl& impl, Response response) {
void OnlineFileRequest::completed(Response response) {
// If we didn't get various caching headers in the response, continue using the
// previous values. Otherwise, update the previous values to the new values.
@@ -345,15 +329,20 @@ void OnlineFileRequestImpl::completed(OnlineFileSource::Impl& impl, Response res
failedRequestReason = Response::Error::Reason::Success;
}
callback(response);
schedule(impl, response.expires);
schedule(response.expires);
// Calling the callback may result in `this` being deleted. It needs to be done last,
// and needs to make a local copy of the callback to ensure that it remains valid for
// the duration of the call.
auto callback_ = callback;
callback_(response);
}
void OnlineFileRequestImpl::networkIsReachableAgain(OnlineFileSource::Impl& impl) {
void OnlineFileRequest::networkIsReachableAgain() {
// We need all requests to fail at least once before we are going to start retrying
// them, and we only immediately restart request that failed due to connection issues.
if (failedRequestReason == Response::Error::Reason::Connection) {
schedule(impl, SystemClock::now());
schedule(SystemClock::now());
}
}

3
test/map/map_context.cpp
View File

@@ -6,10 +6,13 @@
#include <mbgl/platform/default/headless_display.hpp>
#include <mbgl/storage/online_file_source.hpp>
#include <mbgl/util/thread.hpp>
#include <mbgl/util/run_loop.hpp>
using namespace mbgl;
TEST(MapContext, DoubleStyleLoad) {
util::RunLoop loop;
std::shared_ptr<HeadlessDisplay> display = std::make_shared<HeadlessDisplay>();
HeadlessView view(display, 1, 512, 512);
OnlineFileSource fileSource;