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.

Sign up for GitHub

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

Jump to bottom

[GPU] POC for shared plugin config #28667

Open
wants to merge 43 commits into
base: master
Choose a base branch
from
Open

[GPU] POC for shared plugin config #28667

wants to merge 43 commits into from
+2,135 −1,904

Conversation

vladimir-paramuzov
Copy link
Contributor

Details:

  • Added common base class for plugin configuration and migration in GPU plugin.
  • New config defines options as typed members of config class which shall provide more efficient access to the value than AnyMap
  • Property setting is possible before finalization only which shall localize the point when config become immutable.
  • All config options may be read from environment as well as from json config file when OV is built w/ debug caps.
  • Debug and release options are defined and handled similarly now

@vladimir-paramuzov vladimir-paramuzov requested review from a team as code owners January 24, 2025 13:09
@github-actions github-actions bot added category: inference OpenVINO Runtime library - Inference category: Core OpenVINO Core (aka ngraph) category: GPU OpenVINO GPU plugin category: CPU OpenVINO CPU plugin category: build OpenVINO cmake script / infra labels Jan 24, 2025
@dmitry-gorokhov dmitry-gorokhov mentioned this pull request Jan 24, 2025
Open
@vladimir-paramuzov vladimir-paramuzov force-pushed the new_config_poc branch 2 times, most recently from ea3c7c1 to 1bc7a7b Compare January 27, 2025 06:06
t-jankowski
t-jankowski reviewed Jan 27, 2025
View reviewed changes
src/core/src/any.cpp Outdated
Comment on lines 207 to 208
std::set<std::string> off = {"0", "false", "off", "no"};
std::set<std::string> on = {"1", "true", "on", "yes"};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
std::set<std::string> off = {"0", "false", "off", "no"};
std::set<std::string> on = {"1", "true", "on", "yes"};
static const std::set<std::string> off = {"0", "false", "off", "no"};
static const std::set<std::string> on = {"1", "true", "on", "yes"};

@vladimir-paramuzov
Base impl
6ad934b 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
make it common
e5a286c 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
env and config
67fb59c 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
Replace old config & fixes
e5f7cc3 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
prefix for config members and unit tests
90572bd 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
added visibility for options
f87d45b 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
remove old config
4017ffb 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
enhancements
36f0bf6 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
update behavior for set/get property. Add help message
4cb213e 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
refactor
a899aca 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
Hide config class members
b0b5ab2 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
Options visibility update
f57e7a1 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
Fixes and visit_attributes method impl
d84741e 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
Refactor debug knobs
29c978e 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
split set_prop and set_user_prop again
e830be2 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
wip
c652884 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
[GPU] Global static vars. Removed old debug config
c53bb9b 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
fix visitors
db3e55b 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
build fixes
abdec31 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
minor fixes
81ca005 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
cut off debug properties for release build
ea6f10f 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
config clone. Visibility fixes
b7dc614 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
added comment about config copy
a00e1e8 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
fix tests
7fe88be 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
build fixes
b2f9000 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
cpu test fix
e2e8682 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
improve bool any parsing
3402f1e 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
fix cpplint
0ea319b 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
fix build errors
e4d85e0 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
build fixes
a694b8f 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
build fix
d1cd693 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
unit test fixes
6794136 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
fix func tests
6747b23 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
don't visit finalize flag
5bef3d2 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@praasz praasz self-assigned this Jan 28, 2025
@vladimir-paramuzov
few fixes and refactor
15051c2 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
debug caps fixes and tests
69d6dc4 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
Update env/config usage
e50d8bb 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
removed json dep in unit tests
c9c9e84 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
fix env setting
1c2e047 
Signed-off-by: Vladimir Paramuzov <[email protected]>
@vladimir-paramuzov
fix env unset
0a11334 
Signed-off-by: Vladimir Paramuzov <[email protected]>
sshlyapn
sshlyapn reviewed Jan 30, 2025
View reviewed changes
src/inference/dev_api/openvino/runtime/plugin_config.hpp
}
}

void handle_option(ConfigOptionBase* option) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
void handle_option(ConfigOptionBase* option) {
void handle_option(const ConfigOptionBase* option) {

src/inference/src/dev/plugin_config.cpp
Comment on lines +200 to +201
const std::set<std::string> off = {"0", "false", "off", "no"};
const std::set<std::string> on = {"1", "true", "on", "yes"};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const std::set<std::string> off = {"0", "false", "off", "no"};
const std::set<std::string> on = {"1", "true", "on", "yes"};
constexpr std::set<std::string> off = {"0", "false", "off", "no"};
constexpr std::set<std::string> on = {"1", "true", "on", "yes"};

src/plugins/intel_gpu/src/runtime/execution_config.cpp
// In the case of inner program, allow_new_shape_infer flag is setted by outside of program.
// So, do not check allow_new_shape_infer for inner program build
for (const auto& op : ops) {
if (auto multi_subgraph_op = ov::as_type_ptr<op::util::MultiSubGraphOp>(op)) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to have nested subgraphs? This can be moved to process_op() for recurrent processing

src/plugins/intel_gpu/src/runtime/execution_config.cpp
}
if (!info.supports_immad || !is_llm)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Activation scaleing should be applied only if !is_llm:

Suggested change
if (!info.supports_immad || !is_llm)
if (!is_llm)

src/inference/dev_api/openvino/runtime/plugin_config.hpp

virtual void set_any(const ov::Any any) = 0;
virtual ov::Any get_any() const = 0;
virtual bool is_valid_value(ov::Any val) = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
virtual bool is_valid_value(ov::Any val) = 0;
virtual bool is_valid_value(ov::Any val) const = 0;

src/core/src/any.cpp
@@ -6,6 +6,9 @@

#include <limits>
#include <string>
#include <string_view>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This include seems unused

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually used below: std::array<std::string_view, 4>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t-jankowski t-jankowski t-jankowski left review comments

@sshlyapn sshlyapn sshlyapn left review comments

At least 1 approving review is required to merge this pull request.

Assignees

@sshlyapn sshlyapn

@dmitry-gorokhov dmitry-gorokhov

@praasz praasz

Labels
category: build OpenVINO cmake script / infra category: Core OpenVINO Core (aka ngraph) category: CPU OpenVINO CPU plugin category: GPU OpenVINO GPU plugin category: inference OpenVINO Runtime library - Inference
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@vladimir-paramuzov @sshlyapn @t-jankowski @dmitry-gorokhov @praasz