Eliminate copies in deferred cleanup (#3035)

.clang-tidy

@@ -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: '.*'

.gitmodules

@@ -58,3 +58,7 @@
 [submodule "vendor/PMTiles"]
 	path = vendor/PMTiles
 	url = https://github.com/protomaps/PMTiles.git
+[submodule "vendor/nontype_functional"]
+	path = vendor/nontype_functional
+	url = https://github.com/zhihaoy/nontype_functional.git
+	branch = compat

BUILD.bazel

@@ -127,6 +127,7 @@ cc_library(
         "//vendor:earcut.hpp",
         "//vendor:eternal",
         "//vendor:mapbox-base",
+        "//vendor:nontype_functional",
         "//vendor:parsedate",
         "//vendor:pmtiles",
         "//vendor:polylabel",

CMakeLists.txt

@@ -1448,6 +1448,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/pmtiles.cmake)
 include(${PROJECT_SOURCE_DIR}/vendor/polylabel.cmake)
@@ -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)
 
 if(MLN_USE_RUST)
 include(${PROJECT_SOURCE_DIR}/rustutils/rustutils.cmake)
@@ -1475,6 +1476,7 @@ target_link_libraries(
         mbgl-vendor-boost
         mbgl-vendor-earcut.hpp
         mbgl-vendor-eternal
+        $<$<BOOL:${MLN_WITH_METAL}>:mbgl-vendor-metal-cpp>
         mbgl-vendor-parsedate
         mbgl-vendor-pmtiles
         mbgl-vendor-polylabel
@@ -1491,6 +1493,7 @@ target_link_libraries(
         Mapbox::Base::geojson.hpp
         Mapbox::Base::geometry.hpp
         Mapbox::Base::variant
+        mbgl-vendor-nontype_functional
         $<$<BOOL:${MLN_USE_TRACY}>:TracyClient>
         unordered_dense
 )
@@ -1514,14 +1517,15 @@ set(EXPORT_TARGETS
     mbgl-vendor-boost
     mbgl-vendor-earcut.hpp
     mbgl-vendor-eternal
+    mbgl-vendor-metal-cpp
+    mbgl-vendor-nontype_functional
     mbgl-vendor-parsedate
     mbgl-vendor-pmtiles
     mbgl-vendor-polylabel
     mbgl-vendor-protozero
     mbgl-vendor-unique_resource
     mbgl-vendor-vector-tile
     mbgl-vendor-wagyu
-    mbgl-vendor-metal-cpp
     unordered_dense
 )
 

include/mbgl/actor/scheduler.hpp

@@ -4,6 +4,8 @@
 
 #include <mapbox/std/weak.hpp>
 
+#include <std23/move_only_function.h>
+
 #include <functional>
 #include <memory>
 #include <type_traits>
@@ -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.
@@ -57,9 +61,9 @@ class Scheduler {
     /// the given closure to this scheduler, the consequent calls of the
     /// returned closure are ignored.
     ///
-    /// If this scheduler is already deleted by the time the returnded closure
+    /// If this scheduler is already deleted by the time the returned 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
@@ -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>
@@ -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); }
 

include/mbgl/mtl/uniform_buffer.hpp

@@ -43,7 +43,7 @@ class UniformBufferArray final : public gfx::UniformBufferArray {
     }
 
     void bind(RenderPass& renderPass) const noexcept;
-    void unbind(RenderPass& renderPass) const noexcept {};
+    void unbind(RenderPass&) const noexcept {}
 
 private:
     gfx::UniqueUniformBuffer copy(const gfx::UniformBuffer& buffer) override {

include/mbgl/renderer/renderer_observer.hpp

@@ -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 {
@@ -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

include/mbgl/storage/database_file_source.hpp

@@ -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&, std::function<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;
@@ -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&, std23::move_only_function<void()>&& callback);
 
     /**
      * Delete existing database and re-initialize.
@@ -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(std23::move_only_function<void(std::exception_ptr)>&&);
 
     /**
      * Packs the existing database file into a minimal amount of disk space.
@@ -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(std23::move_only_function<void(std::exception_ptr)>&& callback);
 
     /**
      * Sets whether packing the database file occurs automatically after an
@@ -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(std23::move_only_function<void(std::exception_ptr)>&&);
 
     /**
      * Erase resources from the ambient cache, freeing storage space.
@@ -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(std23::move_only_function<void(std::exception_ptr)>&&);
 
     /**
      * Sets the maximum size in bytes for the ambient cache.
@@ -122,7 +122,8 @@ 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,
+                                            std23::move_only_function<void(std::exception_ptr)>&& callback);
 
     // Offline
 
@@ -134,7 +135,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(std23::move_only_function<void(expected<OfflineRegions, std::exception_ptr>)>&&);
 
     /**
      * Retrieve given region in the offline database.
@@ -144,8 +145,9 @@ class DatabaseFileSource : public FileSource {
      * responsibility of the SDK bindings to re-execute a user-provided callback
      * on the main thread.
      */
-    virtual void getOfflineRegion(int64_t regionID,
-                                  std::function<void(expected<std::optional<OfflineRegion>, std::exception_ptr>)>);
+    virtual void getOfflineRegion(
+        int64_t regionID,
+        std23::move_only_function<void(expected<std::optional<OfflineRegion>, std::exception_ptr>)>&&);
 
     /**
      * Create an offline region in the database.
@@ -161,18 +163,19 @@ class DatabaseFileSource : public FileSource {
      */
     virtual void createOfflineRegion(const OfflineRegionDefinition& definition,
                                      const OfflineRegionMetadata& metadata,
-                                     std::function<void(expected<OfflineRegion, std::exception_ptr>)>);
+                                     std23::move_only_function<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>)>);
+    virtual void updateOfflineMetadata(
+        int64_t regionID,
+        const OfflineRegionMetadata& metadata,
+        std23::move_only_function<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.
@@ -185,8 +188,9 @@ class DatabaseFileSource : public FileSource {
      * 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 getOfflineRegionStatus(const OfflineRegion&,
-                                        std::function<void(expected<OfflineRegionStatus, std::exception_ptr>)>) const;
+    virtual void getOfflineRegionStatus(
+        const OfflineRegion&,
+        std23::move_only_function<void(expected<OfflineRegionStatus, std::exception_ptr>)>&&) const;
 
     /**
      * Merge offline regions from a secondary database into the main offline database.
@@ -209,7 +213,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>)>);
+                                     std23::move_only_function<void(expected<OfflineRegions, std::exception_ptr>)>&&);
 
     /**
      * Remove an offline region from the database and perform any resources
@@ -231,15 +235,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&, std23::move_only_function<void(std::exception_ptr)>&&);
 
     /**
      * 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&, std23::move_only_function<void(std::exception_ptr)>&&);
 
     /**
      * Changing or bypassing this limit without permission from Mapbox is

include/mbgl/storage/file_source.hpp

@@ -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>
@@ -38,21 +39,19 @@ class FileSource {
     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&, std::function<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

include/mbgl/storage/online_file_source.hpp

@@ -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&, std::function<void(Response)>) override;
     bool canRequest(const Resource&) const override;
     void pause() override;
     void resume() override;