Fix copy constructor and assignment for ov::Any (#16757) (#17087)

* Fix copy constructor and assignment for ov::Any (#16757) * Fix copy constructor and assignment for ov::Any * Fix1 * Apply comments * Add test * Fix code style * Fix2 * Fix3 * Fix1
2023-05-18 09:29:05 +02:00 · 2023-05-18 09:29:05 +02:00 · 8f42bf1647
parent 8ba2d74f10
commit 8f42bf1647
6 changed files with 171 additions and 23 deletions
--- a/src/core/include/openvino/core/any.hpp
+++ b/src/core/include/openvino/core/any.hpp
@ -646,14 +646,14 @@ public:
    /// @brief Default constructor
    Any() = default;

-    /// @brief Default copy constructor
+    /// @brief Сopy constructor
    /// @param other other Any object
-    Any(const Any& other) = default;
+    Any(const Any& other);

-    /// @brief Default copy assignment operator
+    /// @brief Сopy assignment operator
    /// @param other other Any object
    /// @return reference to the current object
-    Any& operator=(const Any& other) = default;
+    Any& operator=(const Any& other);

    /// @brief Default move constructor
    /// @param other other Any object
--- a/src/core/include/openvino/core/model.hpp
+++ b/src/core/include/openvino/core/model.hpp
@ -233,7 +233,15 @@ public:
        "This method is deprecated and will be removed soon. Please use evaluate with ov::Tensor instead.")
    bool evaluate(const ov::HostTensorVector& output_tensors,
                  const ov::HostTensorVector& input_tensors,
-                  ov::EvaluationContext evaluation_context = ov::EvaluationContext()) const;
+                  ov::EvaluationContext& evaluation_context) const;
+
+    /// \deprecated Use evaluate with ov::Tensor instead
+    /// \brief Evaluate the model on inputs, putting results in outputs.
+    /// \param output_tensors Tensors for the outputs to compute. One for each result
+    /// \param input_tensors Tensors for the inputs. One for each inputs.
+    OPENVINO_DEPRECATED(
+        "This method is deprecated and will be removed soon. Please use evaluate with ov::Tensor instead.")
+    bool evaluate(const ov::HostTensorVector& output_tensors, const ov::HostTensorVector& input_tensors) const;

    /// \brief Evaluate the model on inputs, putting results in outputs.
    /// \param output_tensors Tensors for the outputs to compute. One for each result
@ -242,7 +250,12 @@ public:
    /// when evaluating the model. This additional information can be shared across nodes.
    bool evaluate(ov::TensorVector& output_tensors,
                  const ov::TensorVector& input_tensors,
-                  ov::EvaluationContext evaluation_context = ov::EvaluationContext()) const;
+                  ov::EvaluationContext& evaluation_context) const;
+
+    /// \brief Evaluate the model on inputs, putting results in outputs.
+    /// \param output_tensors Tensors for the outputs to compute. One for each result
+    /// \param input_tensors Tensors for the inputs. One for each inputs.
+    bool evaluate(ov::TensorVector& output_tensors, const ov::TensorVector& input_tensors) const;

    /// \brief Return a list of model's sinks.
    const ov::SinkVector& get_sinks() const {
@ -454,6 +467,10 @@ private:
                               const std::vector<std::string>::const_iterator& begin,
                               const std::vector<std::string>::const_iterator& end) const;

+    bool has_rt_info(const ov::AnyMap& info,
+                     const std::vector<std::string>::const_iterator& begin,
+                     const std::vector<std::string>::const_iterator& end) const;
+
    // Checks rt attribute
    template <class T,
              typename std::enable_if<std::is_same<std::string, T>::value || std::is_same<T, const char*>::value ||
--- a/src/core/src/any.cpp
+++ b/src/core/src/any.cpp
@ -72,6 +72,19 @@ Any::~Any() {
    _impl = {};
 }

+Any::Any(const Any& other) {
+    *this = other;
+};
+
+Any& Any::operator=(const Any& other) {
+    if (other._temp)
+        _temp = other._temp->copy();
+    if (other._impl)
+        _impl = other._impl->copy();
+    _so = other._so;
+    return *this;
+};
+
 Any::Any(const Any& other, const std::vector<std::shared_ptr<void>>& so) : _impl{other._impl}, _so{so} {}

 Any::Any(const char* str) : Any(std::string{str}) {}
--- a/src/core/src/model.cpp
+++ b/src/core/src/model.cpp
@ -530,17 +530,29 @@ inline void update_output_tensors(const ngraph::HostTensorVector& output_values,

 bool ov::Model::evaluate(const HostTensorVector& output_tensors,
                         const HostTensorVector& input_tensors,
-                         EvaluationContext evaluation_context) const {
+                         EvaluationContext& evaluation_context) const {
    ov::TensorVector outputs = create_tmp_tensors(output_tensors);
    ov::TensorVector inputs = create_tmp_tensors(input_tensors);
-    bool sts = evaluate(outputs, inputs, std::move(evaluation_context));
+    bool sts = evaluate(outputs, inputs, evaluation_context);
    update_output_tensors(output_tensors, outputs);
    return sts;
 }

+bool ov::Model::evaluate(const HostTensorVector& output_tensors, const HostTensorVector& input_tensors) const {
+    ov::EvaluationContext evaluation_context;
+    OPENVINO_SUPPRESS_DEPRECATED_START
+    return evaluate(output_tensors, input_tensors, evaluation_context);
+    OPENVINO_SUPPRESS_DEPRECATED_END
+}
+
+bool ov::Model::evaluate(ov::TensorVector& output_tensors, const ov::TensorVector& input_tensors) const {
+    ov::EvaluationContext evaluation_context;
+    return evaluate(output_tensors, input_tensors, evaluation_context);
+}
+
 bool ov::Model::evaluate(ov::TensorVector& output_tensors,
                         const ov::TensorVector& input_tensors,
-                         ov::EvaluationContext evaluation_context) const {
+                         ov::EvaluationContext& evaluation_context) const {
    evaluation_context.emplace("VariableContext", ov::op::util::VariableContext());
    std::map<RawNodeOutput, ov::Tensor> value_map;
    for (size_t i = 0; i < m_parameters.size(); ++i) {
@ -993,18 +1005,21 @@ std::shared_ptr<ov::Model> ov::Model::clone() const {
 }

 bool ov::Model::has_rt_info(const std::vector<std::string>& args) const {
-    ov::AnyMap info = m_rt_info;
-    for (size_t i = 0; i < args.size(); i++) {
-        bool has_attr = has_rt_arg(info, args[i]);
-        if (!has_attr)
-            return false;
-        if (i == args.size() - 1)
-            break;
-        const ov::Any& rt_attr = get_rt_arg<std::string>(info, args[i]);
-        info = get_map_from_attr(rt_attr);
-    }
-    return true;
+    return has_rt_info(m_rt_info, args.cbegin(), args.cend());
 }
+
+bool ov::Model::has_rt_info(const ov::AnyMap& info,
+                            const std::vector<std::string>::const_iterator& begin,
+                            const std::vector<std::string>::const_iterator& end) const {
+    if (!has_rt_arg(info, *begin))
+        return false;
+    if (begin == end - 1) {
+        return true;
+    } else {
+        return has_rt_info(get_map_from_attr(get_rt_arg<std::string>(info, *begin)), begin + 1, end);
+    }
+}
+
 ov::Any& ov::Model::get_rt_info(ov::AnyMap& info,
                                const std::vector<std::string>::const_iterator& begin,
                                const std::vector<std::string>::const_iterator& end) {
--- a/src/core/tests/any.cpp
+++ b/src/core/tests/any.cpp
@ -162,6 +162,111 @@ TEST_F(AnyTests, AnyAsMapOfAnys) {
    ASSERT_EQ(refMap["testParamString"].as<std::string>(), testString);
 }

+TEST_F(AnyTests, AnyAsMapOfMapOfAnys) {
+    std::map<std::string, Any> refMap1;
+    refMap1["testParamInt"] = 4;
+    refMap1["testParamString"] = "test";
+
+    std::map<std::string, Any> refMap2;
+    refMap2["testParamInt"] = 5;
+    refMap2["testParamString"] = "test2";
+
+    std::map<std::string, Any> refMap;
+    refMap["refMap1"] = refMap1;
+    refMap["refMap2"] = refMap2;
+
+    Any p = refMap;
+    bool isMap = p.is<std::map<std::string, Any>>();
+    ASSERT_TRUE(isMap);
+    auto testMap = p.as<std::map<std::string, Any>>();
+
+    ASSERT_NE(testMap.find("refMap1"), testMap.end());
+    auto testMap1 = testMap.at("refMap1").as<std::map<std::string, Any>>();
+    ASSERT_NE(testMap1.find("testParamInt"), testMap1.end());
+    ASSERT_NE(testMap1.find("testParamString"), testMap1.end());
+
+    int testInt1 = testMap1["testParamInt"].as<int>();
+    std::string testString1 = testMap1["testParamString"].as<std::string>();
+
+    ASSERT_EQ(refMap1["testParamInt"].as<int>(), testInt1);
+    ASSERT_EQ(refMap1["testParamString"].as<std::string>(), testString1);
+
+    ASSERT_NE(testMap.find("refMap2"), testMap.end());
+    auto testMap2 = testMap.at("refMap2").as<std::map<std::string, Any>>();
+    ASSERT_NE(testMap2.find("testParamInt"), testMap2.end());
+    ASSERT_NE(testMap2.find("testParamString"), testMap2.end());
+
+    int testInt2 = testMap2["testParamInt"].as<int>();
+    std::string testString2 = testMap2["testParamString"].as<std::string>();
+
+    ASSERT_EQ(refMap2["testParamInt"].as<int>(), testInt2);
+    ASSERT_EQ(refMap2["testParamString"].as<std::string>(), testString2);
+}
+
+TEST_F(AnyTests, AnyDoesNotShareValues) {
+    // simple types
+    {
+        Any a = 1;
+        Any b = a;
+        a = 2;
+        ASSERT_EQ(1, b.as<int>());
+        ASSERT_EQ(2, a.as<int>());
+        b = 3;
+        ASSERT_EQ(2, a.as<int>());
+        ASSERT_EQ(3, b.as<int>());
+    }
+
+    // AnyMap's
+    {
+        AnyMap map{
+            {"1", ov::Any(1)},
+            {"2", ov::Any(2)},
+        };
+
+        Any a = map;
+
+        // check initial state
+        ASSERT_EQ(1, a.as<AnyMap>()["1"].as<int>());
+        ASSERT_EQ(2, a.as<AnyMap>()["2"].as<int>());
+
+        map["1"] = 3;                                 // change map
+        ASSERT_EQ(1, a.as<AnyMap>()["1"].as<int>());  // Any is not changed
+
+        a.as<AnyMap>()["2"] = 4;           // change Any
+        ASSERT_EQ(2, map["2"].as<int>());  // map is not changed
+
+        // erase from Any's map
+        AnyMap from_any_map = a.as<AnyMap>();
+        from_any_map.erase(from_any_map.begin());
+        ASSERT_EQ(2, map.size());
+
+        // erase from map
+        map.erase(map.find("2"));
+        ASSERT_NE(from_any_map.end(), from_any_map.find("2"));
+        ASSERT_EQ(4, a.as<AnyMap>()["2"].as<int>());
+    }
+}
+
+TEST_F(AnyTests, AnyMapSharesValues) {
+    AnyMap map{
+        {"1", 1},
+        {"2", 2},
+    };
+
+    AnyMap copy_map = map;
+
+    // check initial state
+    ASSERT_EQ(1, copy_map["1"].as<int>());
+    ASSERT_EQ(2, copy_map["2"].as<int>());
+
+    // change map
+    map["1"].as<int>() = 110;
+
+    // check copied state
+    EXPECT_EQ(110, map["1"].as<int>());
+    EXPECT_EQ(1, copy_map["1"].as<int>());
+}
+
 TEST_F(AnyTests, AnyNotEmpty) {
    Any p = 4;
    ASSERT_FALSE(p.empty());
--- a/src/tests_deprecated/unit/CMakeLists.txt
+++ b/src/tests_deprecated/unit/CMakeLists.txt
@ -99,9 +99,7 @@ target_include_directories(${TARGET_NAME} PRIVATE
 set_target_properties(${TARGET_NAME} PROPERTIES COMPILE_PDB_NAME ${TARGET_NAME})

 # because IE unit tests use plugin and IE object files compiled with LTO
-if(CMAKE_COMPILER_IS_GNUCXX AND CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 9.0)
-    set_target_properties(${TARGET_NAME} PROPERTIES INTERPROCEDURAL_OPTIMIZATION_RELEASE ${ENABLE_LTO})
-endif()
+set_target_properties(${TARGET_NAME} PROPERTIES INTERPROCEDURAL_OPTIMIZATION_RELEASE ${ENABLE_LTO})

 ## Mock macros doesn't use "override" specificator
 target_compile_options(${TARGET_NAME} PRIVATE $<$<CXX_COMPILER_ID:Clang>: -Wno-error=inconsistent-missing-override >)