Skip to content
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

Eliminate copies in deferred cleanup #3035

Draft
wants to merge 14 commits into
base: main
Choose a base branch
from
Draft
3 changes: 2 additions & 1 deletion .clang-tidy
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ Checks: [
-performance-enum-size,
-misc-include-cleaner,
-readability-redundant-inline-specifier,
-readability-avoid-nested-conditional-operator
-readability-avoid-nested-conditional-operator,
-cppcoreguidelines-pro-type-static-cast-downcast # no RTTI
]
WarningsAsErrors: '*'
HeaderFilterRegex: '.*'
Expand Down
12 changes: 12 additions & 0 deletions .github/workflows/android-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,18 @@ jobs:
working-directory: test/android

steps:
- name: Free Disk Space (Ubuntu)
if: startsWith(runner.name, 'GitHub Actions')
uses: jlumbroso/free-disk-space@main
with:
tool-cache: false
android: false
dotnet: true
haskell: true
large-packages: true
docker-images: true
swap-storage: false

- uses: actions/checkout@v4
with:
submodules: recursive
Expand Down
4 changes: 4 additions & 0 deletions .gitmodules
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,7 @@
[submodule "vendor/glslang"]
path = vendor/glslang
url = https://github.com/KhronosGroup/glslang.git
[submodule "vendor/nontype_functional"]
path = vendor/nontype_functional
url = https://github.com/zhihaoy/nontype_functional.git
branch = compat
1 change: 1 addition & 0 deletions BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ cc_library(
"//vendor:earcut.hpp",
"//vendor:eternal",
"//vendor:mapbox-base",
"//vendor:nontype_functional",
"//vendor:parsedate",
"//vendor:polylabel",
"//vendor:protozero",
Expand Down
9 changes: 6 additions & 3 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1449,6 +1449,8 @@ include(${PROJECT_SOURCE_DIR}/vendor/csscolorparser.cmake)
include(${PROJECT_SOURCE_DIR}/vendor/earcut.hpp.cmake)
include(${PROJECT_SOURCE_DIR}/vendor/eternal.cmake)
include(${PROJECT_SOURCE_DIR}/vendor/mapbox-base.cmake)
include(${PROJECT_SOURCE_DIR}/vendor/metal-cpp.cmake)
include(${PROJECT_SOURCE_DIR}/vendor/nontype_functional.cmake)
include(${PROJECT_SOURCE_DIR}/vendor/parsedate.cmake)
include(${PROJECT_SOURCE_DIR}/vendor/polylabel.cmake)
include(${PROJECT_SOURCE_DIR}/vendor/protozero.cmake)
Expand All @@ -1457,7 +1459,6 @@ include(${PROJECT_SOURCE_DIR}/vendor/unique_resource.cmake)
include(${PROJECT_SOURCE_DIR}/vendor/unordered_dense.cmake)
include(${PROJECT_SOURCE_DIR}/vendor/vector-tile.cmake)
include(${PROJECT_SOURCE_DIR}/vendor/wagyu.cmake)
include(${PROJECT_SOURCE_DIR}/vendor/metal-cpp.cmake)

target_link_libraries(
mbgl-core
Expand All @@ -1472,20 +1473,21 @@ target_link_libraries(
mbgl-vendor-csscolorparser
mbgl-vendor-earcut.hpp
mbgl-vendor-eternal
$<$<BOOL:${MLN_WITH_METAL}>:mbgl-vendor-metal-cpp>
mbgl-vendor-parsedate
mbgl-vendor-polylabel
mbgl-vendor-protozero
mbgl-vendor-unique_resource
mbgl-vendor-vector-tile
mbgl-vendor-wagyu
$<$<BOOL:${MLN_WITH_METAL}>:mbgl-vendor-metal-cpp>
PUBLIC
Mapbox::Base
Mapbox::Base::Extras::expected-lite
Mapbox::Base::Extras::rapidjson
Mapbox::Base::geojson.hpp
Mapbox::Base::geometry.hpp
Mapbox::Base::variant
mbgl-vendor-nontype_functional
$<$<BOOL:${MLN_USE_TRACY}>:TracyClient>
unordered_dense
)
Expand All @@ -1511,13 +1513,14 @@ export(TARGETS
mbgl-vendor-csscolorparser
mbgl-vendor-earcut.hpp
mbgl-vendor-eternal
mbgl-vendor-metal-cpp
mbgl-vendor-nontype_functional
mbgl-vendor-parsedate
mbgl-vendor-polylabel
mbgl-vendor-protozero
mbgl-vendor-unique_resource
mbgl-vendor-vector-tile
mbgl-vendor-wagyu
mbgl-vendor-metal-cpp
unordered_dense

FILE MapboxCoreTargets.cmake
Expand Down
22 changes: 14 additions & 8 deletions include/mbgl/actor/scheduler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#include <mapbox/std/weak.hpp>

#include <std23/move_only_function.h>

#include <functional>
#include <memory>
#include <type_traits>
Expand Down Expand Up @@ -36,16 +38,18 @@ class Mailbox;
*/
class Scheduler {
public:
using Task = std23::move_only_function<void()>;

virtual ~Scheduler() = default;

/// Enqueues a function for execution.
virtual void schedule(std::function<void()>&&) = 0;
virtual void schedule(const util::SimpleIdentity, std::function<void()>&&) = 0;
virtual void schedule(Task&&) = 0;
virtual void schedule(const util::SimpleIdentity, Task&&) = 0;

/// Makes a weak pointer to this Scheduler.
virtual mapbox::base::WeakPtr<Scheduler> makeWeakPtr() = 0;
/// Enqueues a function for execution on the render thread owned by the given tag.
virtual void runOnRenderThread(const util::SimpleIdentity, std::function<void()>&&) {}
virtual void runOnRenderThread(const util::SimpleIdentity, Task&&) {}
/// Run render thread jobs for the given tag
/// @param tag Tag of owner
/// @param closeQueue Runs all render jobs and then removes the internal queue.
Expand All @@ -59,7 +63,7 @@ class Scheduler {
///
/// If this scheduler is already deleted by the time the returnded closure
/// is first invoked, the call is ignored.
std::function<void()> bindOnce(std::function<void()>);
Scheduler::Task bindOnce(Task);

/// Enqueues the given |task| for execution into this scheduler's task queue
/// and then enqueues the |reply| with the captured task result to the
Expand Down Expand Up @@ -103,10 +107,12 @@ class Scheduler {
[[nodiscard]] static std::shared_ptr<Scheduler> GetSequenced();

/// Set a function to be called when an exception occurs on a thread controlled by the scheduler
void setExceptionHandler(std::function<void(const std::exception_ptr)> handler_) { handler = std::move(handler_); }
void setExceptionHandler(std23::move_only_function<void(const std::exception_ptr)> handler_) {
handler = std::move(handler_);
}

protected:
std::function<void(const std::exception_ptr)> handler;
std23::move_only_function<void(const std::exception_ptr)> handler;

private:
template <typename TaskFn, typename ReplyFn>
Expand Down Expand Up @@ -136,8 +142,8 @@ class TaggedScheduler {
/// @brief Get the wrapped scheduler
const std::shared_ptr<Scheduler>& get() const noexcept { return scheduler; }

void schedule(std::function<void()>&& fn) { scheduler->schedule(tag, std::move(fn)); }
void runOnRenderThread(std::function<void()>&& fn) { scheduler->runOnRenderThread(tag, std::move(fn)); }
void schedule(Scheduler::Task&& fn) { scheduler->schedule(tag, std::move(fn)); }
void runOnRenderThread(Scheduler::Task&& fn) { scheduler->runOnRenderThread(tag, std::move(fn)); }
void runRenderJobs(bool closeQueue = false) { scheduler->runRenderJobs(tag, closeQueue); }
void waitForEmpty() const noexcept { scheduler->waitForEmpty(tag); }

Expand Down
13 changes: 6 additions & 7 deletions include/mbgl/renderer/renderer_observer.hpp
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
#pragma once

#include <mbgl/tile/tile_id.hpp>
#include <mbgl/util/font_stack.hpp>
#include <mbgl/text/glyph_range.hpp>
#include <mbgl/tile/tile_operation.hpp>
#include <mbgl/actor/scheduler.hpp>
#include <mbgl/gfx/backend.hpp>
#include <mbgl/shaders/shader_source.hpp>
#include <mbgl/text/glyph_range.hpp>
#include <mbgl/tile/tile_id.hpp>
#include <mbgl/tile/tile_operation.hpp>
#include <mbgl/util/font_stack.hpp>

#include <cstdint>
#include <exception>
#include <functional>
#include <string>

namespace mbgl {
Expand Down Expand Up @@ -55,8 +55,7 @@ class RendererObserver {
virtual void onDidFinishRenderingMap() {}

/// Style is missing an image
using StyleImageMissingCallback = std::function<void()>;
virtual void onStyleImageMissing(const std::string&, const StyleImageMissingCallback& done) { done(); }
virtual void onStyleImageMissing(const std::string&, Scheduler::Task&& done) { done(); }
virtual void onRemoveUnusedStyleImages(const std::vector<std::string>&) {}

// Entry point for custom shader registration
Expand Down
34 changes: 17 additions & 17 deletions include/mbgl/storage/database_file_source.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ class DatabaseFileSource : public FileSource {
~DatabaseFileSource() override;

/// FileSource overrides
std::unique_ptr<AsyncRequest> request(const Resource&, Callback) override;
void forward(const Resource&, const Response&, std::function<void()> callback) override;
std::unique_ptr<AsyncRequest> request(const Resource&, CopyableCallback<void(Response)>) override;
void forward(const Resource&, const Response&, Scheduler::Task&& callback) override;
bool canRequest(const Resource&) const override;
void setProperty(const std::string&, const mapbox::base::Value&) override;
void pause() override;
Expand All @@ -29,7 +29,7 @@ class DatabaseFileSource : public FileSource {
* Sets path of a database to be used by DatabaseFileSource and invokes
* provided callback when a database path is set.
*/
virtual void setDatabasePath(const std::string&, std::function<void()> callback);
virtual void setDatabasePath(const std::string&, Callback<>&& callback);

/**
* Delete existing database and re-initialize.
Expand All @@ -38,7 +38,7 @@ class DatabaseFileSource : public FileSource {
* will be executed on the database thread; it is the responsibility of the
* SDK bindings to re-execute a user-provided callback on the main thread.
*/
virtual void resetDatabase(std::function<void(std::exception_ptr)>);
virtual void resetDatabase(ExceptionCallback&&);

/**
* Packs the existing database file into a minimal amount of disk space.
Expand All @@ -50,7 +50,7 @@ class DatabaseFileSource : public FileSource {
* will be executed on the database thread; it is the responsibility of the
* SDK bindings to re-execute a user-provided callback on the main thread.
*/
virtual void packDatabase(std::function<void(std::exception_ptr)> callback);
virtual void packDatabase(ExceptionCallback&& callback);

/**
* Sets whether packing the database file occurs automatically after an
Expand Down Expand Up @@ -87,7 +87,7 @@ class DatabaseFileSource : public FileSource {
* Resources overlapping with offline regions will not be affected
* by this call.
*/
virtual void invalidateAmbientCache(std::function<void(std::exception_ptr)>);
virtual void invalidateAmbientCache(ExceptionCallback&&);

/**
* Erase resources from the ambient cache, freeing storage space.
Expand All @@ -100,7 +100,7 @@ class DatabaseFileSource : public FileSource {
* Resources overlapping with offline regions will not be affected
* by this call.
*/
virtual void clearAmbientCache(std::function<void(std::exception_ptr)>);
virtual void clearAmbientCache(ExceptionCallback&&);

/**
* Sets the maximum size in bytes for the ambient cache.
Expand All @@ -122,7 +122,7 @@ class DatabaseFileSource : public FileSource {
* This method should always be called before using the database,
* otherwise the default maximum size will be used.
*/
virtual void setMaximumAmbientCacheSize(uint64_t size, std::function<void(std::exception_ptr)> callback);
virtual void setMaximumAmbientCacheSize(uint64_t size, ExceptionCallback&& callback);

// Offline

Expand All @@ -134,7 +134,7 @@ class DatabaseFileSource : public FileSource {
* responsibility of the SDK bindings to re-execute a user-provided callback
* on the main thread.
*/
virtual void listOfflineRegions(std::function<void(expected<OfflineRegions, std::exception_ptr>)>);
virtual void listOfflineRegions(Callback<void(expected<OfflineRegions, std::exception_ptr>)>&&);

/**
* Retrieve given region in the offline database.
Expand All @@ -145,7 +145,7 @@ class DatabaseFileSource : public FileSource {
* on the main thread.
*/
virtual void getOfflineRegion(int64_t regionID,
std::function<void(expected<std::optional<OfflineRegion>, std::exception_ptr>)>);
Callback<void(expected<std::optional<OfflineRegion>, std::exception_ptr>)>&&);

/**
* Create an offline region in the database.
Expand All @@ -161,18 +161,18 @@ class DatabaseFileSource : public FileSource {
*/
virtual void createOfflineRegion(const OfflineRegionDefinition& definition,
const OfflineRegionMetadata& metadata,
std::function<void(expected<OfflineRegion, std::exception_ptr>)>);
Callback<void(expected<OfflineRegion, std::exception_ptr>)>&&);
/**
* Update an offline region metadata in the database.
*/
virtual void updateOfflineMetadata(int64_t regionID,
const OfflineRegionMetadata& metadata,
std::function<void(expected<OfflineRegionMetadata, std::exception_ptr>)>);
Callback<void(expected<OfflineRegionMetadata, std::exception_ptr>)>&&);

/**
* Register an observer to be notified when the state of the region changes.
*/
virtual void setOfflineRegionObserver(const OfflineRegion&, std::unique_ptr<OfflineRegionObserver>);
virtual void setOfflineRegionObserver(const OfflineRegion&, std::unique_ptr<OfflineRegionObserver>&&);

/**
* Pause or resume downloading of regional resources.
Expand All @@ -186,7 +186,7 @@ class DatabaseFileSource : public FileSource {
* bindings to re-execute a user-provided callback on the main thread.
*/
virtual void getOfflineRegionStatus(const OfflineRegion&,
std::function<void(expected<OfflineRegionStatus, std::exception_ptr>)>) const;
Callback<void(expected<OfflineRegionStatus, std::exception_ptr>)>&&) const;

/**
* Merge offline regions from a secondary database into the main offline database.
Expand All @@ -209,7 +209,7 @@ class DatabaseFileSource : public FileSource {
* does not contain all the tiles or resources required by the region definition.
*/
virtual void mergeOfflineRegions(const std::string& sideDatabasePath,
std::function<void(expected<OfflineRegions, std::exception_ptr>)>);
Callback<void(expected<OfflineRegions, std::exception_ptr>)>&&);

/**
* Remove an offline region from the database and perform any resources
Expand All @@ -231,15 +231,15 @@ class DatabaseFileSource : public FileSource {
* will be executed on the database thread; it is the responsibility of the
* SDK bindings to re-execute a user-provided callback on the main thread.
*/
virtual void deleteOfflineRegion(const OfflineRegion&, std::function<void(std::exception_ptr)>);
virtual void deleteOfflineRegion(const OfflineRegion&, ExceptionCallback&&);

/**
* Invalidate all the tiles from an offline region forcing Mapbox GL to
* revalidate the tiles with the server before using. This is more efficient
* than deleting the offline region and downloading it again because if the
* data on the cache matches the server, no new data gets transmitted.
*/
virtual void invalidateOfflineRegion(const OfflineRegion&, std::function<void(std::exception_ptr)>);
virtual void invalidateOfflineRegion(const OfflineRegion&, ExceptionCallback&&);

/**
* Changing or bypassing this limit without permission from Mapbox is
Expand Down
13 changes: 9 additions & 4 deletions include/mbgl/storage/file_source.hpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#pragma once

#include <mbgl/actor/scheduler.hpp>
#include <mbgl/storage/resource_transform.hpp>
#include <mbgl/storage/response.hpp>
#include <mbgl/storage/resource_options.hpp>
Expand Down Expand Up @@ -34,24 +35,28 @@ enum FileSourceType : uint8_t {
// GeoJSONSource, RasterSource, VectorSource, CustomGeometrySource and other *Sources.
class FileSource {
public:
template <typename T = void()>
using Callback = std23::move_only_function<T>;
template <typename T = void()>
using CopyableCallback = std::function<T>;
using ExceptionCallback = Callback<void(std::exception_ptr)>;

FileSource& operator=(const FileSource&) = delete;
virtual ~FileSource() = default;

using Callback = std::function<void(Response)>;

/// Request a resource. The callback will be called asynchronously, in the
/// same thread as the request was made. This thread must have an active
/// RunLoop. The request may be cancelled before completion by releasing the
/// returned AsyncRequest. If the request is cancelled before the callback
/// is executed, the callback will not be executed.
virtual std::unique_ptr<AsyncRequest> request(const Resource&, Callback) = 0;
virtual std::unique_ptr<AsyncRequest> request(const Resource&, CopyableCallback<void(Response)>) = 0;

/// Allows to forward response from one source to another.
/// Optionally, callback can be provided to receive notification for forward
/// operation.
//
// NOLINTNEXTLINE(performance-unnecessary-value-param)
virtual void forward(const Resource&, const Response&, std::function<void()>) {}
virtual void forward(const Resource&, const Response&, Scheduler::Task&&) {}

/// When a file source supports consulting a local cache only, it must
/// return true. Cache-only requests are requests that aren't as urgent, but
Expand Down
2 changes: 1 addition & 1 deletion include/mbgl/storage/online_file_source.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class OnlineFileSource : public FileSource {

private:
// FileSource overrides
std::unique_ptr<AsyncRequest> request(const Resource&, Callback) override;
std::unique_ptr<AsyncRequest> request(const Resource&, CopyableCallback<void(Response)>) override;
bool canRequest(const Resource&) const override;
void pause() override;
void resume() override;
Expand Down
Loading
Loading