Roctracer perfetto flow fixes (#267)

* testing label updates - automatically add "gpu", "roctracer", "rocm-smi", and "rocprofiler" test labels when appropriate * Bump version to v1.9.1 * roctracer and config updates - fix perfetto::Flow - use roctracer correlation ID instead of critical trace correlation ID - renamed ambiguous _cid, _parent_cid, _corr_id variables to _crit_cid, _parent_crit_cid, _roct_cid - use atomic_{mutex,lock} instead of STL mutex/lock - support for individual perfetto annotations for HIP API args - OMNITRACE_PERFETTO_COMPACT_ROCTRACER_ANNOTATIONS option for controlling compact vs. individual perfetto annotations for HIP API args * Update timemory submodule - argparser updates - help prints to std::cout by default now - supports setting custom ostream * cmake formatting * config::get_setting_value updates - config::get_setting_value returns std::optional instead of std::pair<bool, Tp>
ROCm · Mar 23, 2023 · 279a8e0 · 279a8e0
1 parent 9eafb23
commit 279a8e0
Show file tree

Hide file tree

Showing 11 changed files with 173 additions and 89 deletions.
diff --git a/VERSION b/VERSION
@@ -1 +1 @@
-1.9.0
+1.9.1
diff --git a/external/timemory b/external/timemory
diff --git a/source/lib/core/config.cpp b/source/lib/core/config.cpp
@@ -406,11 +406,7 @@ configure_settings(bool _init)
         "durations are needed, see OMNITRACE_TRACE_PERIODS.",
         0.0, "trace", "profile", "perfetto", "timemory");
 
-    auto _clock_s =
-        config::get_setting_value<std::string>("OMNITRACE_TRACE_PERIOD_CLOCK_ID").second;
-
     auto _clock_choices = std::vector<std::string>{};
-
     for(const auto& itr : constraint::get_valid_clock_ids())
     {
         _clock_choices.emplace_back(
@@ -696,6 +692,18 @@ configure_settings(bool _init)
                              "feature may dramatically reduce the size of the trace",
                              true, "perfetto", "data", "debugging", "advanced");
 
+    OMNITRACE_CONFIG_SETTING(
+        bool, "OMNITRACE_PERFETTO_COMPACT_ROCTRACER_ANNOTATIONS",
+        "When PERFETTO_ANNOTATIONS, USE_ROCTRACER, and ROCTRACER_HIP_API are all "
+        "enabled, enabling this option will result in the arg information for HIP API "
+        "calls to all be within one annotation (e.g., args=\"stream=0x0, dst=0x1F, "
+        "sizeBytes=64, src=0x08, kind=1\"). When disabled, each parameter will be an "
+        "individual annotation (e.g. stream, dst, sizeBytes, etc.). The benefit of the "
+        "former is that it is faster to serialize and consumes less file space; the "
+        "benefit of the latter is that it becomes much easier to find slices in the "
+        "trace with the same value",
+        false, "perfetto", "data", "debugging", "roctracer", "rocm", "advanced");
+
     OMNITRACE_CONFIG_SETTING(
         uint64_t, "OMNITRACE_THREAD_POOL_SIZE",
         "Max number of threads for processing background tasks",
@@ -1095,7 +1103,7 @@ configure_mode_settings(const std::shared_ptr<settings>& _config)
         }
         else
         {
-            bool _changed = get_setting_value<bool>(_name).second != _v;
+            bool _changed = get_setting_value<bool>(_name).value_or(!_v) != _v;
             OMNITRACE_BASIC_VERBOSE(
                 1 && _changed,
                 "[configure_mode_settings] Overriding %s to %s in %s mode...\n",
@@ -1105,7 +1113,7 @@ configure_mode_settings(const std::shared_ptr<settings>& _config)
     };
 
     auto _use_causal = get_setting_value<bool>("OMNITRACE_USE_CAUSAL");
-    if(_use_causal.first && _use_causal.second) set_env("OMNITRACE_MODE", "causal", 1);
+    if(_use_causal && *_use_causal) set_env("OMNITRACE_MODE", "causal", 1);
 
     if(get_mode() == Mode::Coverage)
     {

diff --git a/source/lib/core/config.hpp b/source/lib/core/config.hpp
@@ -126,15 +126,15 @@ set_default_setting_value(const std::string& _name, Tp&& _v)
 }
 
 template <typename Tp>
-std::pair<bool, Tp>
+std::optional<Tp>
 get_setting_value(const std::string& _name)
 {
     auto _instance = tim::settings::shared_instance();
-    if(!_instance) return std::make_pair(false, Tp{});
+    if(!_instance) return std::optional<Tp>{};
     auto _setting = _instance->find(_name);
-    if(_setting == _instance->end() || !_setting->second)
-        return std::make_pair(false, Tp{});
-    return _setting->second->get<Tp>();
+    if(_setting == _instance->end() || !_setting->second) return std::optional<Tp>{};
+    auto&& _ret = _setting->second->get<Tp>();
+    return (_ret.first) ? std::optional<Tp>{ _ret.second } : std::optional<Tp>{};
 }
 
 //

diff --git a/source/lib/core/constraint.cpp b/source/lib/core/constraint.cpp
@@ -232,9 +232,10 @@ spec::spec(const std::string& _clock_id, double _delay, double _dur, uint64_t _n
 {}
 
 spec::spec(const std::string& _line)
-: spec{ config::get_setting_value<std::string>("OMNITRACE_TRACE_PERIOD_CLOCK_ID").second,
-        config::get_setting_value<double>("OMNITRACE_TRACE_DELAY").second,
-        config::get_setting_value<double>("OMNITRACE_TRACE_DURATION").second }
+: spec{ config::get_setting_value<std::string>("OMNITRACE_TRACE_PERIOD_CLOCK_ID")
+            .value_or("CLOCK_REALTIME"),
+        config::get_setting_value<double>("OMNITRACE_TRACE_DELAY").value_or(0.0),
+        config::get_setting_value<double>("OMNITRACE_TRACE_DURATION").value_or(0.0) }
 {
     auto _delim = tim::delimit(_line, ":");
     if(!_delim.empty()) delay = utility::convert<double>(_delim.at(0));
@@ -300,12 +301,13 @@ get_trace_specs()
     auto _v = std::vector<constraint::spec>{};
 
     {
-        auto _delay_v = config::get_setting_value<double>("OMNITRACE_TRACE_DELAY").second;
+        auto _delay_v =
+            config::get_setting_value<double>("OMNITRACE_TRACE_DELAY").value_or(0.0);
         auto _duration_v =
-            config::get_setting_value<double>("OMNITRACE_TRACE_DURATION").second;
+            config::get_setting_value<double>("OMNITRACE_TRACE_DURATION").value_or(0.0);
         auto _clock_v = find_clock_identifier(
             config::get_setting_value<std::string>("OMNITRACE_TRACE_PERIOD_CLOCK_ID")
-                .second);
+                .value_or("CLOCK_REALTIME"));
 
         if(_delay_v > 0.0 || _duration_v > 0.0)
         {
@@ -315,7 +317,8 @@ get_trace_specs()
 
     {
         auto _periods_v =
-            config::get_setting_value<std::string>("OMNITRACE_TRACE_PERIODS").second;
+            config::get_setting_value<std::string>("OMNITRACE_TRACE_PERIODS")
+                .value_or("");
         if(!_periods_v.empty())
         {
             for(auto itr : tim::delimit(_periods_v, " ;\t\n"))

diff --git a/source/lib/omnitrace/library/causal/data.cpp b/source/lib/omnitrace/library/causal/data.cpp
@@ -96,8 +96,8 @@ auto&
 get_engine()
 {
     static auto _seed = []() -> hash_value_t {
-        auto _seed_v =
-            config::get_setting_value<uint64_t>("OMNITRACE_CAUSAL_RANDOM_SEED").second;
+        auto _seed_v = config::get_setting_value<uint64_t>("OMNITRACE_CAUSAL_RANDOM_SEED")
+                           .value_or(0);
         if(_seed_v == 0) _seed_v = std::random_device{}();
         return _seed_v;
     }();
@@ -138,7 +138,7 @@ get_filters(std::set<binary::scope_filter::filter_scope> _scopes = {
 
     bool _use_default_excludes =
         config::get_setting_value<bool>("OMNITRACE_CAUSAL_FUNCTION_EXCLUDE_DEFAULTS")
-            .second;
+            .value_or(true);
 
     if(_use_default_excludes && _scopes.count(sf::FUNCTION_FILTER) > 0)
     {
@@ -471,9 +471,9 @@ perform_experiment_impl(std::shared_ptr<std::promise<void>> _started)  // NOLINT
     std::this_thread::sleep_for(std::chrono::milliseconds{ 10 });
 
     double _delay_sec =
-        config::get_setting_value<double>("OMNITRACE_CAUSAL_DELAY").second;
+        config::get_setting_value<double>("OMNITRACE_CAUSAL_DELAY").value_or(0.0);
     double _duration_sec =
-        config::get_setting_value<double>("OMNITRACE_CAUSAL_DURATION").second;
+        config::get_setting_value<double>("OMNITRACE_CAUSAL_DURATION").value_or(0.0);
     auto _duration_nsec = duration_nsec_t{ _duration_sec * units::sec };
 
     if(_delay_sec > 0.0)

diff --git a/source/lib/omnitrace/library/causal/experiment.cpp b/source/lib/omnitrace/library/causal/experiment.cpp
@@ -493,7 +493,7 @@ experiment::save_experiments(std::string _fname_base, const filename_config_t& _
     }
 
     bool _causal_output_reset =
-        config::get_setting_value<bool>("OMNITRACE_CAUSAL_FILE_RESET").second;
+        config::get_setting_value<bool>("OMNITRACE_CAUSAL_FILE_RESET").value_or(false);
 
     // if(current_record.experiments.empty()) return;
 

diff --git a/source/lib/omnitrace/library/coverage.cpp b/source/lib/omnitrace/library/coverage.cpp
@@ -222,9 +222,8 @@ post_process()
 
     auto _get_setting = [](const std::string& _v) {
         auto&& _b = config::get_setting_value<bool>(_v);
-        OMNITRACE_CI_THROW(!_b.first, "Error! No configuration setting named '%s'",
-                           _v.c_str());
-        return (_b.first) ? _b.second : true;
+        OMNITRACE_CI_THROW(!_b, "Error! No configuration setting named '%s'", _v.c_str());
+        return _b.value_or(true);
     };
 
     auto _text_output = _get_setting("OMNITRACE_TEXT_OUTPUT");

diff --git a/source/lib/omnitrace/library/kokkosp.cpp b/source/lib/omnitrace/library/kokkosp.cpp
@@ -269,10 +269,10 @@ extern "C"
 
         _name_len_limit = omnitrace::config::get_setting_value<int64_t>(
                               "OMNITRACE_KOKKOSP_NAME_LENGTH_MAX")
-                              .second;
+                              .value_or(_name_len_limit);
         _kp_prefix =
             omnitrace::config::get_setting_value<std::string>("OMNITRACE_KOKKOSP_PREFIX")
-                .second;
+                .value_or(_kp_prefix);
     }
 
     void kokkosp_finalize_library()