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

[core] Remove request coalescing in OnlineFileSource

Browse Source
This commit is contained in:
John Firebaugh
2016-01-08 14:37:31 -08:00
parent 716a9c92cb
commit 618902e06c
5 changed files with 26 additions and 264 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -24,7 +24,7 @@ private:
friend class OnlineFileRequest;
friend class OnlineFileRequestImpl;
void cancel(const Resource&, FileRequest*);
void cancel(FileRequest*);
class Impl;
const std::unique_ptr<util::Thread<Impl>> thread;

127
platform/default/online_file_source.cpp
View File

@@ -24,19 +24,15 @@ class RequestBase;
class OnlineFileRequest : public FileRequest {
public:
OnlineFileRequest(const Resource& resource_,
OnlineFileSource& fileSource_)
: resource(resource_),
fileSource(fileSource_) {
OnlineFileRequest(OnlineFileSource& fileSource_)
: fileSource(fileSource_) {
}
~OnlineFileRequest() {
fileSource.cancel(resource, this);
fileSource.cancel(this);
}
Resource resource;
OnlineFileSource& fileSource;
std::unique_ptr<WorkRequest> workRequest;
};
@@ -44,14 +40,9 @@ class OnlineFileRequestImpl : public util::noncopyable {
public:
using Callback = std::function<void (Response)>;
OnlineFileRequestImpl(const Resource&);
OnlineFileRequestImpl(const Resource&, Callback, OnlineFileSource::Impl&);
~OnlineFileRequestImpl();
// Observer accessors.
void addObserver(FileRequest*, Callback, OnlineFileSource::Impl&);
void removeObserver(FileRequest*);
bool hasObservers() const;
void networkIsReachableAgain(OnlineFileSource::Impl&);
private:
@@ -62,13 +53,10 @@ private:
std::unique_ptr<WorkRequest> cacheRequest;
RequestBase* realRequest = nullptr;
util::Timer realRequestTimer;
Callback callback;
// Stores a set of all observing Request objects.
std::unordered_map<FileRequest*, Callback> observers;
// The current response data. We're storing it because we can satisfy requests for the same
// resource directly by returning this response object. We also need it to create conditional
// HTTP requests, and to check whether new responses we got changed any data.
// The current response data. Used to create conditional HTTP requests, and to check whether
// new responses we got changed any data.
std::shared_ptr<const Response> response;
// Counts the number of subsequent failed requests. We're using this value for exponential
@@ -86,12 +74,12 @@ public:
void networkIsReachableAgain();
void add(Resource, FileRequest*, Callback);
void cancel(Resource, FileRequest*);
void cancel(FileRequest*);
private:
friend OnlineFileRequestImpl;
std::unordered_map<Resource, std::unique_ptr<OnlineFileRequestImpl>, Resource::Hash> pending;
std::unordered_map<FileRequest*, std::unique_ptr<OnlineFileRequestImpl>> pending;
FileCache* const cache;
const std::unique_ptr<HTTPContextBase> httpContext;
util::AsyncTask reachability;
@@ -135,13 +123,13 @@ std::unique_ptr<FileRequest> OnlineFileSource::request(const Resource& resource,
}
Resource res { resource.kind, url };
auto req = std::make_unique<OnlineFileRequest>(res, *this);
auto req = std::make_unique<OnlineFileRequest>(*this);
req->workRequest = thread->invokeWithCallback(&Impl::add, callback, res, req.get());
return std::move(req);
}
void OnlineFileSource::cancel(const Resource& res, FileRequest* req) {
thread->invoke(&Impl::cancel, res, req);
void OnlineFileSource::cancel(FileRequest* req) {
thread->invoke(&Impl::cancel, req);
}
// ----- Impl -----
@@ -167,34 +155,23 @@ void OnlineFileSource::Impl::networkIsReachableAgain() {
}
void OnlineFileSource::Impl::add(Resource resource, FileRequest* req, Callback callback) {
auto& request = *pending.emplace(resource,
std::make_unique<OnlineFileRequestImpl>(resource)).first->second;
// Add this request as an observer so that it'll get notified when something about this
// request changes.
request.addObserver(req, callback, *this);
pending[req] = std::make_unique<OnlineFileRequestImpl>(resource, callback, *this);
}
void OnlineFileSource::Impl::cancel(Resource resource, FileRequest* req) {
auto it = pending.find(resource);
if (it != pending.end()) {
// If the number of dependent requests of the OnlineFileRequest drops to zero,
// cancel the request and remove it from the pending list.
auto& request = *it->second;
request.removeObserver(req);
if (!request.hasObservers()) {
pending.erase(it);
}
} else {
// There is no request for this URL anymore. Likely, the request already completed
// before we got around to process the cancelation request.
}
void OnlineFileSource::Impl::cancel(FileRequest* req) {
pending.erase(req);
}
// ----- OnlineFileRequest -----
OnlineFileRequestImpl::OnlineFileRequestImpl(const Resource& resource_)
: resource(resource_) {
OnlineFileRequestImpl::OnlineFileRequestImpl(const Resource& resource_, Callback callback_, OnlineFileSource::Impl& impl)
: resource(resource_),
callback(callback_) {
if (impl.cache) {
scheduleCacheRequest(impl);
} else {
scheduleRealRequest(impl);
}
}
OnlineFileRequestImpl::~OnlineFileRequestImpl() {
@@ -205,55 +182,6 @@ OnlineFileRequestImpl::~OnlineFileRequestImpl() {
// realRequestTimer and cacheRequest are automatically canceled upon destruction.
}
void OnlineFileRequestImpl::addObserver(FileRequest* req, Callback callback, OnlineFileSource::Impl& impl) {
if (response) {
// We've at least obtained a cache value, potentially we also got a final response.
// Before returning the existing response, update the `stale` flag if necessary.
if (!response->stale && response->isExpired()) {
// Create a new Response object with `stale = true`, but the same data, and
// replace the current request object we have.
// We're not immediately swapping the member variable because it's declared as `const`, and
// we first have to update the `stale` flag.
auto staleResponse = std::make_shared<Response>(*response);
staleResponse->stale = true;
response = staleResponse;
}
callback(*response);
if (response->stale && !realRequest) {
// We've returned a stale response; now make sure the requester also gets a fresh
// response eventually. It's possible that there's already a request in progress.
// Note that this will also trigger updates to all other existing listeners.
// Since we already have data, we're going to verify
scheduleRealRequest(impl);
} else {
// The response is still fresh (or there's already a request for refreshing the resource
// in progress), so there's nothing we need to do.
}
} else if (!cacheRequest && !realRequest) {
// There is no request in progress, and we don't have a response yet. This means we'll have
// to start the request ourselves.
if (impl.cache) {
scheduleCacheRequest(impl);
} else {
scheduleRealRequest(impl);
}
} else {
// There is a request in progress. We just have to wait.
}
observers.emplace(req, callback);
}
void OnlineFileRequestImpl::removeObserver(FileRequest* req) {
observers.erase(req);
}
bool OnlineFileRequestImpl::hasObservers() const {
return !observers.empty();
}
void OnlineFileRequestImpl::scheduleCacheRequest(OnlineFileSource::Impl& impl) {
// Check the cache for existing data so that we can potentially
// revalidate the information without having to redownload everything.
@@ -263,9 +191,7 @@ void OnlineFileRequestImpl::scheduleCacheRequest(OnlineFileSource::Impl& impl) {
if (response_) {
response_->stale = response_->isExpired();
response = response_;
for (auto& req : observers) {
req.second(*response);
}
callback(*response);
}
scheduleRealRequest(impl);
@@ -341,10 +267,7 @@ void OnlineFileRequestImpl::scheduleRealRequest(OnlineFileSource::Impl& impl, bo
failedRequests = 0;
}
for (auto& req : observers) {
req.second(*response);
}
callback(*response);
scheduleRealRequest(impl);
}, response);
});

2
test/storage/cache_revalidate.cpp
View File

@@ -49,7 +49,6 @@ TEST_F(Storage, CacheRevalidateSame) {
EXPECT_EQ(nullptr, res2.error);
EXPECT_EQ(false, res2.stale);
ASSERT_TRUE(res2.data.get());
EXPECT_EQ(res.data, res2.data);
EXPECT_EQ("Response", *res2.data);
// We use this to indicate that a 304 reply came back.
EXPECT_LT(Seconds::zero(), res2.expires);
@@ -111,7 +110,6 @@ TEST_F(Storage, CacheRevalidateModified) {
EXPECT_EQ(false, res2.stale);
ASSERT_TRUE(res2.data.get());
EXPECT_EQ("Response", *res2.data);
EXPECT_EQ(res.data, res2.data);
// We use this to indicate that a 304 reply came back.
EXPECT_LT(Seconds::zero(), res2.expires);
EXPECT_EQ(1420070400, res2.modified.count());

158
test/storage/http_coalescing.cpp
View File

@@ -1,158 +0,0 @@
#include "storage.hpp"
#include <mbgl/storage/online_file_source.hpp>
#include <mbgl/util/chrono.hpp>
#include <mbgl/util/run_loop.hpp>
TEST_F(Storage, HTTPCoalescing) {
SCOPED_TEST(HTTPCoalescing)
static int counter = 0;
const static int total = 4;
using namespace mbgl;
util::RunLoop loop;
OnlineFileSource fs(nullptr);
static const Response *reference = nullptr;
const auto complete = [&](const Response &res) {
counter++;
// Make sure all of the Response objects are the same.
if (!reference) {
reference = &res;
} else {
EXPECT_EQ(&res, reference);
}
EXPECT_EQ(nullptr, res.error);
ASSERT_TRUE(res.data.get());
EXPECT_EQ("Hello World!", *res.data);
EXPECT_EQ(Seconds::zero(), res.expires);
EXPECT_EQ(Seconds::zero(), res.modified);
EXPECT_EQ("", res.etag);
if (counter >= total) {
loop.stop();
HTTPCoalescing.finish();
}
};
const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/test" };
std::unique_ptr<FileRequest> reqs[total];
for (int i = 0; i < total; i++) {
reqs[i] = fs.request(resource, [&complete, &fs, &reqs, i] (Response res) {
reqs[i].reset();
complete(res);
});
}
loop.run();
}
TEST_F(Storage, HTTPMultiple) {
SCOPED_TEST(HTTPMultiple)
using namespace mbgl;
util::RunLoop loop;
OnlineFileSource fs(nullptr);
const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/test?expires=2147483647" };
std::unique_ptr<FileRequest> req1;
std::unique_ptr<FileRequest> req2;
req1 = fs.request(resource, [&] (Response res) {
// Do not cancel the request right away.
EXPECT_EQ(nullptr, res.error);
ASSERT_TRUE(res.data.get());
EXPECT_EQ("Hello World!", *res.data);
EXPECT_EQ(2147483647, res.expires.count());
EXPECT_EQ(Seconds::zero(), res.modified);
EXPECT_EQ("", res.etag);
// Start a second request for the same resource after the first one has been completed.
req2 = fs.request(resource, [&, res] (Response res2) {
// Make sure we get the same data as before.
EXPECT_EQ(res.data.get(), res2.data.get());
// Now cancel both requests after both have been notified.
req1.reset();
req2.reset();
EXPECT_EQ(nullptr, res2.error);
ASSERT_TRUE(res2.data.get());
EXPECT_EQ("Hello World!", *res2.data);
EXPECT_EQ(2147483647, res2.expires.count());
EXPECT_EQ(Seconds::zero(), res2.modified);
EXPECT_EQ("", res2.etag);
loop.stop();
HTTPMultiple.finish();
});
});
loop.run();
}
// Tests that we get stale responses from previous requests when requesting the same thing again.
TEST_F(Storage, HTTPStale) {
SCOPED_TEST(HTTPStale)
using namespace mbgl;
util::RunLoop loop;
OnlineFileSource fs(nullptr);
int updates = 0;
int stale = 0;
const Resource resource { Resource::Unknown, "http://127.0.0.1:3000/test" };
std::unique_ptr<FileRequest> req1;
std::unique_ptr<FileRequest> req2;
req1 = fs.request(resource, [&] (Response res) {
// Do not cancel the request right away.
EXPECT_EQ(nullptr, res.error);
ASSERT_TRUE(res.data.get());
EXPECT_EQ("Hello World!", *res.data);
EXPECT_EQ(false, res.stale);
EXPECT_EQ(Seconds::zero(), res.expires);
EXPECT_EQ(Seconds::zero(), res.modified);
EXPECT_EQ("", res.etag);
// Don't start the request twice in case this callback gets fired multiple times.
if (req2) {
return;
}
updates++;
// Start a second request for the same resource after the first one has been completed.
req2 = fs.request(resource, [&] (Response res2) {
EXPECT_EQ(nullptr, res2.error);
ASSERT_TRUE(res2.data.get());
EXPECT_EQ("Hello World!", *res2.data);
EXPECT_EQ(Seconds::zero(), res2.expires);
EXPECT_EQ(Seconds::zero(), res2.modified);
EXPECT_EQ("", res2.etag);
if (res2.stale) {
EXPECT_EQ(0, stale);
stale++;
} else {
// Now cancel both requests after both have been notified.
req1.reset();
req2.reset();
loop.stop();
HTTPStale.finish();
}
});
});
loop.run();
EXPECT_EQ(1, stale);
EXPECT_EQ(1, updates);
}

1
test/test.gypi
View File

@@ -76,7 +76,6 @@
'storage/asset_file_source.cpp',
'storage/headers.cpp',
'storage/http_cancel.cpp',
'storage/http_coalescing.cpp',
'storage/http_error.cpp',
'storage/http_header_parsing.cpp',
'storage/http_issue_1369.cpp',