[core] Avoid extra `Layer::Impl` copying during style parsing

Thus we improve style parsing performance.

Before this change, `Layer::Impl` was copied at every `Layer::setProperty()` call, now it is not the case during the style parsing.

`Layer::Impl` instance copying is not needed during style parsing as the instance is not shared at that moment.

16 files changed, 100 insertions, 17 deletions

diff --git a/include/mbgl/style/conversion/layer.hpp b/include/mbgl/style/conversion/layer.hpp
index 9cf019378b..6c9ea3e105 100644
--- a/include/mbgl/style/conversion/layer.hpp
+++ b/include/mbgl/style/conversion/layer.hpp
@@ -16,7 +16,7 @@ public:
     optional<std::unique_ptr<Layer>> operator()(const Convertible& value, Error& error) const;
 };
 
-optional<Error> setPaintProperties(Layer& layer, const Convertible& value);
+optional<Error> setPaintProperties(LayerInitializer&, const Convertible& value);
 
 } // namespace conversion
 } // namespace style
diff --git a/include/mbgl/style/layer.hpp b/include/mbgl/style/layer.hpp
index 1bd3071368..f87a7c7fba 100644
--- a/include/mbgl/style/layer.hpp
+++ b/include/mbgl/style/layer.hpp
@@ -127,7 +127,6 @@ public:
 
     
     void setObserver(LayerObserver*);
-
     // For use in SDK bindings, which store a reference to a platform-native peer
     // object here, so that separately-obtained references to this object share
     // identical platform-native peers.
@@ -147,6 +146,8 @@ protected:
                                                             const conversion::Convertible& value) = 0;
     virtual StyleProperty getPropertyInternal(const std::string&) const = 0;
     LayerObserver* observer;
+    friend class LayerInitializer;
+    bool initializing = false;
     mapbox::base::WeakPtrFactory<Layer> weakFactory {this};
 
 private:
@@ -155,5 +156,33 @@ private:
     optional<conversion::Error> setMaxZoom(const conversion::Convertible& value);
 };
 
+class LayerInitializer {
+public:
+    explicit LayerInitializer(std::unique_ptr<Layer> layer_) : layer(std::move(layer_)) {
+        assert(layer);
+        // The impl must be managed only by |layer|.
+        assert(layer->baseImpl.useCount() == 1u);
+        layer->initializing = true;
+    }
+
+    optional<conversion::Error> setProperty(const std::string& name, const conversion::Convertible& value) {
+        assert(layer);
+        return layer->setProperty(name, value);
+    }
+
+    const LayerTypeInfo* getTypeInfo() const {
+        assert(layer);
+        return layer->getTypeInfo();
+    }
+
+    explicit operator std::unique_ptr<Layer>() && {
+        layer->initializing = false;
+        return std::move(layer);
+    }
+
+private:
+    std::unique_ptr<Layer> layer;
+};
+
 } // namespace style
 } // namespace mbgl
diff --git a/include/mbgl/util/immutable.hpp b/include/mbgl/util/immutable.hpp
index fbd3ac9b34..683af55c57 100644
--- a/include/mbgl/util/immutable.hpp
+++ b/include/mbgl/util/immutable.hpp
@@ -4,6 +4,9 @@
 
 namespace mbgl {
 
+template <class T>
+class Immutable;
+
 /**
  * `Mutable<T>` is a non-nullable uniquely owning reference to a `T`. It can be efficiently converted
  * to `Immutable<T>`.
@@ -42,6 +45,9 @@ private:
     template <class S, class... Args> friend Mutable<S> makeMutable(Args&&...);
     // NOLINTNEXTLINE(readability-redundant-declaration)
     template <class S, class U> friend Mutable<S> staticMutableCast(const Mutable<U>&);
+    // NOLINTNEXTLINE(readability-redundant-declaration)
+    template <class S, class U>
+    friend Mutable<S> constImmutableCast(const Immutable<U>&);
 };
 
 template <class T, class... Args>
@@ -83,6 +89,8 @@ public:
         return *this;
     }
 
+    auto useCount() const { return ptr.use_count(); }
+
     Immutable& operator=(Immutable&&) noexcept = default;
     Immutable& operator=(const Immutable&) = default;
 
@@ -108,6 +116,9 @@ private:
 
     // NOLINTNEXTLINE(readability-redundant-declaration)
     template <class S, class U> friend Immutable<S> staticImmutableCast(const Immutable<U>&);
+
+    template <class S, class U>
+    friend Mutable<S> constImmutableCast(const Immutable<U>&);
 };
 
 template <class S, class U>
@@ -115,6 +126,15 @@ Immutable<S> staticImmutableCast(const Immutable<U>& u) {
     return Immutable<S>(std::static_pointer_cast<const S>(u.ptr));
 }
 
+// Note: this function is breaking the intended Mutable/Immutable design,
+// so its usage shall be limited.
+// The immutable object must be managed only by |u|.
+template <class S, class U>
+Mutable<S> constImmutableCast(const Immutable<U>& u) {
+    assert(u.useCount() == 1u);
+    return Mutable<S>(std::static_pointer_cast<S>(std::const_pointer_cast<U>(u.ptr)));
+}
+
 /**
  * Constrained mutation of an immutable reference. Makes a temporarily-mutable copy of the
  * input Immutable using the inner type's copy constructor, runs the given callable on the
diff --git a/src/mbgl/style/conversion/layer.cpp b/src/mbgl/style/conversion/layer.cpp
index 50401d1679..9690e0941e 100644
--- a/src/mbgl/style/conversion/layer.cpp
+++ b/src/mbgl/style/conversion/layer.cpp
@@ -10,9 +10,9 @@ namespace style {
 namespace conversion {
 
 namespace {
-bool setObjectMember(std::unique_ptr<Layer>& layer, const Convertible& value, const char* member, Error& error) {
+bool setObjectMember(LayerInitializer& layer, const Convertible& value, const char* member, Error& error) {
     if (auto memberValue = objectMember(value, member)) {
-        if (auto error_ = layer->setProperty(member, *memberValue)) {
+        if (auto error_ = layer.setProperty(member, *memberValue)) {
             error = *error_;
             return false;
         }
@@ -21,7 +21,7 @@ bool setObjectMember(std::unique_ptr<Layer>& layer, const Convertible& value, co
 }
 } // namespace
 
-optional<Error> setPaintProperties(Layer& layer, const Convertible& value) {
+optional<Error> setPaintProperties(LayerInitializer& layer, const Convertible& value) {
     auto paintValue = objectMember(value, "paint");
     if (!paintValue) {
         return nullopt;
@@ -62,14 +62,14 @@ optional<std::unique_ptr<Layer>> Converter<std::unique_ptr<Layer>>::operator()(c
         return nullopt;
     }
 
-    std::unique_ptr<Layer> layer = LayerManager::get()->createLayer(*type, *id, value, error);
+    auto layer = LayerManager::get()->createLayer(*type, *id, value, error);
     if (!layer) return nullopt;
-
-    if (!setObjectMember(layer, value, "minzoom", error)) return nullopt;
-    if (!setObjectMember(layer, value, "maxzoom", error)) return nullopt;
-    if (!setObjectMember(layer, value, "filter", error)) return nullopt;
-    if (layer->getTypeInfo()->source == LayerTypeInfo::Source::Required) {
-        if (!setObjectMember(layer, value, "source-layer", error)) return nullopt;
+    LayerInitializer initializer{std::move(layer)};
+    if (!setObjectMember(initializer, value, "minzoom", error)) return nullopt;
+    if (!setObjectMember(initializer, value, "maxzoom", error)) return nullopt;
+    if (!setObjectMember(initializer, value, "filter", error)) return nullopt;
+    if (initializer.getTypeInfo()->source == LayerTypeInfo::Source::Required) {
+        if (!setObjectMember(initializer, value, "source-layer", error)) return nullopt;
     }
 
     auto layoutValue = objectMember(value, "layout");
@@ -79,20 +79,20 @@ optional<std::unique_ptr<Layer>> Converter<std::unique_ptr<Layer>>::operator()(c
             return nullopt;
         }
         optional<Error> error_ = eachMember(
-            *layoutValue, [&](const std::string& k, const Convertible& v) { return layer->setProperty(k, v); });
+            *layoutValue, [&](const std::string& k, const Convertible& v) { return initializer.setProperty(k, v); });
         if (error_) {
             error = *error_;
             return nullopt;
         }
     }
 
-    optional<Error> error_ = setPaintProperties(*layer, value);
+    optional<Error> error_ = setPaintProperties(initializer, value);
     if (error_) {
         error = *error_;
         return nullopt;
     }
 
-    return layer;
+    return std::unique_ptr<Layer>(std::move(initializer));
 }
 
 } // namespace conversion
diff --git a/src/mbgl/style/layers/background_layer.cpp b/src/mbgl/style/layers/background_layer.cpp
index 336cd33599..10ef64c51a 100644
--- a/src/mbgl/style/layers/background_layer.cpp
+++ b/src/mbgl/style/layers/background_layer.cpp
@@ -47,6 +47,9 @@ const BackgroundLayer::Impl& BackgroundLayer::impl() const {
 }
 
 Mutable<BackgroundLayer::Impl> BackgroundLayer::mutableImpl() const {
+    if (initializing) {
+        return constImmutableCast<Impl, Layer::Impl>(baseImpl);
+    }
     return makeMutable<Impl>(impl());
 }
 
diff --git a/src/mbgl/style/layers/circle_layer.cpp b/src/mbgl/style/layers/circle_layer.cpp
index 8c81d8a23f..488dcb24b1 100644
--- a/src/mbgl/style/layers/circle_layer.cpp
+++ b/src/mbgl/style/layers/circle_layer.cpp
@@ -47,6 +47,9 @@ const CircleLayer::Impl& CircleLayer::impl() const {
 }
 
 Mutable<CircleLayer::Impl> CircleLayer::mutableImpl() const {
+    if (initializing) {
+        return constImmutableCast<Impl, Layer::Impl>(baseImpl);
+    }
     return makeMutable<Impl>(impl());
 }
 
diff --git a/src/mbgl/style/layers/fill_extrusion_layer.cpp b/src/mbgl/style/layers/fill_extrusion_layer.cpp
index 50afe80ea3..4be4c14087 100644
--- a/src/mbgl/style/layers/fill_extrusion_layer.cpp
+++ b/src/mbgl/style/layers/fill_extrusion_layer.cpp
@@ -47,6 +47,9 @@ const FillExtrusionLayer::Impl& FillExtrusionLayer::impl() const {
 }
 
 Mutable<FillExtrusionLayer::Impl> FillExtrusionLayer::mutableImpl() const {
+    if (initializing) {
+        return constImmutableCast<Impl, Layer::Impl>(baseImpl);
+    }
     return makeMutable<Impl>(impl());
 }
 
diff --git a/src/mbgl/style/layers/fill_layer.cpp b/src/mbgl/style/layers/fill_layer.cpp
index 83049a33f0..aa7801e2b5 100644
--- a/src/mbgl/style/layers/fill_layer.cpp
+++ b/src/mbgl/style/layers/fill_layer.cpp
@@ -47,6 +47,9 @@ const FillLayer::Impl& FillLayer::impl() const {
 }
 
 Mutable<FillLayer::Impl> FillLayer::mutableImpl() const {
+    if (initializing) {
+        return constImmutableCast<Impl, Layer::Impl>(baseImpl);
+    }
     return makeMutable<Impl>(impl());
 }
 
diff --git a/src/mbgl/style/layers/heatmap_layer.cpp b/src/mbgl/style/layers/heatmap_layer.cpp
index 4f86519b31..8afba25171 100644
--- a/src/mbgl/style/layers/heatmap_layer.cpp
+++ b/src/mbgl/style/layers/heatmap_layer.cpp
@@ -47,6 +47,9 @@ const HeatmapLayer::Impl& HeatmapLayer::impl() const {
 }
 
 Mutable<HeatmapLayer::Impl> HeatmapLayer::mutableImpl() const {
+    if (initializing) {
+        return constImmutableCast<Impl, Layer::Impl>(baseImpl);
+    }
     return makeMutable<Impl>(impl());
 }
 
diff --git a/src/mbgl/style/layers/hillshade_layer.cpp b/src/mbgl/style/layers/hillshade_layer.cpp
index b5fef6e628..b6ae821d26 100644
--- a/src/mbgl/style/layers/hillshade_layer.cpp
+++ b/src/mbgl/style/layers/hillshade_layer.cpp
@@ -47,6 +47,9 @@ const HillshadeLayer::Impl& HillshadeLayer::impl() const {
 }
 
 Mutable<HillshadeLayer::Impl> HillshadeLayer::mutableImpl() const {
+    if (initializing) {
+        return constImmutableCast<Impl, Layer::Impl>(baseImpl);
+    }
     return makeMutable<Impl>(impl());
 }
 
diff --git a/src/mbgl/style/layers/layer.cpp.ejs b/src/mbgl/style/layers/layer.cpp.ejs
index 903d713755..b707bc198e 100644
--- a/src/mbgl/style/layers/layer.cpp.ejs
+++ b/src/mbgl/style/layers/layer.cpp.ejs
@@ -124,6 +124,9 @@ const <%- camelize(type) %>Layer::Impl& <%- camelize(type) %>Layer::impl() const
 }
 
 Mutable<<%- camelize(type) %>Layer::Impl> <%- camelize(type) %>Layer::mutableImpl() const {
+    if (initializing) {
+        return constImmutableCast<Impl, Layer::Impl>(baseImpl);
+    }
     return makeMutable<Impl>(impl());
 }
 
diff --git a/src/mbgl/style/layers/line_layer.cpp b/src/mbgl/style/layers/line_layer.cpp
index f5b5df9af7..1522f8ff2b 100644
--- a/src/mbgl/style/layers/line_layer.cpp
+++ b/src/mbgl/style/layers/line_layer.cpp
@@ -47,6 +47,9 @@ const LineLayer::Impl& LineLayer::impl() const {
 }
 
 Mutable<LineLayer::Impl> LineLayer::mutableImpl() const {
+    if (initializing) {
+        return constImmutableCast<Impl, Layer::Impl>(baseImpl);
+    }
     return makeMutable<Impl>(impl());
 }
 
diff --git a/src/mbgl/style/layers/location_indicator_layer.cpp b/src/mbgl/style/layers/location_indicator_layer.cpp
index b02e044c96..fe28399384 100644
--- a/src/mbgl/style/layers/location_indicator_layer.cpp
+++ b/src/mbgl/style/layers/location_indicator_layer.cpp
@@ -47,6 +47,9 @@ const LocationIndicatorLayer::Impl& LocationIndicatorLayer::impl() const {
 }
 
 Mutable<LocationIndicatorLayer::Impl> LocationIndicatorLayer::mutableImpl() const {
+    if (initializing) {
+        return constImmutableCast<Impl, Layer::Impl>(baseImpl);
+    }
     return makeMutable<Impl>(impl());
 }
 
diff --git a/src/mbgl/style/layers/raster_layer.cpp b/src/mbgl/style/layers/raster_layer.cpp
index 6db4feb65e..63240e816c 100644
--- a/src/mbgl/style/layers/raster_layer.cpp
+++ b/src/mbgl/style/layers/raster_layer.cpp
@@ -47,6 +47,9 @@ const RasterLayer::Impl& RasterLayer::impl() const {
 }
 
 Mutable<RasterLayer::Impl> RasterLayer::mutableImpl() const {
+    if (initializing) {
+        return constImmutableCast<Impl, Layer::Impl>(baseImpl);
+    }
     return makeMutable<Impl>(impl());
 }
 
diff --git a/src/mbgl/style/layers/symbol_layer.cpp b/src/mbgl/style/layers/symbol_layer.cpp
index b9f0b70209..b768827b9e 100644
--- a/src/mbgl/style/layers/symbol_layer.cpp
+++ b/src/mbgl/style/layers/symbol_layer.cpp
@@ -47,6 +47,9 @@ const SymbolLayer::Impl& SymbolLayer::impl() const {
 }
 
 Mutable<SymbolLayer::Impl> SymbolLayer::mutableImpl() const {
+    if (initializing) {
+        return constImmutableCast<Impl, Layer::Impl>(baseImpl);
+    }
     return makeMutable<Impl>(impl());
 }
 
diff --git a/src/mbgl/style/parser.cpp b/src/mbgl/style/parser.cpp
index 9d9b80276a..b8ad4088d3 100644
--- a/src/mbgl/style/parser.cpp
+++ b/src/mbgl/style/parser.cpp
@@ -253,8 +253,9 @@ void Parser::parseLayer(const std::string& id, const JSValue& value, std::unique
             return;
         }
 
-        layer = reference->cloneRef(id);
-        conversion::setPaintProperties(*layer, conversion::Convertible(&value));
+        LayerInitializer layerInitializer{reference->cloneRef(id)};
+        conversion::setPaintProperties(layerInitializer, conversion::Convertible(&value));
+        layer = std::unique_ptr<Layer>(std::move(layerInitializer));
     } else {
         conversion::Error error;
         optional<std::unique_ptr<Layer>> converted = conversion::convert<std::unique_ptr<Layer>>(value, error);