-
Notifications
You must be signed in to change notification settings - Fork 267
[xpupti] Support more than 1 device in ptiMetricsScopeConfigure #1479
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
291e3e8
9dc8a64
f2275e5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,10 @@ | |
| #include <fmt/ostream.h> | ||
| #include <fmt/ranges.h> | ||
|
|
||
| #include <algorithm> | ||
| #include <iterator> | ||
| #include <ranges> | ||
|
|
||
| namespace KINETO_NAMESPACE { | ||
|
|
||
| // number of scopes affect the size of counter data binary used by | ||
|
|
@@ -33,6 +37,14 @@ bool XpuptiScopeProfilerConfig::handleOption( | |
| xpuptiProfilerPerKernel_ = toBool(val); | ||
| } else if (name == "XPUPTI_PROFILER_MAX_SCOPES"sv) { | ||
| xpuptiProfilerMaxScopes_ = toInt64(val); | ||
| } else if (name == "XPUPTI_PROFILER_DEVICES"sv) { | ||
| const auto tokens = splitAndTrim(val, ','); | ||
| const auto nonEmpty = [](const std::string& tok) { return !tok.empty(); }; | ||
| const auto toIndex = [this](const std::string& tok) { return toInt32(tok); }; | ||
| xpuptiProfilerDevices_.clear(); | ||
| std::ranges::copy( | ||
| tokens | std::views::filter(nonEmpty) | std::views::transform(toIndex), | ||
| std::back_inserter(xpuptiProfilerDevices_)); | ||
| } else { | ||
| return false; | ||
| } | ||
|
|
@@ -53,10 +65,14 @@ void XpuptiScopeProfilerConfig::printActivityProfilerConfig( | |
| s, | ||
| "Xpupti Profiler metrics : {}\n" | ||
| "Xpupti Profiler measure per kernel : {}\n" | ||
| "Xpupti Profiler max scopes : {}\n", | ||
| "Xpupti Profiler max scopes : {}\n" | ||
| "Xpupti Profiler devices : {}\n", | ||
| fmt::join(activitiesXpuptiMetrics_, ", "), | ||
| xpuptiProfilerPerKernel_, | ||
| xpuptiProfilerMaxScopes_); | ||
| xpuptiProfilerMaxScopes_, | ||
| xpuptiProfilerDevices_.empty() | ||
| ? std::string("all") | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "auto" may be more accurate than "all" here - For example, on a host where the workload only touches 4 of 16 cards, only 4 get profiled, not "all." |
||
| : fmt::format("{}", fmt::join(xpuptiProfilerDevices_, ", "))); | ||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,13 +6,16 @@ | |
| * LICENSE file in the root directory of this source tree. | ||
| */ | ||
|
|
||
| #include "src/plugin/xpupti/XpuptiScopeProfilerApi.h" | ||
| #include "src/plugin/xpupti/XpuptiScopeProfilerConfig.h" | ||
|
|
||
| #include <fmt/core.h> | ||
| #include <fmt/ranges.h> | ||
|
|
||
| #include <gtest/gtest.h> | ||
|
|
||
| #include <array> | ||
|
|
||
| namespace KN = KINETO_NAMESPACE; | ||
|
|
||
| class XpuptiScopeProfilerConfigTest : public ::testing::Test { | ||
|
|
@@ -68,3 +71,66 @@ TEST_F(XpuptiScopeProfilerConfigTest, ScopesDefaults) { | |
| EXPECT_EQ(user_scopes, 10); | ||
| EXPECT_EQ(auto_scopes, 1500); | ||
| } | ||
|
|
||
| TEST_F(XpuptiScopeProfilerConfigTest, DevicesDefaultEmpty) { | ||
| KN::Config cfg; | ||
| EXPECT_TRUE(cfg.parse("XPUPTI_PROFILER_METRICS = metric1")); | ||
| // No XPUPTI_PROFILER_DEVICES set -> empty means "all devices". | ||
| const KN::XpuptiScopeProfilerConfig& xpupti_cfg = | ||
| KN::XpuptiScopeProfilerConfig::get(cfg); | ||
| EXPECT_TRUE(xpupti_cfg.xpuptiProfilerDevices().empty()); | ||
| } | ||
|
|
||
| TEST_F(XpuptiScopeProfilerConfigTest, DevicesParsedList) { | ||
| KN::Config cfg; | ||
| EXPECT_TRUE(cfg.parse("XPUPTI_PROFILER_METRICS = metric1")); | ||
| EXPECT_TRUE(cfg.parse("XPUPTI_PROFILER_DEVICES = 0, 2, 3")); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: would be nice to add a test for testing empty "tokens", for example: Trailing comma + doubled comma: empty tokens must be skipped, not parsed as index 0 or rejected as invalid integers. Additionally, what if there are duplicated indices: XPUPTI_PROFILER_DEVICES=0,0,2? |
||
| const KN::XpuptiScopeProfilerConfig& xpupti_cfg = | ||
| KN::XpuptiScopeProfilerConfig::get(cfg); | ||
| const std::vector<int> expected{0, 2, 3}; | ||
| EXPECT_EQ(xpupti_cfg.xpuptiProfilerDevices(), expected); | ||
| } | ||
|
|
||
| TEST_F(XpuptiScopeProfilerConfigTest, DevicesSingle) { | ||
| KN::Config cfg; | ||
| EXPECT_TRUE(cfg.parse("XPUPTI_PROFILER_DEVICES = 1")); | ||
| const KN::XpuptiScopeProfilerConfig& xpupti_cfg = | ||
| KN::XpuptiScopeProfilerConfig::get(cfg); | ||
| const std::vector<int> expected{1}; | ||
| EXPECT_EQ(xpupti_cfg.xpuptiProfilerDevices(), expected); | ||
| } | ||
|
|
||
| TEST_F(XpuptiScopeProfilerConfigTest, SelectDeviceHandlesSubset) { | ||
| // Fabricate 4 distinct opaque handles; the helper never dereferences them. | ||
| std::array<pti_device_handle_t, 4> handles{ | ||
| reinterpret_cast<pti_device_handle_t>(0x10), | ||
| reinterpret_cast<pti_device_handle_t>(0x20), | ||
| reinterpret_cast<pti_device_handle_t>(0x30), | ||
| reinterpret_cast<pti_device_handle_t>(0x40)}; | ||
| const std::vector<int> indices{0, 2, 3}; | ||
| auto out = KN::selectDeviceHandles(handles, indices); | ||
| ASSERT_EQ(out.size(), 3u); | ||
| EXPECT_EQ(out[0], handles[0]); | ||
| EXPECT_EQ(out[1], handles[2]); | ||
| EXPECT_EQ(out[2], handles[3]); | ||
| } | ||
|
|
||
| TEST_F(XpuptiScopeProfilerConfigTest, SelectDeviceHandlesOutOfRangeThrows) { | ||
| std::array<pti_device_handle_t, 2> handles{ | ||
| reinterpret_cast<pti_device_handle_t>(0x10), | ||
| reinterpret_cast<pti_device_handle_t>(0x20)}; | ||
| const std::vector<int> indices{0, 5}; | ||
| EXPECT_THROW( | ||
| KN::selectDeviceHandles(handles, indices), | ||
| std::runtime_error); | ||
| } | ||
|
|
||
| TEST_F(XpuptiScopeProfilerConfigTest, SelectDeviceHandlesNegativeThrows) { | ||
| std::array<pti_device_handle_t, 2> handles{ | ||
| reinterpret_cast<pti_device_handle_t>(0x10), | ||
| reinterpret_cast<pti_device_handle_t>(0x20)}; | ||
| const std::vector<int> indices{-1}; | ||
| EXPECT_THROW( | ||
| KN::selectDeviceHandles(handles, indices), | ||
| std::runtime_error); | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment was inaccurate. When requested devices is 0, it will profile whichever devices the workload actually uses.
Note: you may also want to update the PR body.