https://github.com/oontvoo updated
https://github.com/llvm/llvm-project/pull/128534
>From 236ab76b7c9433f8fed6586a4ee18351779cc191 Mon Sep 17 00:00:00 2001
From: Vy Nguyen <v...@google.com>
Date: Mon, 24 Feb 2025 11:32:03 -0500
Subject: [PATCH 1/3] [llvm][telemetr]Change Telemetry-disabling mechanism.
Details:
- Previously, we used the LLVM_BUILD_TELEMETRY flag to control whether any
Telemetry code will be built.
This has proven to cause more nuisance to both users of the Telemetry and any
further extension of it.
(Eg., we needed to put #ifdef around caller/user code)
- So the new approach is to:
+ Remove this flag and introduce LLVM_ENABLE_TELEMETRY which would be true by
default.
+ If LLVM_ENABLE_TELEMETRY is set to FALSE (at buildtime), the library would
still be built BUT Telemetry cannot be enabled.
And no data can be collected.
The benefit of this is that it simplifies user (and extension) code since we
just need to put the check on Config::EnableTelemetry.
Besides, the Telemetry library itself is very small, hence the additional code
to be built would not cause any difference in build performance.
---
lldb/source/Core/CMakeLists.txt | 4 +---
lldb/source/Core/Telemetry.cpp | 7 ++-----
lldb/unittests/Core/CMakeLists.txt | 6 +++---
llvm/CMakeLists.txt | 3 ++-
llvm/cmake/modules/LLVMConfig.cmake.in | 1 +
llvm/include/llvm/Config/llvm-config.h.cmake | 2 ++
llvm/include/llvm/Telemetry/Telemetry.h | 12 ++++++++++--
llvm/lib/CMakeLists.txt | 5 ++---
llvm/unittests/CMakeLists.txt | 4 +---
.../gn/secondary/llvm/include/llvm/Config/BUILD.gn | 1 +
utils/bazel/llvm_configs/llvm-config.h.cmake | 2 ++
11 files changed, 27 insertions(+), 20 deletions(-)
diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt
index 82fb5f42f9f4b..a3c12e4c1bdbc 100644
--- a/lldb/source/Core/CMakeLists.txt
+++ b/lldb/source/Core/CMakeLists.txt
@@ -16,9 +16,7 @@ if (LLDB_ENABLE_CURSES)
endif()
endif()
-if (LLVM_BUILD_TELEMETRY)
- set(TELEMETRY_DEPS Telemetry)
-endif()
+set(TELEMETRY_DEPS Telemetry)
# TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore
add_lldb_library(lldbCore
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 5222f76704f91..51a860ea5313b 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -8,8 +8,6 @@
#include "llvm/Config/llvm-config.h"
-#ifdef LLVM_BUILD_TELEMETRY
-
#include "lldb/Core/Telemetry.h"
#include "lldb/Core/Debugger.h"
#include "lldb/Utility/LLDBLog.h"
@@ -57,7 +55,8 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer)
const {
}
TelemetryManager::TelemetryManager(std::unique_ptr<Config> config)
- : m_config(std::move(config)) {}
+ : m_config(std::move(config))
+}
llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
// Do nothing for now.
@@ -75,5 +74,3 @@ void
TelemetryManager::setInstance(std::unique_ptr<TelemetryManager> manager) {
} // namespace telemetry
} // namespace lldb_private
-
-#endif // LLVM_BUILD_TELEMETRY
diff --git a/lldb/unittests/Core/CMakeLists.txt
b/lldb/unittests/Core/CMakeLists.txt
index d4d3764b67ae3..e8df299631e2e 100644
--- a/lldb/unittests/Core/CMakeLists.txt
+++ b/lldb/unittests/Core/CMakeLists.txt
@@ -1,6 +1,6 @@
-if (LLVM_BUILD_TELEMETRY)
- set(TELEMETRY_DEPS Telemetry)
-endif()
+
+set(TELEMETRY_DEPS Telemetry)
+
add_lldb_unittest(LLDBCoreTests
CommunicationTest.cpp
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 88512d0f1dd96..9188fb93b7846 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -835,7 +835,8 @@ option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm
API documentation." OF
option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF)
option (LLVM_ENABLE_OCAMLDOC "Build OCaml bindings documentation." ON)
option (LLVM_ENABLE_BINDINGS "Build bindings." ON)
-option (LLVM_BUILD_TELEMETRY "Build the telemetry library. This does not
enable telemetry." ON)
+option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]Enable
the telemetry library. If set to OFF, library cannot be enabled after build
(eg., at runtime)" ON)
+option (LLVM_ENABLE_TELEMETRY "Enable the telemetry library. If set to OFF,
library cannot be enabled after build (eg., at runtime)" ON)
set(LLVM_INSTALL_DOXYGEN_HTML_DIR "${CMAKE_INSTALL_DOCDIR}/llvm/doxygen-html"
CACHE STRING "Doxygen-generated HTML documentation install directory")
diff --git a/llvm/cmake/modules/LLVMConfig.cmake.in
b/llvm/cmake/modules/LLVMConfig.cmake.in
index 28655ee3ab87d..134d107ce79ba 100644
--- a/llvm/cmake/modules/LLVMConfig.cmake.in
+++ b/llvm/cmake/modules/LLVMConfig.cmake.in
@@ -101,6 +101,7 @@ set(LLVM_ENABLE_PIC @LLVM_ENABLE_PIC@)
set(LLVM_BUILD_32_BITS @LLVM_BUILD_32_BITS@)
set(LLVM_BUILD_TELEMETRY @LLVM_BUILD_TELEMETRY@)
+set(LLVM_EANBLE_TELEMETRY @LLVM_ENABLE_TELEMETRY@)
if (NOT "@LLVM_PTHREAD_LIB@" STREQUAL "")
set(LLVM_PTHREAD_LIB "@LLVM_PTHREAD_LIB@")
diff --git a/llvm/include/llvm/Config/llvm-config.h.cmake
b/llvm/include/llvm/Config/llvm-config.h.cmake
index 239f9dd3f38db..13db9935870f8 100644
--- a/llvm/include/llvm/Config/llvm-config.h.cmake
+++ b/llvm/include/llvm/Config/llvm-config.h.cmake
@@ -202,6 +202,8 @@
#cmakedefine LLVM_HAS_LOGF128
/* Define if building LLVM with LLVM_BUILD_TELEMETRY */
+/* DEPRECATED - use LLVM_ENABLE_TELEMETRY */
#cmakedefine LLVM_BUILD_TELEMETRY ${LLVM_BUILD_TELEMETRY}
+#cmakedefine LLVM_ENABLE_TELEMETRY ${LLVM_ENABLE_TELEMETRY}
#endif
diff --git a/llvm/include/llvm/Telemetry/Telemetry.h
b/llvm/include/llvm/Telemetry/Telemetry.h
index 42319f3ef51f2..f60c4c762ca7b 100644
--- a/llvm/include/llvm/Telemetry/Telemetry.h
+++ b/llvm/include/llvm/Telemetry/Telemetry.h
@@ -64,11 +64,19 @@ class Serializer {
/// This struct can be extended as needed to add additional configuration
/// points specific to a vendor's implementation.
struct Config {
- virtual ~Config() = default;
+#ifdef LLVM_ENABLE_TELEMETRY
+ static bool BuildTimeEnableTelemetry = true;
+#else
+ static bool BuildTimeEnableTelemetry = false;
+#endif
+ virtual ~Config() : EnableTelemetry(BuildTimeEnableTelemetry) {}
// If true, telemetry will be enabled.
const bool EnableTelemetry;
- Config(bool E) : EnableTelemetry(E) {}
+
+ // Telemetry can only be enabled if both the runtime and buildtime flag
+ // are set.
+ Config(bool E) : EnableTelemetry(E && BuildTimeEnableTelemetry) {}
virtual std::optional<std::string> makeSessionId() { return std::nullopt; }
};
diff --git a/llvm/lib/CMakeLists.txt b/llvm/lib/CMakeLists.txt
index d0a2bc9294381..37b6fcf1236e3 100644
--- a/llvm/lib/CMakeLists.txt
+++ b/llvm/lib/CMakeLists.txt
@@ -41,9 +41,8 @@ add_subdirectory(ProfileData)
add_subdirectory(Passes)
add_subdirectory(TargetParser)
add_subdirectory(TextAPI)
-if (LLVM_BUILD_TELEMETRY)
- add_subdirectory(Telemetry)
-endif()
+add_subdirectory(Telemetry)
+
add_subdirectory(ToolDrivers)
add_subdirectory(XRay)
if (LLVM_INCLUDE_TESTS)
diff --git a/llvm/unittests/CMakeLists.txt b/llvm/unittests/CMakeLists.txt
index 12e229b1c3498..81abce51b8939 100644
--- a/llvm/unittests/CMakeLists.txt
+++ b/llvm/unittests/CMakeLists.txt
@@ -63,9 +63,7 @@ add_subdirectory(Support)
add_subdirectory(TableGen)
add_subdirectory(Target)
add_subdirectory(TargetParser)
-if (LLVM_BUILD_TELEMETRY)
- add_subdirectory(Telemetry)
-endif()
+add_subdirectory(Telemetry)
add_subdirectory(Testing)
add_subdirectory(TextAPI)
add_subdirectory(Transforms)
diff --git a/llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
b/llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
index 5a13545a15b13..8f6c431a713af 100644
--- a/llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
+++ b/llvm/utils/gn/secondary/llvm/include/llvm/Config/BUILD.gn
@@ -293,6 +293,7 @@ write_cmake_config("llvm-config") {
"LLVM_BUILD_LLVM_DYLIB=",
"LLVM_BUILD_SHARED_LIBS=",
"LLVM_BUILD_TELEMETRY=",
+ "LLVM_ENABLE_TELEMETRY=",
"LLVM_DEFAULT_TARGET_TRIPLE=$llvm_target_triple",
"LLVM_ENABLE_DUMP=",
"LLVM_ENABLE_HTTPLIB=",
diff --git a/utils/bazel/llvm_configs/llvm-config.h.cmake
b/utils/bazel/llvm_configs/llvm-config.h.cmake
index 239f9dd3f38db..315ddfb37362c 100644
--- a/utils/bazel/llvm_configs/llvm-config.h.cmake
+++ b/utils/bazel/llvm_configs/llvm-config.h.cmake
@@ -202,6 +202,8 @@
#cmakedefine LLVM_HAS_LOGF128
/* Define if building LLVM with LLVM_BUILD_TELEMETRY */
+/* DEPRECATED - Use LLVM_ENABLE_TELEMETRY */
#cmakedefine LLVM_BUILD_TELEMETRY ${LLVM_BUILD_TELEMETRY}
+#cmakedefine LLVM_ENABLE_TELEMETRY ${LLVM_ENABLE_TELEMETRY}
#endif
>From 45869d7a8e18bd2334c2aab7e62adddffaf93d1d Mon Sep 17 00:00:00 2001
From: Vy Nguyen <v...@google.com>
Date: Mon, 24 Feb 2025 13:01:44 -0500
Subject: [PATCH 2/3] formatting + more checks
---
lldb/source/Core/Telemetry.cpp | 9 ++++++---
llvm/include/llvm/Telemetry/Telemetry.h | 4 ++--
llvm/lib/Telemetry/Telemetry.cpp | 2 ++
3 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 51a860ea5313b..865488928d25d 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -55,8 +55,7 @@ void LLDBBaseTelemetryInfo::serialize(Serializer &serializer)
const {
}
TelemetryManager::TelemetryManager(std::unique_ptr<Config> config)
- : m_config(std::move(config))
-}
+ : m_config(std::move(config)) {}
llvm::Error TelemetryManager::preDispatch(TelemetryInfo *entry) {
// Do nothing for now.
@@ -66,7 +65,11 @@ llvm::Error TelemetryManager::preDispatch(TelemetryInfo
*entry) {
}
std::unique_ptr<TelemetryManager> TelemetryManager::g_instance = nullptr;
-TelemetryManager *TelemetryManager::getInstance() { return g_instance.get(); }
+TelemetryManager *TelemetryManager::getInstance() {
+ if (!Config::BuildTimeEnableTelemetry)
+ return nullptr;
+ return g_instance.get();
+}
void TelemetryManager::setInstance(std::unique_ptr<TelemetryManager> manager) {
g_instance = std::move(manager);
diff --git a/llvm/include/llvm/Telemetry/Telemetry.h
b/llvm/include/llvm/Telemetry/Telemetry.h
index f60c4c762ca7b..bed3eca6fde5e 100644
--- a/llvm/include/llvm/Telemetry/Telemetry.h
+++ b/llvm/include/llvm/Telemetry/Telemetry.h
@@ -65,9 +65,9 @@ class Serializer {
/// points specific to a vendor's implementation.
struct Config {
#ifdef LLVM_ENABLE_TELEMETRY
- static bool BuildTimeEnableTelemetry = true;
+ static const bool BuildTimeEnableTelemetry = true;
#else
- static bool BuildTimeEnableTelemetry = false;
+ static const bool BuildTimeEnableTelemetry = false;
#endif
virtual ~Config() : EnableTelemetry(BuildTimeEnableTelemetry) {}
diff --git a/llvm/lib/Telemetry/Telemetry.cpp b/llvm/lib/Telemetry/Telemetry.cpp
index d86ad9c1c37bb..caac7e6c332cb 100644
--- a/llvm/lib/Telemetry/Telemetry.cpp
+++ b/llvm/lib/Telemetry/Telemetry.cpp
@@ -21,6 +21,8 @@ void TelemetryInfo::serialize(Serializer &serializer) const {
}
Error Manager::dispatch(TelemetryInfo *Entry) {
+ assert(Config::BuildTimeEnableTelemetry &&
+ "Telemetry should have been enabled");
if (Error Err = preDispatch(Entry))
return Err;
>From b199854ac2e7100eac7fea20e9464dcb9938122c Mon Sep 17 00:00:00 2001
From: Vy Nguyen <v...@google.com>
Date: Mon, 24 Feb 2025 13:10:21 -0500
Subject: [PATCH 3/3] more clean up
---
lldb/source/Core/CMakeLists.txt | 4 +---
lldb/source/Core/Telemetry.cpp | 3 ---
lldb/unittests/Core/CMakeLists.txt | 5 +----
lldb/unittests/Core/TelemetryTest.cpp | 7 -------
llvm/CMakeLists.txt | 2 +-
llvm/include/llvm/Config/llvm-config.h.cmake | 2 ++
utils/bazel/llvm_configs/llvm-config.h.cmake | 2 ++
7 files changed, 7 insertions(+), 18 deletions(-)
diff --git a/lldb/source/Core/CMakeLists.txt b/lldb/source/Core/CMakeLists.txt
index a3c12e4c1bdbc..d5d8a9d5088fc 100644
--- a/lldb/source/Core/CMakeLists.txt
+++ b/lldb/source/Core/CMakeLists.txt
@@ -16,8 +16,6 @@ if (LLDB_ENABLE_CURSES)
endif()
endif()
-set(TELEMETRY_DEPS Telemetry)
-
# TODO: Add property `NO_PLUGIN_DEPENDENCIES` to lldbCore
add_lldb_library(lldbCore
Address.cpp
@@ -82,7 +80,7 @@ add_lldb_library(lldbCore
Support
Demangle
TargetParser
- ${TELEMETRY_DEPS}
+ Telemetry
)
add_dependencies(lldbCore
diff --git a/lldb/source/Core/Telemetry.cpp b/lldb/source/Core/Telemetry.cpp
index 865488928d25d..428bdbbdfb602 100644
--- a/lldb/source/Core/Telemetry.cpp
+++ b/lldb/source/Core/Telemetry.cpp
@@ -5,9 +5,6 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
-
-#include "llvm/Config/llvm-config.h"
-
#include "lldb/Core/Telemetry.h"
#include "lldb/Core/Debugger.h"
#include "lldb/Utility/LLDBLog.h"
diff --git a/lldb/unittests/Core/CMakeLists.txt
b/lldb/unittests/Core/CMakeLists.txt
index e8df299631e2e..60265f794b5e8 100644
--- a/lldb/unittests/Core/CMakeLists.txt
+++ b/lldb/unittests/Core/CMakeLists.txt
@@ -1,7 +1,4 @@
-set(TELEMETRY_DEPS Telemetry)
-
-
add_lldb_unittest(LLDBCoreTests
CommunicationTest.cpp
DiagnosticEventTest.cpp
@@ -31,5 +28,5 @@ add_lldb_unittest(LLDBCoreTests
LLVMTestingSupport
LINK_COMPONENTS
Support
- ${TELEMETRY_DEPS}
+ Telemetry
)
diff --git a/lldb/unittests/Core/TelemetryTest.cpp
b/lldb/unittests/Core/TelemetryTest.cpp
index 0f2eaccb21a2c..caae31a42641f 100644
--- a/lldb/unittests/Core/TelemetryTest.cpp
+++ b/lldb/unittests/Core/TelemetryTest.cpp
@@ -5,11 +5,6 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
-
-#include "llvm/Config/llvm-config.h"
-
-#ifdef LLVM_BUILD_TELEMETRY
-
#include "lldb/Core/PluginInterface.h"
#include "lldb/Core/PluginManager.h"
#include "lldb/Core/Telemetry.h"
@@ -94,5 +89,3 @@ TEST(TelemetryTest, PluginTest) {
ASSERT_EQ("FakeTelemetryPlugin", ins->GetInstanceName());
}
-
-#endif // LLVM_BUILD_TELEMETRY
diff --git a/llvm/CMakeLists.txt b/llvm/CMakeLists.txt
index 9188fb93b7846..30864beb29e7d 100644
--- a/llvm/CMakeLists.txt
+++ b/llvm/CMakeLists.txt
@@ -835,7 +835,7 @@ option (LLVM_ENABLE_DOXYGEN "Use doxygen to generate llvm
API documentation." OF
option (LLVM_ENABLE_SPHINX "Use Sphinx to generate llvm documentation." OFF)
option (LLVM_ENABLE_OCAMLDOC "Build OCaml bindings documentation." ON)
option (LLVM_ENABLE_BINDINGS "Build bindings." ON)
-option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]Enable
the telemetry library. If set to OFF, library cannot be enabled after build
(eg., at runtime)" ON)
+option (LLVM_BUILD_TELEMETRY "[DEPRECATED - use LLVM_ENABLE_TELEMTRY]" ON)
option (LLVM_ENABLE_TELEMETRY "Enable the telemetry library. If set to OFF,
library cannot be enabled after build (eg., at runtime)" ON)
set(LLVM_INSTALL_DOXYGEN_HTML_DIR "${CMAKE_INSTALL_DOCDIR}/llvm/doxygen-html"
diff --git a/llvm/include/llvm/Config/llvm-config.h.cmake
b/llvm/include/llvm/Config/llvm-config.h.cmake
index 13db9935870f8..b184ef9cf26af 100644
--- a/llvm/include/llvm/Config/llvm-config.h.cmake
+++ b/llvm/include/llvm/Config/llvm-config.h.cmake
@@ -204,6 +204,8 @@
/* Define if building LLVM with LLVM_BUILD_TELEMETRY */
/* DEPRECATED - use LLVM_ENABLE_TELEMETRY */
#cmakedefine LLVM_BUILD_TELEMETRY ${LLVM_BUILD_TELEMETRY}
+
+/* Define if building LLVM with LLVM_ENABLE_TELEMETRY */
#cmakedefine LLVM_ENABLE_TELEMETRY ${LLVM_ENABLE_TELEMETRY}
#endif
diff --git a/utils/bazel/llvm_configs/llvm-config.h.cmake
b/utils/bazel/llvm_configs/llvm-config.h.cmake
index 315ddfb37362c..30bbf47b3d342 100644
--- a/utils/bazel/llvm_configs/llvm-config.h.cmake
+++ b/utils/bazel/llvm_configs/llvm-config.h.cmake
@@ -204,6 +204,8 @@
/* Define if building LLVM with LLVM_BUILD_TELEMETRY */
/* DEPRECATED - Use LLVM_ENABLE_TELEMETRY */
#cmakedefine LLVM_BUILD_TELEMETRY ${LLVM_BUILD_TELEMETRY}
+
+/* Define if building LLVM with LLVM_ENABLE_TELEMETRY */
#cmakedefine LLVM_ENABLE_TELEMETRY ${LLVM_ENABLE_TELEMETRY}
#endif
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
