From e61dcc6d1c11573b3631e7d184e43c60c1e6a0b2 Mon Sep 17 00:00:00 2001 From: tovinkere Date: Sun, 5 Sep 2021 06:44:59 -0700 Subject: [PATCH] [XPTI] Add new class xpti::framework::tracepoint_t (#4462) + Supports the creation of universal IDs and posting them to TLS storage. Public entry points will need to use the new approach to propagate Universal IDs through multiple layers of the SW Stack. + Added tests to ensure the correctness of new APIs to support this feature and the functionality of the tracepoint object. + New methods added to the spec: - xptiRegisterPayload - xptiQueryPayloadByUID Signed-off-by: Vasanth Tovinkere --- xpti/include/xpti_data_types.h | 34 ++++-- xpti/include/xpti_trace_framework.h | 21 ++++ xpti/include/xpti_trace_framework.hpp | 115 ++++++++++++++++++++ xpti/src/xpti_proxy.cpp | 24 ++++ xptifw/src/xpti_trace_framework.cpp | 49 ++++++++- xptifw/unit_test/xpti_api_tests.cpp | 30 +++++ xptifw/unit_test/xpti_correctness_tests.cpp | 43 ++++++++ 7 files changed, 306 insertions(+), 10 deletions(-) diff --git a/xpti/include/xpti_data_types.h b/xpti/include/xpti_data_types.h index 7e81b84ebb301..5dfed90dd23c0 100644 --- a/xpti/include/xpti_data_types.h +++ b/xpti/include/xpti_data_types.h @@ -92,6 +92,8 @@ enum class payload_flag_t { ColumnInfoAvailable = 1 << 4, /// Caller/Callee stack trace available when source/kernel info not available StackTraceAvailable = 1 << 5, + /// Payload has been registered with the framework + PayloadRegistered = 1 << 15, // A 64-bit hash is already available for this payload HashAvailable = 2 << 16 }; @@ -172,7 +174,9 @@ struct payload_t { source_file = nullptr; ///< Invalid source file string pointer line_no = invalid_id; ///< Invalid line number column_no = invalid_id; ///< Invalid column number - flags = (uint64_t)payload_flag_t::CodePointerAvailable; + if (codeptr) { + flags = (uint64_t)payload_flag_t::CodePointerAvailable; + } } // If neither an address or the fully identifyable source file name and @@ -184,15 +188,21 @@ struct payload_t { code_ptr_va = nullptr; name = func_name; ///< Invalid name string pointer source_file = nullptr; ///< Invalid source file string pointer - flags = (uint64_t)(payload_flag_t::NameAvailable); + if (func_name) { + flags = (uint64_t)(payload_flag_t::NameAvailable); + } } payload_t(const char *func_name, void *codeptr) { code_ptr_va = codeptr; name = func_name; ///< Invalid name string pointer source_file = nullptr; ///< Invalid source file string pointer - flags = (uint64_t)payload_flag_t::NameAvailable | - (uint64_t)payload_flag_t::CodePointerAvailable; + if (func_name) { + flags = (uint64_t)(payload_flag_t::NameAvailable); + } + if (codeptr) { + flags |= (uint64_t)payload_flag_t::CodePointerAvailable; + } } // When the end user opts out of preserving the code location information and @@ -228,11 +238,17 @@ struct payload_t { source_file = sf; line_no = line; column_no = col; - flags = (uint64_t)payload_flag_t::NameAvailable | - (uint64_t)payload_flag_t::SourceFileAvailable | - (uint64_t)payload_flag_t::LineInfoAvailable | - (uint64_t)payload_flag_t::ColumnInfoAvailable | - (uint64_t)payload_flag_t::CodePointerAvailable; + if (kname) { + flags = (uint64_t)payload_flag_t::NameAvailable; + } + if (sf) { + flags |= (uint64_t)payload_flag_t::SourceFileAvailable | + (uint64_t)payload_flag_t::LineInfoAvailable | + (uint64_t)payload_flag_t::ColumnInfoAvailable; + } + if (codeptr) { + flags |= (uint64_t)payload_flag_t::CodePointerAvailable; + } } int32_t name_sid() const { return (int32_t)(uid.p2 & 0x00000000ffffffff); } diff --git a/xpti/include/xpti_trace_framework.h b/xpti/include/xpti_trace_framework.h index f382685dde58b..e6c5cdc72818c 100644 --- a/xpti/include/xpti_trace_framework.h +++ b/xpti/include/xpti_trace_framework.h @@ -96,6 +96,17 @@ XPTI_EXPORT_API xpti::string_id_t xptiRegisterString(const char *string, /// @return A reference to the string identified by the string ID. XPTI_EXPORT_API const char *xptiLookupString(xpti::string_id_t id); +/// @brief Register a payload with the framework +/// @details Since a payload may contain multiple strings that may have been +/// defined on the stack, it is recommended the payload object is registered +/// with the system as soon as possible. The framework will register all the +/// strings in the payload in the string table and replace the pointers to +/// strings on the stack with the pointers from the string table that should be +/// valid for the lifetime of the application. +/// @param payload The payload object that is registered with the system. +/// @return The unique hash value for the payload. +XPTI_EXPORT_API uint64_t xptiRegisterPayload(xpti::payload_t *payload); + /// @brief Register a stream by its name and get a stream ID /// @details When events in a given stream have to be notified to the /// subscribers, the stream ID to which the events belong to is required. This @@ -252,6 +263,14 @@ XPTI_EXPORT_API const xpti::trace_event_data_t *xptiFindEvent(uint64_t uid); XPTI_EXPORT_API const xpti::payload_t * xptiQueryPayload(xpti::trace_event_data_t *lookup_object); +/// @brief Retrieves the payload information associated with an universal ID +/// @details An universal ID references the unique payload it represents and +/// this function allows you to query the payload with the universal ID. +/// +/// @param uid The universal ID for which the payload is to be retrieved. +/// @return The payload data structure pointer for the event. +XPTI_EXPORT_API const xpti::payload_t *xptiQueryPayloadByUID(uint64_t uid); + /// @brief Registers a callback for a trace point type /// @details Subscribers receive notifications to the trace point types they /// register a callback with. This function allows subscribers to register the @@ -400,6 +419,7 @@ typedef void (*xpti_finalize_t)(const char *); typedef uint64_t (*xpti_get_unique_id_t)(); typedef xpti::string_id_t (*xpti_register_string_t)(const char *, char **); typedef const char *(*xpti_lookup_string_t)(xpti::string_id_t); +typedef uint64_t (*xpti_register_payload_t)(xpti::payload_t *); typedef uint8_t (*xpti_register_stream_t)(const char *); typedef xpti::result_t (*xpti_unregister_stream_t)(const char *); typedef uint16_t (*xpti_register_user_defined_tp_t)(const char *, uint8_t); @@ -410,6 +430,7 @@ typedef xpti::trace_event_data_t *(*xpti_make_event_t)( typedef const xpti::trace_event_data_t *(*xpti_find_event_t)(int64_t); typedef const xpti::payload_t *(*xpti_query_payload_t)( xpti::trace_event_data_t *); +typedef const xpti::payload_t *(*xpti_query_payload_by_uid_t)(uint64_t uid); typedef xpti::result_t (*xpti_register_cb_t)(uint8_t, uint16_t, xpti::tracepoint_callback_api_t); typedef xpti::result_t (*xpti_unregister_cb_t)(uint8_t, uint16_t, diff --git a/xpti/include/xpti_trace_framework.hpp b/xpti/include/xpti_trace_framework.hpp index 8c4aa058ff2d0..24bf75305e513 100644 --- a/xpti/include/xpti_trace_framework.hpp +++ b/xpti/include/xpti_trace_framework.hpp @@ -7,7 +7,9 @@ // #pragma once #include +#include #include +#include #include "xpti_data_types.h" #include "xpti_trace_framework.h" @@ -269,6 +271,7 @@ class PlatformHelper { } // namespace utils namespace framework { +static thread_local uint64_t g_tls_uid = xpti::invalid_uid; constexpr uint16_t signal = (uint16_t)xpti::trace_point_type_t::signal; constexpr uint16_t graph_create = (uint16_t)xpti::trace_point_type_t::graph_create; @@ -317,5 +320,117 @@ class scoped_notify { const void *m_user_data; uint64_t m_instance; }; + +// --------------- Commented section of the code ------------- +// +// github.com/bombela/backward-cpp/blob/master/backward.hpp +// +// Need to figure out the process for considering 3rd party +// code that helps with addressing the gaps when the developer +// doesn't opt-in. +//------------------------------------------------------------ +// #include "backward.hpp" +// class backtrace_t { +// public: +// backtrace_t(int levels = 2) { +// m_st.load_here(levels); +// m_tr.load_stacktrace(m_st); +// m_parent = m_tr.resolve(m_st[1]); +// m_curr = m_tr.resolve(m_st[0]); +// if(m_parent.source.filename) { +// m_payload = xpti::payload_t(m_curr.source.function, +// m_parent.source.filename, m_parent.source.line, 0, m_curr.addr); +// } +// else { +// m_packed_string = m_parent.source.function + std::string("::") + +// m_curr.source.function; m_payload = +// xpti::payload_t(m_curr.source.function, m_packed_string.c_str(), +// m_curr.addr); +// } +// } +// +// xpti::payload_t *payload() { return &m_payload;} +// private: +// backward::StackTrace m_st; +// backward::TraceResolver m_tr; +// backward::ResolvedTrace m_curr, m_parent; +// std::string m_packed_string; +// xpti::payload_t m_payload; +// }; + +/// @brief Tracepoint data type allows the construction of Universal ID +/// @details The tracepoint data type builds on the payload data type by +/// combining the functionality of payload and xpti::makeEvent() to create the +/// unique Universal ID and stash it in the TLS for use by downstream layers in +/// the SW stack. +/// +/// Usage:- +/// #ifdef XPTI_TRACE_ENABLED +/// xpti::payload_t p, *payload = &p; +/// #ifdef SYCL_TOOL_PROFILE +/// // sycl::detail::code_location cLoc = +/// // sycl::detail::code_location::current(); +/// if(cLoc.valid()) +/// p = xpti::payload_t(cLoc.functionname(), cLoc.fileName(), +/// cLoc.lineNumber(), cLoc.columnNumber(), codeptr); +/// else +/// p = xpti::payload_t(KernelInfo.funcName(), KernelInfo.sourceFileName(), +/// KernelInfo.lineNo(), KernelInfor.columnNo(), codeptr); +/// #else +/// xpti::framework::backtrace_t b; +/// payload = b.payload(); +/// #endif +/// xpti::tracepoint_t t(payload); +/// #endif +/// +/// See also: xptiTracePointTest in xpti_correctness_tests.cpp +class tracepoint_t { +public: + // Constructor that makes calls to xpti API layer to register strings and + // create the Universal ID that is stored in the TLS entry for lookup + tracepoint_t(xpti::payload_t *p) : m_payload(nullptr), m_top(false) { + if (p) { + // We expect the payload input has been populated with the information + // available at that time + uint64_t uid = g_tls_uid; + if (uid != xpti::invalid_uid) { + // We already have a parent SW layer that has a tracepoint defined + m_payload = xptiQueryPayloadByUID(uid); + } else { + m_top = true; + uid = xptiRegisterPayload(p); + if (uid != xpti::invalid_uid) { + g_tls_uid = uid; + m_payload = xptiQueryPayloadByUID(uid); + } + } + } + } + ~tracepoint_t() { + if (m_top) { + g_tls_uid = xpti::invalid_uid; + } + } + + // The payload object that is returned will have the UID object populated and + // can be looked up in the xpti lookup APIs or be used to make an event. + const payload_t *payload() { return m_payload; } + + uint64_t universal_id() { + if (m_payload && + (m_payload->flags & + static_cast(xpti::payload_flag_t::HashAvailable))) { + return m_payload->internal; + } else { + return xpti::invalid_uid; + } + } + +private: + /// The payload data structure that is prepared from code_location(), + /// caller_callee string or kernel name/codepointer based on the opt-in flag. + const payload_t *m_payload; + bool m_top; +}; } // namespace framework } // namespace xpti diff --git a/xpti/src/xpti_proxy.cpp b/xpti/src/xpti_proxy.cpp index 3c9c18e9ae89c..4d04110d98b2c 100644 --- a/xpti/src/xpti_proxy.cpp +++ b/xpti/src/xpti_proxy.cpp @@ -31,6 +31,8 @@ enum functions_t { XPTI_ADD_METADATA, XPTI_QUERY_METADATA, XPTI_TRACE_ENABLED, + XPTI_REGISTER_PAYLOAD, + XPTI_QUERY_PAYLOAD_BY_UID, // All additional functions need to appear before // the XPTI_FW_API_COUNT enum @@ -45,6 +47,7 @@ class ProxyLoader { {XPTI_GET_UNIQUE_ID, "xptiGetUniqueId"}, {XPTI_REGISTER_STRING, "xptiRegisterString"}, {XPTI_LOOKUP_STRING, "xptiLookupString"}, + {XPTI_REGISTER_PAYLOAD, "xptiRegisterPayload"}, {XPTI_REGISTER_STREAM, "xptiRegisterStream"}, {XPTI_UNREGISTER_STREAM, "xptiUnregisterStream"}, {XPTI_REGISTER_USER_DEFINED_TP, "xptiRegisterUserDefinedTracePoint"}, @@ -52,6 +55,7 @@ class ProxyLoader { {XPTI_MAKE_EVENT, "xptiMakeEvent"}, {XPTI_FIND_EVENT, "xptiFindEvent"}, {XPTI_QUERY_PAYLOAD, "xptiQueryPayload"}, + {XPTI_QUERY_PAYLOAD_BY_UID, "xptiQueryPayloadByUID"}, {XPTI_REGISTER_CALLBACK, "xptiRegisterCallback"}, {XPTI_UNREGISTER_CALLBACK, "xptiUnregisterCallback"}, {XPTI_NOTIFY_SUBSCRIBERS, "xptiNotifySubscribers"}, @@ -203,6 +207,16 @@ XPTI_EXPORT_API const char *xptiLookupString(xpti::string_id_t id) { return nullptr; } +XPTI_EXPORT_API uint64_t xptiRegisterPayload(xpti::payload_t *payload) { + if (xpti::g_loader.noErrors()) { + auto f = xpti::g_loader.functionByIndex(XPTI_REGISTER_PAYLOAD); + if (f) { + return (*(xpti_register_payload_t)f)(payload); + } + } + return xpti::invalid_uid; +} + XPTI_EXPORT_API uint8_t xptiRegisterStream(const char *stream_name) { if (xpti::g_loader.noErrors()) { auto f = xpti::g_loader.functionByIndex(XPTI_REGISTER_STREAM); @@ -256,6 +270,16 @@ xptiQueryPayload(xpti::trace_event_data_t *lookup_object) { return nullptr; } +XPTI_EXPORT_API const xpti::payload_t *xptiQueryPayloadByUID(uint64_t uid) { + if (xpti::g_loader.noErrors()) { + auto f = xpti::g_loader.functionByIndex(XPTI_QUERY_PAYLOAD_BY_UID); + if (f) { + return (*(xpti_query_payload_by_uid_t)f)(uid); + } + } + return nullptr; +} + XPTI_EXPORT_API xpti::result_t xptiRegisterCallback(uint8_t stream_id, uint16_t trace_type, xpti::tracepoint_callback_api_t cb) { diff --git a/xptifw/src/xpti_trace_framework.cpp b/xptifw/src/xpti_trace_framework.cpp index 9f7de9d857354..efa59b28d5cbc 100644 --- a/xptifw/src/xpti_trace_framework.cpp +++ b/xptifw/src/xpti_trace_framework.cpp @@ -319,6 +319,17 @@ class Tracepoints { } } + const xpti::payload_t *payloadDataByUID(uint64_t uid) { + if (uid == xpti::invalid_uid) + return nullptr; + // Scoped lock until the information is retrieved from the map + { + std::lock_guard Lock(MEventMutex); + // Cache it in case it is not already cached + return &MPayloads[uid]; + } + } + const xpti::trace_event_data_t *eventData(uint64_t UId) { if (UId == xpti::invalid_uid) return nullptr; @@ -373,7 +384,21 @@ class Tracepoints { #endif } -private: + uint64_t registerPayload(xpti::payload_t *Payload) { + auto HashValue = makeHash(Payload); + if (HashValue == xpti::invalid_uid) + return xpti::invalid_uid; + + std::lock_guard Lock(MEventMutex); + // We also want to query the payload by universal ID that has been + // generated + auto &CurrentPayload = MPayloads[HashValue]; + Payload->flags |= (uint64_t)payload_flag_t::PayloadRegistered; + CurrentPayload = *Payload; // when it uses tbb, should be thread-safe + + return HashValue; + } + /// Goals: To create a hash value from payload /// 1. Check the payload structure to see if it is valid. If valid, then /// check to see if any strings are provided and add them to the string @@ -430,6 +455,7 @@ class Tracepoints { return HashValue; } +private: // Register the payload and generate a universal ID for it. // Once registered, the payload is accessible through the // Universal ID that corresponds to the payload. @@ -476,6 +502,8 @@ class Tracepoints { // generated auto &CurrentPayload = MPayloads[HashValue]; CurrentPayload = TempPayload; // when it uses tbb, should be thread-safe + CurrentPayload.flags |= (uint64_t)payload_flag_t::PayloadRegistered; + xpti::trace_event_data_t *Event = &MEvents[HashValue]; // We are seeing this unique ID for the first time, so we will // initialize the event structure with defaults and set the unique_id to @@ -872,6 +900,13 @@ class Framework { return MStringTableRef.query(ID); } + uint64_t registerPayload(xpti::payload_t *payload) { + if (!payload) + return xpti::invalid_id; + + return MTracepoints.registerPayload(payload); + } + xpti::result_t registerCallback(uint8_t StreamID, uint16_t TraceType, xpti::tracepoint_callback_api_t cbFunc) { return MNotifier.registerCallback(StreamID, TraceType, cbFunc); @@ -923,6 +958,10 @@ class Framework { return MTracepoints.payloadData(Event); } + const xpti::payload_t *queryPayloadByUID(uint64_t uid) { + return MTracepoints.payloadDataByUID(uid); + } + void printStatistics() { MNotifier.printStatistics(); MStringTableRef.printStatistics(); @@ -992,6 +1031,10 @@ XPTI_EXPORT_API const char *xptiLookupString(xpti::string_id_t ID) { return xpti::GXPTIFramework.lookupString(ID); } +XPTI_EXPORT_API uint64_t xptiRegisterPayload(xpti::payload_t *payload) { + return xpti::GXPTIFramework.registerPayload(payload); +} + XPTI_EXPORT_API uint8_t xptiRegisterStream(const char *StreamName) { return xpti::GXPTIFramework.registerStream(StreamName); } @@ -1016,6 +1059,10 @@ xptiQueryPayload(xpti::trace_event_data_t *LookupObject) { return xpti::GXPTIFramework.queryPayload(LookupObject); } +XPTI_EXPORT_API const xpti::payload_t *xptiQueryPayloadByUID(uint64_t uid) { + return xpti::GXPTIFramework.queryPayloadByUID(uid); +} + XPTI_EXPORT_API xpti::result_t xptiRegisterCallback(uint8_t StreamID, uint16_t TraceType, xpti::tracepoint_callback_api_t cbFunc) { diff --git a/xptifw/unit_test/xpti_api_tests.cpp b/xptifw/unit_test/xpti_api_tests.cpp index 9877f5beb467d..01041163e578a 100644 --- a/xptifw/unit_test/xpti_api_tests.cpp +++ b/xptifw/unit_test/xpti_api_tests.cpp @@ -8,6 +8,7 @@ #include #include #include +#include static int func_callback_update = 0; @@ -57,6 +58,22 @@ TEST(xptiApiTest, xptiLookupStringGoodInput) { EXPECT_STREQ("foo", LookUpString); } +TEST(xptiApiTest, xptiRegisterPayloadGoodInput) { + xpti::payload_t p("foo", "foo.cpp", 10, 0, (void *)0xdeadbeef); + + auto ID = xptiRegisterPayload(&p); + EXPECT_NE(ID, xpti::invalid_id); + EXPECT_EQ(p.internal, ID); + EXPECT_EQ(p.uid.hash(), ID); +} + +TEST(xptiApiTest, xptiRegisterPayloadBadInput) { + xpti::payload_t p; + + auto ID = xptiRegisterPayload(&p); + EXPECT_EQ(ID, xpti::invalid_uid); +} + TEST(xptiApiTest, xptiGetUniqueId) { std::set IDs; for (int i = 0; i < 10; ++i) { @@ -162,6 +179,19 @@ TEST(xptiApiTest, xptiQueryPayloadGoodInput) { EXPECT_EQ(Payload.line_no, NewResult->line_no); } +TEST(xptiApiTest, xptiQueryPayloadByUIDGoodInput) { + xpti::payload_t p("foo", "foo.cpp", 10, 0, (void *)0xdeadbeef); + + auto ID = xptiRegisterPayload(&p); + EXPECT_NE(ID, xpti::invalid_id); + EXPECT_EQ(p.internal, ID); + EXPECT_EQ(p.uid.hash(), ID); + + auto pp = xptiQueryPayloadByUID(ID); + EXPECT_EQ(p.internal, pp->internal); + EXPECT_EQ(p.uid.hash(), pp->uid.hash()); +} + TEST(xptiApiTest, xptiTraceEnabled) { // If no env is set, this should be false // The state is determined at app startup diff --git a/xptifw/unit_test/xpti_correctness_tests.cpp b/xptifw/unit_test/xpti_correctness_tests.cpp index 6a72c40456858..1d58e65e4c1b2 100644 --- a/xptifw/unit_test/xpti_correctness_tests.cpp +++ b/xptifw/unit_test/xpti_correctness_tests.cpp @@ -59,6 +59,49 @@ TEST(xptiCorrectnessTest, xptiRegisterString) { EXPECT_STREQ(LUTStr, TStr); } +void nestedTest(xpti::payload_t *p, std::vector &uids) { + xpti::framework::tracepoint_t t(p); + uint64_t hash = t.universal_id(); + uids.push_back(hash); + + if (uids.size() < 5) { + xpti::payload_t pp; + nestedTest(&pp, uids); + } +} + +TEST(xptiCorrectnessTest, xptiTracePointTest) { + std::vector uids; + xpti::payload_t p("foo", "foo.cpp", 10, 0, (void *)0xdeadbeef); + + auto ID = xptiRegisterPayload(&p); + + uint64_t id = xpti::invalid_uid; + nestedTest(&p, uids); + for (auto &e : uids) { + EXPECT_NE(e, xpti::invalid_uid); + if (id != xpti::invalid_uid) { + EXPECT_EQ(e, id); + id = e; + } + } + + uids.clear(); + xpti::payload_t p1("bar", "foo.cpp", 15, 0, (void *)0xdeaddead); + + ID = xptiRegisterPayload(&p1); + + id = xpti::invalid_uid; + nestedTest(&p1, uids); + for (auto &e : uids) { + EXPECT_NE(e, xpti::invalid_uid); + if (id != xpti::invalid_uid) { + EXPECT_EQ(e, id); + id = e; + } + } +} + TEST(xptiCorrectnessTest, xptiInitializeForDefaultTracePointTypes) { // We will test functionality of a subscriber // without actually creating a plugin