From 4f7443654d64201f1a6c35dc89b9b1ac592b63e1 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Mon, 15 Nov 2021 13:39:17 +0100 Subject: [PATCH 01/11] some testcases ... --- test/cases/numpy_dot_large.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 test/cases/numpy_dot_large.py diff --git a/test/cases/numpy_dot_large.py b/test/cases/numpy_dot_large.py new file mode 100644 index 0000000..16c6790 --- /dev/null +++ b/test/cases/numpy_dot_large.py @@ -0,0 +1,19 @@ +import numpy +import numpy.linalg +import scorep.instrumenter + +with scorep.instrumenter.enable(): + a = [] + b = [] + + for i in range(1000): + a.append([]) + b.append([]) + for j in range(1000): + a[i].append(i * j) + b[i].append(i * j) + + c = numpy.dot(a, b) + c = numpy.matmul(a, c) + q, r = numpy.linalg.qr(c) + print(q, r) From 558a575298f95a2a63771b46de71591c9b163bda Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 4 May 2022 16:09:32 +0200 Subject: [PATCH 02/11] add caller line --- src/scorepy/cInstrumenter.cpp | 4 ++++ src/scorepy/events.cpp | 12 ++++++++++++ src/scorepy/events.hpp | 3 +++ 3 files changed, 19 insertions(+) diff --git a/src/scorepy/cInstrumenter.cpp b/src/scorepy/cInstrumenter.cpp index 5dd0ba7..7b0fba5 100644 --- a/src/scorepy/cInstrumenter.cpp +++ b/src/scorepy/cInstrumenter.cpp @@ -127,6 +127,10 @@ bool CInstrumenter::on_event(PyFrameObject& frame, int what, PyObject*) region_begin(name, module_name, file_name, line_number, code); } } + if (frame.f_back != nullptr) + { + region_add_caller_line(frame.f_back.f_lineno); + } break; } case PyTrace_RETURN: diff --git a/src/scorepy/events.cpp b/src/scorepy/events.cpp index 7a25405..3c592bc 100644 --- a/src/scorepy/events.cpp +++ b/src/scorepy/events.cpp @@ -127,6 +127,18 @@ void region_end_error_handling(const std::string& region_name) } } +void region_add_caller(std::string value) +{ + static SCOREP_User_ParameterHandle caller = SCOREP_USER_INVALID_PARAMETER; + SCOREP_User_ParameterString(&caller, "caller", value.c_str()); +} + +void region_add_caller_line(int64_t value) +{ + static SCOREP_User_ParameterHandle caller_line = SCOREP_USER_INVALID_PARAMETER; + SCOREP_User_ParameterInt64(&caller_line, "Called from line", value); +} + void rewind_begin(std::string region_name, std::string file_name, std::uint64_t line_number) { auto pair = rewind_regions.emplace(make_pair(region_name, region_handle())); diff --git a/src/scorepy/events.hpp b/src/scorepy/events.hpp index b3e56ea..6596797 100644 --- a/src/scorepy/events.hpp +++ b/src/scorepy/events.hpp @@ -86,6 +86,9 @@ void region_end(std::string_view& function_name, std::string_view& module, compat::PyCodeObject* identifier); void region_end(std::string_view& function_name, std::string_view& module); +void region_add_caller(std::string value); +void region_add_caller_line(uint64_t value); + void region_end_error_handling(const std::string& region_name); void rewind_begin(std::string region_name, std::string file_name, std::uint64_t line_number); From 22154cad50c348431aa944bbce7d0eeced7b8d1f Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 4 May 2022 16:15:57 +0200 Subject: [PATCH 03/11] pointers -.- --- src/scorepy/cInstrumenter.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scorepy/cInstrumenter.cpp b/src/scorepy/cInstrumenter.cpp index 7b0fba5..cdd649a 100644 --- a/src/scorepy/cInstrumenter.cpp +++ b/src/scorepy/cInstrumenter.cpp @@ -129,7 +129,7 @@ bool CInstrumenter::on_event(PyFrameObject& frame, int what, PyObject*) } if (frame.f_back != nullptr) { - region_add_caller_line(frame.f_back.f_lineno); + region_add_caller_line(frame.f_back->f_lineno); } break; } From c582e86cf5e60b01f04dd39aa3cb3a31830b1122 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 4 May 2022 21:24:47 +0200 Subject: [PATCH 04/11] remove ref and static from make_region_name We are trying to avoid string handling as much as possible these days. So this function shouldn't be called often, and we can remove this optimisation. --- src/scorepy/events.hpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/scorepy/events.hpp b/src/scorepy/events.hpp index 6596797..167f25e 100644 --- a/src/scorepy/events.hpp +++ b/src/scorepy/events.hpp @@ -30,10 +30,9 @@ struct region_handle constexpr region_handle uninitialised_region_handle = region_handle(); /// Combine the arguments into a region name -/// Return value is a statically allocated string to avoid memory (re)allocations -inline const std::string& make_region_name(std::string_view& module_name, std::string_view& name) +inline std::string make_region_name(std::string_view& module_name, std::string_view& name) { - static std::string region; + std::string region; region = module_name; region += ":"; region += name; From b5ca4d13384460ed0c44b3043945d7c0a56fabec Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 4 May 2022 21:28:42 +0200 Subject: [PATCH 05/11] Proper handling of Callers While user parameters seems to be the best fit for adding caller information, it does not seem the optimal solution, specialy as there is no linking to the sourcode in Vampir. --- src/scorepy/cInstrumenter.cpp | 15 ++++++++++++++- src/scorepy/events.cpp | 22 ++++++++++++++++++++- src/scorepy/events.hpp | 36 +++++++++++++++++++++++++++++++++-- 3 files changed, 69 insertions(+), 4 deletions(-) diff --git a/src/scorepy/cInstrumenter.cpp b/src/scorepy/cInstrumenter.cpp index cdd649a..55ba4fb 100644 --- a/src/scorepy/cInstrumenter.cpp +++ b/src/scorepy/cInstrumenter.cpp @@ -129,7 +129,20 @@ bool CInstrumenter::on_event(PyFrameObject& frame, int what, PyObject*) } if (frame.f_back != nullptr) { - region_add_caller_line(frame.f_back->f_lineno); + PyCodeObject* caller_code = frame.f_back->f_code; + + bool success = try_add_caller(caller_code, frame.f_back->f_lineno); + if (!success) + { + std::string_view name = compat::get_string_as_utf_8(caller_code->co_name); + std::string_view module_name = get_module_name(*frame.f_back); + if (name.compare("_unsetprofile") != 0 && module_name.compare(0, 6, "scorep") != 0) + { + const int line_number = frame.f_back->f_lineno; + const std::string file_name = get_file_name(*frame.f_back); + region_add_caller(name, module_name, file_name, line_number, caller_code); + } + } } break; } diff --git a/src/scorepy/events.cpp b/src/scorepy/events.cpp index 3c592bc..6ca5a9e 100644 --- a/src/scorepy/events.cpp +++ b/src/scorepy/events.cpp @@ -11,6 +11,7 @@ namespace scorepy { std::unordered_map regions; +std::unordered_map callers; static std::unordered_map user_regions; static std::unordered_map rewind_regions; @@ -36,7 +37,7 @@ void region_begin(std::string_view& function_name, std::string_view& module, if (region == uninitialised_region_handle) { - auto& region_name = make_region_name(module, function_name); + auto region_name = make_region_name(module, function_name); SCOREP_User_RegionInit(®ion.value, NULL, NULL, region_name.c_str(), SCOREP_USER_REGION_TYPE_FUNCTION, file_name.c_str(), line_number); @@ -133,6 +134,25 @@ void region_add_caller(std::string value) SCOREP_User_ParameterString(&caller, "caller", value.c_str()); } +void region_add_caller(std::string_view& function_name, std::string_view& module, + const std::string& file_name, const std::uint64_t line_number, + compat::PyCodeObject* identifier) + +{ + caller_handle& caller = callers[identifier]; + + if (caller == uninitialised_caller_handle) + { + // caller is only needed on first registration. + auto caller_region_name = std::string("Caller: ") + make_region_name(module, function_name); + SCOREP_User_ParameterInt64(&caller.value, caller_region_name.c_str(), line_number); + } + else + { + SCOREP_User_ParameterInt64(&caller.value, "", line_number); + } +} + void region_add_caller_line(int64_t value) { static SCOREP_User_ParameterHandle caller_line = SCOREP_USER_INVALID_PARAMETER; diff --git a/src/scorepy/events.hpp b/src/scorepy/events.hpp index 167f25e..1bbc8bb 100644 --- a/src/scorepy/events.hpp +++ b/src/scorepy/events.hpp @@ -27,7 +27,24 @@ struct region_handle SCOREP_User_RegionHandle value = SCOREP_USER_INVALID_REGION; }; +struct caller_handle +{ + constexpr caller_handle() = default; + ~caller_handle() = default; + constexpr bool operator==(const caller_handle& other) + { + return this->value == other.value; + } + constexpr bool operator!=(const caller_handle& other) + { + return this->value != other.value; + } + + SCOREP_User_ParameterHandle value = SCOREP_USER_INVALID_PARAMETER; +}; + constexpr region_handle uninitialised_region_handle = region_handle(); +constexpr caller_handle uninitialised_caller_handle = caller_handle(); /// Combine the arguments into a region name inline std::string make_region_name(std::string_view& module_name, std::string_view& name) @@ -40,6 +57,7 @@ inline std::string make_region_name(std::string_view& module_name, std::string_v } extern std::unordered_map regions; +extern std::unordered_map callers; /** tries to enter a region. Return true on success * @@ -85,8 +103,22 @@ void region_end(std::string_view& function_name, std::string_view& module, compat::PyCodeObject* identifier); void region_end(std::string_view& function_name, std::string_view& module); -void region_add_caller(std::string value); -void region_add_caller_line(uint64_t value); +inline bool try_add_caller(compat::PyCodeObject* identifier, const std::uint64_t line_number) +{ + auto it = callers.find(identifier); + if (it != callers.end()) + { + SCOREP_User_ParameterInt64(&it->second.value, "", line_number); + return true; + } + else + { + return false; + } +} +void region_add_caller(std::string_view& function_name, std::string_view& module, + const std::string& file_name, const std::uint64_t line_number, + compat::PyCodeObject* identifier); void region_end_error_handling(const std::string& region_name); From 9c69fb3da6c9a7abde49f53008025c1a9e666008 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Thu, 5 May 2022 08:24:45 +0200 Subject: [PATCH 06/11] erase caller on object destruction --- src/scorepy/events.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/scorepy/events.cpp b/src/scorepy/events.cpp index 6ca5a9e..9148958 100644 --- a/src/scorepy/events.cpp +++ b/src/scorepy/events.cpp @@ -23,6 +23,7 @@ static std::unordered_map rewind_regions; void on_dealloc(PyCodeObject* co) { regions.erase(co); + callers.erase(co); } static compat::RegisterCodeDealloc register_dealloc(on_dealloc); From 62c2e2896eb477c482e161bda52244aa307d48cc Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Fri, 20 May 2022 10:37:11 +0200 Subject: [PATCH 07/11] add callsite support --- src/scorepy/cInstrumenter.cpp | 52 ++++++++++++++++++----------- src/scorepy/events.cpp | 36 ++++++++++++++++++-- src/scorepy/events.hpp | 63 ++++++++++++++++------------------- 3 files changed, 94 insertions(+), 57 deletions(-) diff --git a/src/scorepy/cInstrumenter.cpp b/src/scorepy/cInstrumenter.cpp index 55ba4fb..bcb27aa 100644 --- a/src/scorepy/cInstrumenter.cpp +++ b/src/scorepy/cInstrumenter.cpp @@ -28,9 +28,8 @@ void CInstrumenter::deinit() void CInstrumenter::enable_instrumenter() { - const auto callback = [](PyObject* obj, PyFrameObject* frame, int what, PyObject* arg) -> int { - return from_PyObject(obj)->on_event(*frame, what, arg) ? 0 : -1; - }; + const auto callback = [](PyObject* obj, PyFrameObject* frame, int what, PyObject* arg) -> int + { return from_PyObject(obj)->on_event(*frame, what, arg) ? 0 : -1; }; if (threading_set_instrumenter) { PyRefObject result(PyObject_CallFunction(threading_set_instrumenter, "O", to_PyObject()), @@ -115,32 +114,45 @@ bool CInstrumenter::on_event(PyFrameObject& frame, int what, PyObject*) case PyTrace_CALL: { PyCodeObject* code = frame.f_code; - bool success = try_region_begin(code); - if (!success) + if (frame.f_back == nullptr) { - std::string_view name = compat::get_string_as_utf_8(code->co_name); - std::string_view module_name = get_module_name(frame); - if (name.compare("_unsetprofile") != 0 && module_name.compare(0, 6, "scorep") != 0) + bool success = try_region_begin(code); + if (!success) { - const int line_number = code->co_firstlineno; - const std::string file_name = get_file_name(frame); - region_begin(name, module_name, file_name, line_number, code); + std::string_view name = compat::get_string_as_utf_8(code->co_name); + std::string_view module_name = get_module_name(frame); + if (name.compare("_unsetprofile") != 0 && module_name.compare(0, 6, "scorep") != 0) + { + const int line_number = code->co_firstlineno; + const std::string file_name = get_file_name(frame); + region_begin(name, module_name, file_name, line_number, code); + } } } - if (frame.f_back != nullptr) + else { - PyCodeObject* caller_code = frame.f_back->f_code; - - bool success = try_add_caller(caller_code, frame.f_back->f_lineno); + PyCodeObject* callsite_code = frame.f_back->f_code; + bool success = + try_region_begin_with_callsite(code, callsite_code, frame.f_back->f_lineno); if (!success) { - std::string_view name = compat::get_string_as_utf_8(caller_code->co_name); - std::string_view module_name = get_module_name(*frame.f_back); + std::string_view name = compat::get_string_as_utf_8(code->co_name); + std::string_view module_name = get_module_name(frame); if (name.compare("_unsetprofile") != 0 && module_name.compare(0, 6, "scorep") != 0) { - const int line_number = frame.f_back->f_lineno; - const std::string file_name = get_file_name(*frame.f_back); - region_add_caller(name, module_name, file_name, line_number, caller_code); + const int line_number = code->co_firstlineno; + const std::string file_name = get_file_name(frame); + + std::string_view callsite_name = + compat::get_string_as_utf_8(callsite_code->co_name); + std::string_view callsite_module_name = get_module_name(*frame.f_back); + const int callsite_line_number_start = code->co_firstlineno; + const std::string callsite_file_name = get_file_name(frame); + + region_begin_with_callsite(name, module_name, file_name, line_number, code, + callsite_code, callsite_name, callsite_module_name, + callsite_file_name, callsite_line_number_start, + frame.f_back->f_lineno); } } } diff --git a/src/scorepy/events.cpp b/src/scorepy/events.cpp index 9148958..869b38e 100644 --- a/src/scorepy/events.cpp +++ b/src/scorepy/events.cpp @@ -11,7 +11,6 @@ namespace scorepy { std::unordered_map regions; -std::unordered_map callers; static std::unordered_map user_regions; static std::unordered_map rewind_regions; @@ -23,7 +22,6 @@ static std::unordered_map rewind_regions; void on_dealloc(PyCodeObject* co) { regions.erase(co); - callers.erase(co); } static compat::RegisterCodeDealloc register_dealloc(on_dealloc); @@ -65,6 +63,40 @@ void region_begin(std::string_view& function_name, std::string_view& module, SCOREP_User_RegionEnter(region.value); } +void region_begin_with_callsite( + std::string_view& function_name, std::string_view& module, const std::string& file_name, + const std::uint64_t line_number, compat::PyCodeObject* identifier, + compat::PyCodeObject* callsite_identifier, std::string_view& callsite_function_name, + std::string_view& callsite_module, const std::string& callsite_file_name, + const std::uint64_t callsite_line_number_start, uint32_t callsite_line) +{ + region_handle& region = regions[identifier]; + + if (region == uninitialised_region_handle) + { + auto region_name = make_region_name(module, function_name); + SCOREP_User_RegionInit(®ion.value, NULL, NULL, region_name.c_str(), + SCOREP_USER_REGION_TYPE_FUNCTION, file_name.c_str(), line_number); + + SCOREP_User_RegionSetGroup(region.value, std::string(module, 0, module.find('.')).c_str()); + } + + region_handle& callsite_region = regions[callsite_identifier]; + if (callsite_region == uninitialised_region_handle) + { + auto region_name = make_region_name(callsite_module, callsite_function_name); + SCOREP_User_RegionInit(&callsite_region.value, NULL, NULL, region_name.c_str(), + SCOREP_USER_REGION_TYPE_FUNCTION, callsite_file_name.c_str(), + callsite_line_number_start); + + SCOREP_User_RegionSetGroup( + callsite_region.value, + std::string(callsite_module, 0, callsite_module.find('.')).c_str()); + } + + SCOREP_User_RegionEnterWithCallsite(region.value, callsite_region.value, callsite_line); +} + // Used for regions, that have an identifier, aka a code object id. (instrumenter regions and // some decorated regions) void region_end(std::string_view& function_name, std::string_view& module, diff --git a/src/scorepy/events.hpp b/src/scorepy/events.hpp index 1bbc8bb..9ddb2c5 100644 --- a/src/scorepy/events.hpp +++ b/src/scorepy/events.hpp @@ -27,24 +27,7 @@ struct region_handle SCOREP_User_RegionHandle value = SCOREP_USER_INVALID_REGION; }; -struct caller_handle -{ - constexpr caller_handle() = default; - ~caller_handle() = default; - constexpr bool operator==(const caller_handle& other) - { - return this->value == other.value; - } - constexpr bool operator!=(const caller_handle& other) - { - return this->value != other.value; - } - - SCOREP_User_ParameterHandle value = SCOREP_USER_INVALID_PARAMETER; -}; - constexpr region_handle uninitialised_region_handle = region_handle(); -constexpr caller_handle uninitialised_caller_handle = caller_handle(); /// Combine the arguments into a region name inline std::string make_region_name(std::string_view& module_name, std::string_view& name) @@ -57,7 +40,6 @@ inline std::string make_region_name(std::string_view& module_name, std::string_v } extern std::unordered_map regions; -extern std::unordered_map callers; /** tries to enter a region. Return true on success * @@ -76,12 +58,40 @@ inline bool try_region_begin(compat::PyCodeObject* identifier) } } +/** tries to enter a region. Return true on success + * + */ +inline bool try_region_begin_with_callsite(compat::PyCodeObject* identifier, + compat::PyCodeObject* callsite_identifier, + uint32_t callsite_line) +{ + auto it = regions.find(identifier); + if (it != regions.end()) + { + auto callsite_it = regions.find(callsite_identifier); + if (it != regions.end()) + { + SCOREP_User_RegionEnterWithCallsite(it->second.value, callsite_it->second.value, + callsite_line); + return true; + } + } + return false; +} + void region_begin(std::string_view& function_name, std::string_view& module, const std::string& file_name, const std::uint64_t line_number, compat::PyCodeObject* identifier); void region_begin(std::string_view& function_name, std::string_view& module, const std::string& file_name, const std::uint64_t line_number); +void region_begin_with_callsite( + std::string_view& function_name, std::string_view& module, const std::string& file_name, + const std::uint64_t line_number, compat::PyCodeObject* identifier, + compat::PyCodeObject* callsite_identifier, std::string_view& callsite_function_name, + std::string_view& callsite_module, const std::string& callsite_file_name, + const std::uint64_t callsite_line_number_start, uint32_t callsite_line); + /** tries to end a region. Return true on success * */ @@ -103,23 +113,6 @@ void region_end(std::string_view& function_name, std::string_view& module, compat::PyCodeObject* identifier); void region_end(std::string_view& function_name, std::string_view& module); -inline bool try_add_caller(compat::PyCodeObject* identifier, const std::uint64_t line_number) -{ - auto it = callers.find(identifier); - if (it != callers.end()) - { - SCOREP_User_ParameterInt64(&it->second.value, "", line_number); - return true; - } - else - { - return false; - } -} -void region_add_caller(std::string_view& function_name, std::string_view& module, - const std::string& file_name, const std::uint64_t line_number, - compat::PyCodeObject* identifier); - void region_end_error_handling(const std::string& region_name); void rewind_begin(std::string region_name, std::string file_name, std::uint64_t line_number); From e2ba009c1453b6d33199027a3a8660d5d9b118b8 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Fri, 20 May 2022 12:26:34 +0200 Subject: [PATCH 08/11] it somehow works --- setup.py | 2 +- src/scorepy/cInstrumenter.cpp | 2 +- src/scorepy/events.cpp | 34 ++-------------------------------- src/scorepy/events.hpp | 1 + 4 files changed, 5 insertions(+), 34 deletions(-) diff --git a/setup.py b/setup.py index 58f216d..07e76c1 100644 --- a/setup.py +++ b/setup.py @@ -13,7 +13,7 @@ 'Score-P not build with "--enable-shared". Link mode is:\n{}'.format(link_mode) ) -check_compiler = scorep.helper.get_scorep_config("C99 compiler used:") +check_compiler = scorep.helper.get_scorep_config("C99 compiler") if "gcc" in check_compiler: gcc_plugin = scorep.helper.get_scorep_config("GCC plug-in support:") if not ("yes" in gcc_plugin): diff --git a/src/scorepy/cInstrumenter.cpp b/src/scorepy/cInstrumenter.cpp index bcb27aa..217abb0 100644 --- a/src/scorepy/cInstrumenter.cpp +++ b/src/scorepy/cInstrumenter.cpp @@ -152,7 +152,7 @@ bool CInstrumenter::on_event(PyFrameObject& frame, int what, PyObject*) region_begin_with_callsite(name, module_name, file_name, line_number, code, callsite_code, callsite_name, callsite_module_name, callsite_file_name, callsite_line_number_start, - frame.f_back->f_lineno); + PyFrame_GetLineNumber(frame.f_back)); } } } diff --git a/src/scorepy/events.cpp b/src/scorepy/events.cpp index 869b38e..b04e870 100644 --- a/src/scorepy/events.cpp +++ b/src/scorepy/events.cpp @@ -93,7 +93,7 @@ void region_begin_with_callsite( callsite_region.value, std::string(callsite_module, 0, callsite_module.find('.')).c_str()); } - + std::cout << "RegionEnterWithCallsite" << std::endl; SCOREP_User_RegionEnterWithCallsite(region.value, callsite_region.value, callsite_line); } @@ -105,6 +105,7 @@ void region_end(std::string_view& function_name, std::string_view& module, const auto it_region = regions.find(identifier); if (it_region != regions.end()) { + std::cout << "RegionEnd" << std::endl; SCOREP_User_RegionEnd(it_region->second.value); } else @@ -161,37 +162,6 @@ void region_end_error_handling(const std::string& region_name) } } -void region_add_caller(std::string value) -{ - static SCOREP_User_ParameterHandle caller = SCOREP_USER_INVALID_PARAMETER; - SCOREP_User_ParameterString(&caller, "caller", value.c_str()); -} - -void region_add_caller(std::string_view& function_name, std::string_view& module, - const std::string& file_name, const std::uint64_t line_number, - compat::PyCodeObject* identifier) - -{ - caller_handle& caller = callers[identifier]; - - if (caller == uninitialised_caller_handle) - { - // caller is only needed on first registration. - auto caller_region_name = std::string("Caller: ") + make_region_name(module, function_name); - SCOREP_User_ParameterInt64(&caller.value, caller_region_name.c_str(), line_number); - } - else - { - SCOREP_User_ParameterInt64(&caller.value, "", line_number); - } -} - -void region_add_caller_line(int64_t value) -{ - static SCOREP_User_ParameterHandle caller_line = SCOREP_USER_INVALID_PARAMETER; - SCOREP_User_ParameterInt64(&caller_line, "Called from line", value); -} - void rewind_begin(std::string region_name, std::string file_name, std::uint64_t line_number) { auto pair = rewind_regions.emplace(make_pair(region_name, region_handle())); diff --git a/src/scorepy/events.hpp b/src/scorepy/events.hpp index 9ddb2c5..a3c0343 100644 --- a/src/scorepy/events.hpp +++ b/src/scorepy/events.hpp @@ -71,6 +71,7 @@ inline bool try_region_begin_with_callsite(compat::PyCodeObject* identifier, auto callsite_it = regions.find(callsite_identifier); if (it != regions.end()) { + std::cout << "try RegionEnterWithCallsite" << std::endl; SCOREP_User_RegionEnterWithCallsite(it->second.value, callsite_it->second.value, callsite_line); return true; From 7387bda77abf334e9ccd426df06c87977c683b25 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Sat, 21 May 2022 12:51:28 +0200 Subject: [PATCH 09/11] remove debug output --- src/scorepy/events.cpp | 2 -- src/scorepy/events.hpp | 1 - 2 files changed, 3 deletions(-) diff --git a/src/scorepy/events.cpp b/src/scorepy/events.cpp index b04e870..2cb33af 100644 --- a/src/scorepy/events.cpp +++ b/src/scorepy/events.cpp @@ -93,7 +93,6 @@ void region_begin_with_callsite( callsite_region.value, std::string(callsite_module, 0, callsite_module.find('.')).c_str()); } - std::cout << "RegionEnterWithCallsite" << std::endl; SCOREP_User_RegionEnterWithCallsite(region.value, callsite_region.value, callsite_line); } @@ -105,7 +104,6 @@ void region_end(std::string_view& function_name, std::string_view& module, const auto it_region = regions.find(identifier); if (it_region != regions.end()) { - std::cout << "RegionEnd" << std::endl; SCOREP_User_RegionEnd(it_region->second.value); } else diff --git a/src/scorepy/events.hpp b/src/scorepy/events.hpp index a3c0343..9ddb2c5 100644 --- a/src/scorepy/events.hpp +++ b/src/scorepy/events.hpp @@ -71,7 +71,6 @@ inline bool try_region_begin_with_callsite(compat::PyCodeObject* identifier, auto callsite_it = regions.find(callsite_identifier); if (it != regions.end()) { - std::cout << "try RegionEnterWithCallsite" << std::endl; SCOREP_User_RegionEnterWithCallsite(it->second.value, callsite_it->second.value, callsite_line); return true; From f6b8cbcf079a8138733e0ef734d6c9c61a0766e7 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Sat, 21 May 2022 14:34:09 +0200 Subject: [PATCH 10/11] add callsite test --- test/cases/callsite.py | 9 +++++++++ test/test_scorep.py | 29 +++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 test/cases/callsite.py diff --git a/test/cases/callsite.py b/test/cases/callsite.py new file mode 100644 index 0000000..da98b0e --- /dev/null +++ b/test/cases/callsite.py @@ -0,0 +1,9 @@ + +def bar(): + print("bar") + +def foo(): + print("foo") + bar() + +foo() diff --git a/test/test_scorep.py b/test/test_scorep.py index d9249a5..782b892 100755 --- a/test/test_scorep.py +++ b/test/test_scorep.py @@ -76,6 +76,7 @@ def requires_package(name): ] foreach_instrumenter = pytest.mark.parametrize("instrumenter", ALL_INSTRUMENTERS) +foreach_cinstrumenter = pytest.mark.parametrize("instrumenter", ALL_INSTRUMENTERS[2:3]) @pytest.fixture @@ -552,3 +553,31 @@ def test_io(scorep_env, instrumenter): ) print(regex_str) assert re.search(regex_str, io_trace) + + +@foreach_cinstrumenter +def test_callsite(scorep_env, instrumenter): + trace_path = get_trace_path(scorep_env) + + # Also test when no instrumenter is given + instrumenter_type = ["--instrumenter-type=" + instrumenter] + std_out, std_err = call_with_scorep( + "cases/callsite.py", ["--nocompiler"] + instrumenter_type, env=scorep_env + ) + + assert std_err == "" + assert std_out == "foo\nbar\n" + + std_out, std_err = call(["otf2-print", trace_path]) + + assert std_err == "" + region_callsite = [("__main__:foo", "__main__:", 9), + ("__main__:bar", "__main__:foo", 7)] + + for region, callsite, callsite_line in region_callsite: + assert re.search( + 'ENTER[ ]*[0-9 ]*[0-9 ]*Region: "%s" \\<[0-9]*\\>\n' + '[ ]*ADDITIONAL ATTRIBUTES: \\("CALLSITE_REGION" \\<[0-9]*\\>; REGION; "%s" \\<[0-9]*\\>\\)\\, ' + '\\("CALLSITE_LINE" \\<[0-9]*\\>; UINT32; %s\\)' % ( + region, callsite, callsite_line), std_out + ) From 48c636e4481dc12d21ca9cbbd1b191802d2f7ebd Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Sat, 21 May 2022 14:34:31 +0200 Subject: [PATCH 11/11] fix the different test failures --- src/scorepy/cInstrumenter.cpp | 14 ++++++++++++-- src/scorepy/events.cpp | 7 +++++++ src/scorepy/events.hpp | 2 +- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/scorepy/cInstrumenter.cpp b/src/scorepy/cInstrumenter.cpp index 217abb0..ec7db5f 100644 --- a/src/scorepy/cInstrumenter.cpp +++ b/src/scorepy/cInstrumenter.cpp @@ -132,8 +132,8 @@ bool CInstrumenter::on_event(PyFrameObject& frame, int what, PyObject*) else { PyCodeObject* callsite_code = frame.f_back->f_code; - bool success = - try_region_begin_with_callsite(code, callsite_code, frame.f_back->f_lineno); + bool success = try_region_begin_with_callsite(code, callsite_code, + PyFrame_GetLineNumber(frame.f_back)); if (!success) { std::string_view name = compat::get_string_as_utf_8(code->co_name); @@ -146,6 +146,15 @@ bool CInstrumenter::on_event(PyFrameObject& frame, int what, PyObject*) std::string_view callsite_name = compat::get_string_as_utf_8(callsite_code->co_name); std::string_view callsite_module_name = get_module_name(*frame.f_back); + if (callsite_name.compare("_unsetprofile") == 0 || + callsite_module_name.compare(0, 6, "scorep") == 0 || + (callsite_module_name.compare("threading") == 0 && + (callsite_name.compare("_bootstrap_inner") == 0 || + callsite_name.compare("_bootstrap")))) // there needs to be a better way + { + callsite_code = nullptr; // dont save that handle + } + const int callsite_line_number_start = code->co_firstlineno; const std::string callsite_file_name = get_file_name(frame); @@ -161,6 +170,7 @@ bool CInstrumenter::on_event(PyFrameObject& frame, int what, PyObject*) case PyTrace_RETURN: { PyCodeObject* code = frame.f_code; + bool success = try_region_end(code); if (!success) { diff --git a/src/scorepy/events.cpp b/src/scorepy/events.cpp index 2cb33af..dede701 100644 --- a/src/scorepy/events.cpp +++ b/src/scorepy/events.cpp @@ -81,7 +81,14 @@ void region_begin_with_callsite( SCOREP_User_RegionSetGroup(region.value, std::string(module, 0, module.find('.')).c_str()); } + if (callsite_identifier == nullptr) + { + SCOREP_User_RegionEnter(region.value); + return; + } + region_handle& callsite_region = regions[callsite_identifier]; + if (callsite_region == uninitialised_region_handle) { auto region_name = make_region_name(callsite_module, callsite_function_name); diff --git a/src/scorepy/events.hpp b/src/scorepy/events.hpp index 9ddb2c5..df0dd5f 100644 --- a/src/scorepy/events.hpp +++ b/src/scorepy/events.hpp @@ -69,7 +69,7 @@ inline bool try_region_begin_with_callsite(compat::PyCodeObject* identifier, if (it != regions.end()) { auto callsite_it = regions.find(callsite_identifier); - if (it != regions.end()) + if (callsite_it != regions.end()) { SCOREP_User_RegionEnterWithCallsite(it->second.value, callsite_it->second.value, callsite_line);