diff --git a/libgraph/src/SharedMemSys.cpp b/libgraph/src/SharedMemSys.cpp index c6c96c07d9..292bd3ca2b 100644 --- a/libgraph/src/SharedMemSys.cpp +++ b/libgraph/src/SharedMemSys.cpp @@ -20,10 +20,12 @@ #include "katana/SharedMemSys.h" #include "katana/CommBackend.h" +#include "katana/Experimental.h" #include "katana/Galois.h" #include "katana/GaloisRuntime.h" #include "katana/Logging.h" #include "katana/Plugin.h" +#include "katana/Strings.h" #include "katana/TextTracer.h" #include "tsuba/FileStorage.h" #include "tsuba/tsuba.h" @@ -44,6 +46,23 @@ katana::SharedMemSys::SharedMemSys(std::unique_ptr tracer) if (auto init_good = tsuba::Init(&comm_backend); !init_good) { KATANA_LOG_FATAL("tsuba::Init: {}", init_good.error()); } + + auto features_on = katana::internal::ExperimentalFeature::ReportEnabled(); + if (!features_on.empty()) { + auto feature_string = katana::Join(features_on, ","); + tracer->GetActiveSpan().SetTags( + {{"experimental_features_enabled", feature_string}}); + } + + auto unrecognized = + katana::internal::ExperimentalFeature::ReportUnrecognized(); + if (!unrecognized.empty()) { + KATANA_LOG_WARN( + "these values from KATANA_ENABLE_EXPERIMENTAL did not match any " + "features:\n\t{}", + katana::Join(unrecognized, " ")); + } + katana::ProgressTracer::Set(std::move(tracer)); } diff --git a/libsupport/CMakeLists.txt b/libsupport/CMakeLists.txt index f43c6d7f3f..0b22f92e26 100644 --- a/libsupport/CMakeLists.txt +++ b/libsupport/CMakeLists.txt @@ -19,16 +19,17 @@ set(sources src/EntityTypeManager.cpp src/Env.cpp src/ErrorCode.cpp - src/HostAllocator.cpp + src/Experimental.cpp src/HTTP.cpp + src/HostAllocator.cpp src/JSON.cpp src/JSONTracer.cpp src/Logging.cpp src/NoopTracer.cpp - src/Random.cpp - src/Result.cpp src/Plugin.cpp src/ProgressTracer.cpp + src/Random.cpp + src/Result.cpp src/Signals.cpp src/Strings.cpp src/TextTracer.cpp diff --git a/libsupport/include/katana/Experimental.h b/libsupport/include/katana/Experimental.h new file mode 100644 index 0000000000..f864f7e107 --- /dev/null +++ b/libsupport/include/katana/Experimental.h @@ -0,0 +1,122 @@ +#ifndef KATANA_LIBSUPPORT_KATANA_EXPERIMENTAL_H_ +#define KATANA_LIBSUPPORT_KATANA_EXPERIMENTAL_H_ + +#include +#include +#include +#include +#include + +#include "katana/config.h" + +namespace katana { +namespace internal { + +/// ExperimentalFeature tracks the state of feature flags set in the environment; +/// It is not intended to be used directly; please see the macro KATANA_EXPERIMENTAL_FEATURE +/// below +class KATANA_EXPORT ExperimentalFeature { +public: + static ExperimentalFeature* Register( + const std::string& feature_name, const std::string& filename, + int line_number); + + /// report the feature flags that were checked on codepaths that were executed and + /// the flag was set to true + static std::vector ReportEnabled(); + + /// report the feature flags that were used but stayed false + static std::vector ReportDisabled(); + + /// report the feature flags that were provided but did not match any registered flag + static std::vector ReportUnrecognized(); + + bool IsEnabled() { return is_enabled_; } + + ExperimentalFeature(const ExperimentalFeature& no_copy) = delete; + ExperimentalFeature& operator=(const ExperimentalFeature& no_copy) = delete; + ExperimentalFeature(ExperimentalFeature&& no_move) = delete; + ExperimentalFeature& operator=(ExperimentalFeature&& no_move) = delete; + ~ExperimentalFeature() = default; + + const std::string& name() const { return name_; } + const std::string& filename() const { return filename_; } + int line_number() const { return line_number_; } + +private: + ExperimentalFeature(std::string name, std::string filename, int line_number) + : name_(std::move(name)), + filename_(std::move(filename)), + line_number_(line_number) { + CheckEnv(); + } + + void CheckEnv(); + + std::string name_; + std::string filename_; + int line_number_{}; + bool is_enabled_{}; + + static std::mutex registered_features_mutex_; + static std::unordered_map> + registered_features_; +}; + +} // namespace internal +} // namespace katana + +/// KATANA_EXPERIMENTAL_FEATURE creates a flag that can be set from the environment. +/// The macro takes a feature_name which should be an unquoted, unique string that +/// looks like a function name. Developers can then use the macro +/// KATANA_EXPERIMENTAL_ENABLED using the same string to detect if the flag was set. +/// +/// Flags are set using the environment variable KATANA_ENABLE_EXPERIMENTAL. Users +/// pass the same string passed to KATANA_EXPERIMENTAL_FEATURE to set a particular +/// flag. Multiple flags may be set by providing a comma delimited list of feature +/// names. +/// +/// NB: an implication of the above is that these flags are only useful for runtime +/// configuration. If the desire is to control compile-time changes, a different +/// mechanism is required. +/// +/// Example: +/// +/// Program env: +/// KATANA_ENABLE_EXPERIMENTAL="UnstableButUseful" +/// +/// active_development.cpp: +/// KATANA_EXPERIMENTAL_FEATURE(UnstableButUseful); +/// +/// void important_function() { +/// if (KATANA_EXPERIMENTAL_ENABLED(UnstableButUseful)) { +/// // do something useful +/// } else { +/// // be conservative +/// } +/// } +/// +/// Flags declared in different parts of the codebase can conflict. A good +/// practice is to choose good name for your feature and grep for this macro +/// to be sure it does not collide with another. +/// +/// Another best practice is to heavily comment where the macro is defined, +/// detailing what the feature does and the state of tests to avoid regressions. +#define KATANA_EXPERIMENTAL_FEATURE(feature_name) \ + namespace katana::internal { \ + class ExperimentalFeature; \ + static auto* katana_experimental_feature_ptr_##feature_name = \ + ::katana::internal::ExperimentalFeature::Register( \ + #feature_name, __FILE__, __LINE__); \ + } \ + static_assert( \ + std::is_same< \ + ::katana::internal::ExperimentalFeature, \ + katana::internal::ExperimentalFeature>::value, \ + "KATANA_EXPERIMENTAL_FEATURE must not be inside a namespace block") + +#define KATANA_EXPERIMENTAL_ENABLED(feature_name) \ + (::katana::internal::katana_experimental_feature_ptr_##feature_name \ + ->IsEnabled()) + +#endif diff --git a/libsupport/src/Experimental.cpp b/libsupport/src/Experimental.cpp new file mode 100644 index 0000000000..24d3ee13c4 --- /dev/null +++ b/libsupport/src/Experimental.cpp @@ -0,0 +1,132 @@ +#include "katana/Experimental.h" + +#include +#include + +#include "katana/Env.h" +#include "katana/Logging.h" +#include "katana/Strings.h" + +namespace { + +class ExperimentalFeatureEnvState { +public: + static ExperimentalFeatureEnvState* Get() { + std::call_once(init_flag_, [&]() { + state_ = std::unique_ptr( + new ExperimentalFeatureEnvState()); + }); + return state_.get(); + } + + bool WasInEnv(const std::string& feature) { + auto it = features_used_.find(feature); + if (it == features_used_.end()) { + return false; + } + it->second = true; + return true; + } + + const std::unordered_map& features_used() { + return features_used_; + } + +private: + ExperimentalFeatureEnvState() { + std::string val; + if (!katana::GetEnv("KATANA_ENABLE_EXPERIMENTAL", &val)) { + return; + } + + auto strings = katana::SplitView(val, ","); + for (const auto& feature : strings) { + features_used_.emplace(feature, false); + } + } + + std::unordered_map features_used_; + + static std::once_flag init_flag_; + static std::unique_ptr state_; +}; + +std::once_flag ExperimentalFeatureEnvState::init_flag_; + +std::unique_ptr + ExperimentalFeatureEnvState::state_; + +} // namespace + +katana::internal::ExperimentalFeature* +katana::internal::ExperimentalFeature::Register( + const std::string& feature_name, const std::string& filename, + int line_number) { + std::lock_guard lock(registered_features_mutex_); + + auto [it, was_created] = registered_features_.emplace( + feature_name, + std::unique_ptr( + new ExperimentalFeature(feature_name, filename, line_number))); + const auto& flag = it->second; + if (!was_created && + (flag->filename() != filename || flag->line_number() != line_number)) { + KATANA_LOG_WARN( + "{} declared in multiple places:\n\there: {}:{}\n\tand here: {}:{}", + feature_name, flag->filename(), flag->line_number(), filename, + line_number); + } + return it->second.get(); +} + +void +katana::internal::ExperimentalFeature::CheckEnv() { + is_enabled_ = ExperimentalFeatureEnvState::Get()->WasInEnv(name_); +} + +std::vector +katana::internal::ExperimentalFeature::ReportEnabled() { + std::lock_guard lock(registered_features_mutex_); + + std::vector enabled; + + for (const auto& [name, ptr] : registered_features_) { + if (ptr->IsEnabled()) { + enabled.emplace_back(name); + } + } + return enabled; +} + +std::vector +katana::internal::ExperimentalFeature::ReportDisabled() { + std::lock_guard lock(registered_features_mutex_); + + std::vector disabled; + + for (const auto& [name, ptr] : registered_features_) { + if (!ptr->IsEnabled()) { + disabled.emplace_back(name); + } + } + return disabled; +} + +std::vector +katana::internal::ExperimentalFeature::ReportUnrecognized() { + std::vector unused; + + for (const auto& [name, was_used] : + ExperimentalFeatureEnvState::Get()->features_used()) { + if (!was_used) { + unused.emplace_back(name); + } + } + return unused; +} + +std::mutex katana::internal::ExperimentalFeature::registered_features_mutex_; + +std::unordered_map< + std::string, std::unique_ptr> + katana::internal::ExperimentalFeature::registered_features_; diff --git a/libsupport/test/CMakeLists.txt b/libsupport/test/CMakeLists.txt index 5e72205aca..d7365dba6e 100644 --- a/libsupport/test/CMakeLists.txt +++ b/libsupport/test/CMakeLists.txt @@ -22,6 +22,7 @@ add_unit_test(bitmath) add_unit_test(cache) add_unit_test(disjoint_range_iterator) add_unit_test(env) +add_unit_test(experimental) add_unit_test(logging) add_unit_test(opaque-id) add_unit_test(random) @@ -33,6 +34,9 @@ add_unit_test(type-manager) add_unit_test(uri) add_unit_test(zip_iterator) +set_tests_properties(experimental-test PROPERTIES + ENVIRONMENT KATANA_ENABLE_EXPERIMENTAL=TestOn,TestSecond,DefinedButUnused,EnvironmentOnly) + add_executable(arrow-bench arrow-bench.cpp) target_link_libraries(arrow-bench katana_support benchmark::benchmark) add_test(NAME arrow-bench COMMAND arrow-bench --benchmark_filter=/1024) diff --git a/libsupport/test/experimental.cpp b/libsupport/test/experimental.cpp new file mode 100644 index 0000000000..d5c104437a --- /dev/null +++ b/libsupport/test/experimental.cpp @@ -0,0 +1,44 @@ +#include "katana/Experimental.h" + +#include + +#include "katana/Logging.h" + +KATANA_EXPERIMENTAL_FEATURE(TestOn); +KATANA_EXPERIMENTAL_FEATURE(TestOff); +KATANA_EXPERIMENTAL_FEATURE(TestSecond); + +KATANA_EXPERIMENTAL_FEATURE(DefinedButUnused); + +// this causes the compiler to print: +// error: static assertion failed: KATANA_EXPERIMENTAL_FEATURE must not be +// inside a namespace block +// +// namespace test { +// +// KATANA_EXPERIMENTAL_FEATURE(ShouldNotCompile); +// +// } // namespace test + +int +main() { + KATANA_LOG_ASSERT(KATANA_EXPERIMENTAL_ENABLED(TestOn)); + KATANA_LOG_ASSERT(!KATANA_EXPERIMENTAL_ENABLED(TestOff)); + KATANA_LOG_ASSERT(KATANA_EXPERIMENTAL_ENABLED(TestSecond)); + + auto unused_in_env = + katana::internal::ExperimentalFeature::ReportUnrecognized(); + KATANA_LOG_ASSERT(unused_in_env.size() == 1); + KATANA_LOG_ASSERT(unused_in_env[0] == "EnvironmentOnly"); + + auto enabled = katana::internal::ExperimentalFeature::ReportEnabled(); + KATANA_LOG_ASSERT(enabled.size() == 3); + std::sort(enabled.begin(), enabled.end()); + KATANA_LOG_ASSERT(enabled[0] == "DefinedButUnused"); + KATANA_LOG_ASSERT(enabled[1] == "TestOn"); + KATANA_LOG_ASSERT(enabled[2] == "TestSecond"); + + auto disabled = katana::internal::ExperimentalFeature::ReportDisabled(); + KATANA_LOG_ASSERT(disabled.size() == 1); + KATANA_LOG_ASSERT(disabled[0] == "TestOff"); +}