[GPU] Graph serialization for GPU #2 by e-ddykim · Pull Request #13986 · openvinotoolkit/openvino

e-ddykim · 2022-11-14T12:52:30Z

Details:

Follow up PR of [GPU] Graph serialization for GPU #13801

Tickets:

57672
95408

src/plugins/intel_gpu/src/runtime/CMakeLists.txt

e-ddykim · 2022-11-14T14:23:47Z

src/plugins/intel_gpu/include/intel_gpu/graph/network.hpp


 #include "intel_gpu/graph/topology.hpp"
 #include "intel_gpu/graph/program.hpp"
+#include "intel_gpu/graph/serialization/binary_buffer.hpp"


#13801 (comment)
Moved the include path. Thank you.

e-ddykim · 2022-11-14T14:49:14Z

src/plugins/intel_gpu/include/intel_gpu/graph/serialization/binary_buffer.hpp

-            throw std::runtime_error("Failed to write " + std::to_string(size) + " bytes to stream! Wrote " + std::to_string(written_size));
-        }
+        OPENVINO_ASSERT(written_size == size,
+            "Failed to write " + std::to_string(size) + " bytes to stream! Wrote " + std::to_string(written_size));


#13801 (comment)
Changed to use OPENVINO_ASSERT. Thank you.

e-ddykim · 2022-11-14T14:53:14Z

src/plugins/intel_gpu/src/graph/program.cpp


-    if (_engine.configuration().kernels_cache_path.empty())
-        _kernels_cache->reset();
+    _kernels_cache->reset();


#13801 (comment)
Reverted. Thank you.

e-ddykim · 2022-11-14T15:15:38Z

src/plugins/intel_gpu/thirdparty/CMakeLists.txt

        set(ONEDNN_INSTALL_DIR "${CMAKE_CURRENT_BINARY_DIR}/onednn_gpu_install/")
        set(ONEDNN_PREFIX_DIR "${CMAKE_CURRENT_BINARY_DIR}/onednn_gpu_root")
-        execute_process(COMMAND git apply --verbose ../onednn_gpu.patch
+        execute_process(COMMAND git apply ../onednn_gpu.patch OUTPUT_QUIET ERROR_QUIET


#13801 (comment)
That error message occurs when trying to patch code that has already been patched again. I updated not to be displayed to prevent confusion.

Even first cmake run contains this error

e-ddykim · 2022-11-17T03:18:40Z

src/plugins/intel_gpu/include/intel_gpu/plugin/compiled_model.hpp

    InferenceEngine::ITaskExecutor::Ptr m_waitExecutor;
+
+private:
+    bool is_serializable();


#13801 (comment)
I changed its visibility and name regarding to the coding style. Thank you.

e-ddykim · 2022-11-17T03:20:19Z

src/plugins/intel_gpu/include/intel_gpu/graph/serialization/layout_serializer.hpp

-            _sizes[1] = _tmp_sizes[1];
-        }
-        buffer << _sizes;
+        buffer << _layout.get_partial_shape();


#13801 (comment)
Updated to serialize partial_shape instead of tensor. Thank you.

e-ddykim · 2022-11-17T03:22:28Z

src/plugins/intel_gpu/src/graph/primitive_inst.cpp

            }
        }
-        OPENVINO_ASSERT(found, _exec_dep_ids[i], "not found in _exec_order");
+        OPENVINO_ASSERT(found, _exec_dep_ids[i], "not found in primitives while rebuilding _exec_deps");


#13801 (comment)
I updated error messages here and other places. Thank you.

e-ddykim · 2022-11-17T03:23:47Z

src/plugins/intel_gpu/src/graph/primitive_inst.cpp

+            std::vector<uint8_t> _buf;
+            _buf.resize(data_size);
+            ib >> cldnn::make_data(_buf.data(), data_size);
+            _outputs[0]->copy_from(get_network().get_stream(), _buf.data());


#13801 (comment)
Updated to use std::vector<uint8_t> instead of new/delete. Thank you.

e-ddykim · 2022-11-17T03:25:27Z

src/plugins/intel_gpu/src/graph/primitive_inst.cpp

-        std::cout << "[get_index_in_deps]: not found" << std::endl;
-
-    return (idx == _deps.size()) ? -1 : idx;
+    IE_THROW() << "[get_index_in_deps]: not found in _deps";


#13801 (comment)
Changed to throw an exception. Thank you.

e-ddykim · 2022-11-17T03:29:38Z

src/plugins/intel_gpu/src/graph/primitive_inst.cpp

 void primitive_inst::build_deps() {
-    if (_node == nullptr)
-        return;
+    OPENVINO_ASSERT(_node != nullptr, "_node should not be nullptr for build_deps.");


#13801 (comment)
Updated to throw an exception when _node is null. Thank you.

e-ddykim · 2022-11-17T04:28:00Z

src/plugins/intel_gpu/src/graph/network.cpp

    int num_data_nodes;
    ib >> num_data_nodes;

-    _memory_pool->clear_pool_for_network(net_id);


#13801 (comment)
I removed it. Thank you.

vladimir-paramuzov · 2022-11-15T14:40:04Z

src/plugins/intel_gpu/src/runtime/kernels_cache.cpp


 void kernels_cache::load(BinaryInputBuffer& ib) {
-    OPENVINO_ASSERT(_engine.type() == engine_types::ocl, "[GPU] not supported engine type");
+    OPENVINO_ASSERT(_engine.type() == engine_types::ocl, "Not supported engine type");


Please keep [GPU] tag for error messages

I added a prefix '[GPU]' to error messages. Thank you.

vladimir-paramuzov · 2022-11-15T14:42:02Z

src/plugins/intel_gpu/src/graph/impls/ocl/dft.cpp

 }  // namespace cldnn

-BIND_BINARY_BUFFER_WITH_TYPE(cldnn::ocl::dft_impl, cldnn::object_type::DFT_IMPL)
+BIND_BINARY_BUFFER_WITH_TYPE(cldnn::ocl::dft_impl)


please setup your ide to insert new line in eof

I updated them. Thank you.

vladimir-paramuzov · 2022-11-16T05:53:16Z

src/plugins/intel_gpu/thirdparty/CMakeLists.txt

        set(ONEDNN_INSTALL_DIR "${CMAKE_CURRENT_BINARY_DIR}/onednn_gpu_install/")
        set(ONEDNN_PREFIX_DIR "${CMAKE_CURRENT_BINARY_DIR}/onednn_gpu_root")
-        execute_process(COMMAND git apply --verbose ../onednn_gpu.patch
+        execute_process(COMMAND git apply ../onednn_gpu.patch OUTPUT_QUIET ERROR_QUIET


Even first cmake run contains this error

vladimir-paramuzov · 2022-11-17T05:43:54Z

src/plugins/intel_gpu/include/intel_gpu/primitives/primitive.hpp

    CLDNN_DEFINE_TYPE_ID(PType)              \
    CLDNN_DEFINE_TYPE_STRING(PType)

+#define CLDNN_DEFINE_PRIMITIVE_TYPE_ID(PType)           \


use INTEL_GPU or GPU prefix intead of CLDNN

I renamed it as GPU_DEFINE_PRIMITIVE_TYPE_ID. Thank you.

vladimir-paramuzov · 2022-11-17T06:45:18Z

src/plugins/intel_gpu/include/intel_gpu/primitives/primitive.hpp

+    }
+
+private:
+    std::unordered_map<std::string, cldnn::primitive_type_id> map;


Maybe we can make this map a static field of primitive_type and insert type_id in primitive_type_base c-tor?

If the point to insert type_id is moved to c-tor, primitives that have not yet been created will not exist in the map. Then, when deserializing, the type_id cannot be obtained by type name, which causes a problem.

primitive_type objects are static, so all primitives are supposed to be initialized on app startup, aren't they?

After adding a ctor in primitive_type_base, I checked the execution step by debugger.
In my test, the first calling point was not on app startup as below.

e-ddykim · 2022-11-17T14:17:45Z

src/plugins/intel_gpu/src/graph/network.cpp

    , _is_primary_stream(false)
    , _reset_arguments(true) {
-    net_id += 1;
+    net_id = get_new_net_id();


#13801 (comment)
I added a new function get_new_net_id() to emit an unique id, and applied it to network ctors. Thank you.

e-ddykim · 2022-11-17T14:50:41Z

src/plugins/intel_gpu/src/plugin/compiled_model.cpp

+// Cache blob format:
+//     [ ConstInputsDataMap / ConstOutputsDataMap ]
+//     [ ov::Node::Input/ ov::Node::Output ]
+//     [ ov::intel_gpu::Graph ]


#13801 (comment)
I added cache blob descriptions here, Graph, network, primitive_inst, typed_primitive_impl_ocl and typed_primitive_onednn_impl. Thank you.

e-ddykim · 2022-11-19T05:05:05Z

src/plugins/intel_gpu/src/graph/mutable_data.cpp

-void mutable_data_inst::save(cldnn::BinaryOutputBuffer& ob) const {
-    parent::save(ob);
-
-    if (!_mem_allocated) {
-        for (size_t dep_idx = 0; dep_idx < _deps.size(); ++dep_idx) {
-            for (size_t m_idx = 0; m_idx < _deps[dep_idx]->_deps.size(); ++m_idx) {
-                if (get_network().get_engine().is_the_same_buffer(*_outputs[0], *_deps[dep_idx]->_deps[m_idx]->_outputs[0])) {
-                    ob << true << dep_idx << m_idx;
-                    return;
-                }
-            }
-        }
-    }
-    ob << false;
-}
-
-void mutable_data_inst::load(cldnn::BinaryInputBuffer& ib) {
-    parent::load(ib);
-
-    bool from_dep;
-    ib >> from_dep;
-    if (from_dep && !_mem_allocated) {
-        size_t dep_idx, m_idx;
-        ib >> dep_idx >> m_idx;
-
-        auto prev_node = get_network().get_primitive(_dep_ids[dep_idx]);
-        _outputs[0] = get_network().get_primitive(prev_node->_dep_ids[m_idx])->output_memory_ptr();
-    }
-}


#13801 (comment)
I removed this logic as you reviewed. Thank you.

e-ddykim · 2022-11-19T08:17:01Z

src/plugins/intel_gpu/src/plugin/plugin.cpp

    } else if (name == ov::caching_properties) {
        std::vector<ov::PropertyName> cachingProperties;
        return decltype(ov::caching_properties)::value_type(cachingProperties);
+    } else if (name == ov::device::architecture) {


#13801 (comment)
I added ov::device::architecture in supported properties. Thank you.

e-ddykim · 2022-11-19T09:47:42Z

src/plugins/intel_gpu/src/graph/primitive_inst.cpp

+//     [ memory dependency information ]
+//     [ execution dependency information ]
+//     [ intermediate memory information ]
+void primitive_inst::save(cldnn::BinaryOutputBuffer& ob) const {


#13801 (comment)
I overrided save and load methods for data/mutable data, and removed a branch. Thank you.

vladimir-paramuzov

Overall LGTM

…13986) * moved serialization include path * quiet onednn-gpu patching * save and load kernels in _impls * changed to use OPENVINO_ASSERT * fix errata * updated to follow OpenVINO naming convention * updated error messages * binary buffer by vector<uint8_t> * partial_shape serialization * removed object_type * added a new storage class for primitive_type_string and id * updated to throw an exception when _node is null in build_deps(). * removed redundant memory_pool clearing * added a new net_id creator * newline at eof * updated CLDNN with GPU * added cache blob descriptions * updated output allocation logic in serialization * added ov::device::architecture in supported properties * overrided save and load in data_inst and mutable_data_inst * removed save and load functions in mutable_data * baseline for serialization unit tests * added serialization unit tests * added serialization unit tests * updated not to execute build_deps when deserialized * make_data without namespace * updated to use default layout c-tor * updated get_unique_net_id() * updated get_type_id() to a pure virtual method * updated ov::caching_properties * added [GPU] tags * updated network c-tor * updated unit tests

e-ddykim force-pushed the gpu-serial_poc2 branch from 8c73c78 to 71874ae Compare November 14, 2022 14:06

e-ddykim commented Nov 14, 2022

View reviewed changes

src/plugins/intel_gpu/src/runtime/CMakeLists.txt Show resolved Hide resolved

e-ddykim mentioned this pull request Nov 14, 2022

[GPU] Graph serialization for GPU #13801

Merged

e-ddykim commented Nov 14, 2022

View reviewed changes

e-ddykim force-pushed the gpu-serial_poc2 branch 2 times, most recently from 1ee8426 to abc9275 Compare November 15, 2022 13:53

e-ddykim commented Nov 17, 2022

View reviewed changes

e-ddykim force-pushed the gpu-serial_poc2 branch from 8392485 to 7b24290 Compare November 17, 2022 04:33

vladimir-paramuzov reviewed Nov 17, 2022

View reviewed changes

e-ddykim force-pushed the gpu-serial_poc2 branch from 2c6ecac to 0538a96 Compare November 17, 2022 10:21

e-ddykim commented Nov 17, 2022

View reviewed changes

e-ddykim force-pushed the gpu-serial_poc2 branch 2 times, most recently from 1962d76 to adb8080 Compare November 19, 2022 04:59

e-ddykim commented Nov 19, 2022

View reviewed changes

e-ddykim marked this pull request as ready for review November 19, 2022 12:54

e-ddykim requested review from a team as code owners November 19, 2022 12:54

e-ddykim added 22 commits November 22, 2022 03:44

updated to throw an exception when _node is null in build_deps().

fee6fbf

removed redundant memory_pool clearing

a9bb84c

added a new net_id creator

c190318

newline at eof

d7d878f

updated CLDNN with GPU

8de74ae

added cache blob descriptions

094ae97

updated output allocation logic in serialization

dd37c11

added ov::device::architecture in supported properties

5e36324

overrided save and load in data_inst and mutable_data_inst

4f805dc

removed save and load functions in mutable_data

fbca09f

baseline for serialization unit tests

b170557

added serialization unit tests

43f71e9

added serialization unit tests

8a4999f

updated not to execute build_deps when deserialized

30a0ea1

make_data without namespace

916123f

updated to use default layout c-tor

bcea37c

updated get_unique_net_id()

2797292

updated get_type_id() to a pure virtual method

1fc6660

updated ov::caching_properties

23d6dfd

added [GPU] tags

183a1d8

updated network c-tor

a0dfa37

updated unit tests

5d3ab0d

e-ddykim requested a review from vladimir-paramuzov November 21, 2022 18:49

vladimir-paramuzov added the category: GPU OpenVINO GPU plugin label Nov 22, 2022

vladimir-paramuzov added this to the 2022.3 milestone Nov 22, 2022

vladimir-paramuzov approved these changes Nov 22, 2022

View reviewed changes

yeonbok enabled auto-merge (squash) November 22, 2022 06:11

yeonbok merged commit 0b1e366 into openvinotoolkit:master Nov 22, 2022

e-ddykim deleted the gpu-serial_poc2 branch February 8, 2024 05:29

Conversation

e-ddykim commented Nov 14, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Details:

Tickets:

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vladimir-paramuzov left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

e-ddykim commented Nov 14, 2022 •

edited

Loading