From df41342e139f376c5f8a0a9c3ce47e860315cb81 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Fri, 12 Jun 2020 17:30:00 +0200 Subject: [PATCH 01/27] a first approach --- scorep/_instrumenters/scorep_profile.py | 20 ++++++++++++++++++++ test/cases/classes.py | 23 +++++++++++++++++++++++ test/cases/classes2.py | 4 ++++ 3 files changed, 47 insertions(+) create mode 100644 test/cases/classes.py create mode 100644 test/cases/classes2.py diff --git a/scorep/_instrumenters/scorep_profile.py b/scorep/_instrumenters/scorep_profile.py index cd0d427..f30e72a 100644 --- a/scorep/_instrumenters/scorep_profile.py +++ b/scorep/_instrumenters/scorep_profile.py @@ -4,6 +4,7 @@ from scorep._instrumenters.utils import get_module_name, get_file_name from scorep._instrumenters.scorep_instrumenter import ScorepInstrumenter from scorep import scorep_bindings +import inspect try: import threading @@ -38,6 +39,25 @@ def _globaltrace(self, frame, why, arg): """ if why == 'call': code = frame.f_code + print("#########################################") + if "arg" in frame.f_locals: + print(code.co_name) + print(code.co_varnames) + print(code.co_cellvars) + print(frame.f_globals.keys()) + print(frame.f_locals.keys()) + print(dir(frame.f_code)) + print(dir(frame.f_locals)) + #print(frame.f_globals.get('__name__', None)) + #print(code.co_name) + #print(code.__dir__()) + #print(frame.f_locals["self"].__class__.__name__) + #print(frame.f_globals) + #print(frame.f_locals) + + #print(dir(frame.f_globals)) + + print("#########################################") modulename = get_module_name(frame) if not code.co_name == "_unsetprofile" and not modulename[:6] == "scorep": full_file_name = get_file_name(frame) diff --git a/test/cases/classes.py b/test/cases/classes.py new file mode 100644 index 0000000..2969a11 --- /dev/null +++ b/test/cases/classes.py @@ -0,0 +1,23 @@ + + +class TestClass: + def foo(self): + print("foo") + def doo(arg): + print("doo") + arg.foo() + + +def foo(): + print("bar") + + def doo(arg): + print("asdgh") + doo("test") + +if __name__ == "__main__": + t = TestClass() + + + t.doo() + foo() \ No newline at end of file diff --git a/test/cases/classes2.py b/test/cases/classes2.py new file mode 100644 index 0000000..fcec7e6 --- /dev/null +++ b/test/cases/classes2.py @@ -0,0 +1,4 @@ +import classes + +t = classes.TestClass +t().doo() \ No newline at end of file From 35daf297486a9f38919ecfd700b619c881ffe4c3 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Fri, 12 Jun 2020 18:44:04 +0200 Subject: [PATCH 02/27] special case, using the object pointer --- scorep/_instrumenters/scorep_profile.py | 16 +++++------ test/cases/reload.py | 35 +++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 10 deletions(-) create mode 100644 test/cases/reload.py diff --git a/scorep/_instrumenters/scorep_profile.py b/scorep/_instrumenters/scorep_profile.py index f30e72a..0484950 100644 --- a/scorep/_instrumenters/scorep_profile.py +++ b/scorep/_instrumenters/scorep_profile.py @@ -39,15 +39,11 @@ def _globaltrace(self, frame, why, arg): """ if why == 'call': code = frame.f_code - print("#########################################") - if "arg" in frame.f_locals: - print(code.co_name) - print(code.co_varnames) - print(code.co_cellvars) - print(frame.f_globals.keys()) - print(frame.f_locals.keys()) - print(dir(frame.f_code)) - print(dir(frame.f_locals)) + + if "foo" in code.co_name: + print("#########################################") + print("code.co_name == foo", id(frame.f_code)) + print("#########################################") #print(frame.f_globals.get('__name__', None)) #print(code.co_name) #print(code.__dir__()) @@ -57,7 +53,7 @@ def _globaltrace(self, frame, why, arg): #print(dir(frame.f_globals)) - print("#########################################") + modulename = get_module_name(frame) if not code.co_name == "_unsetprofile" and not modulename[:6] == "scorep": full_file_name = get_file_name(frame) diff --git a/test/cases/reload.py b/test/cases/reload.py new file mode 100644 index 0000000..466912f --- /dev/null +++ b/test/cases/reload.py @@ -0,0 +1,35 @@ +import importlib +import os + +data1 = """ +def foo(): + print("foo1") +""" + +data2 = """ +def foo(arg): + print(arg) +def bar(): + print("bar") +""" + + +with open("reload_test.py","w") as f: + f.write(data1) + +import reload_test +reload_test.foo() + +importlib.reload(reload_test) +reload_test.foo() + + +with open("reload_test.py","w") as f: + f.write(data2) + +importlib.reload(reload_test) +reload_test.foo("foo2") +reload_test.bar() + +os.remove("reload_test.py") + From 17fec66ee784b5f6d8722b4f357c92a9ee7adf7c Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Fri, 12 Jun 2020 18:45:01 +0200 Subject: [PATCH 03/27] adopt testcase --- test/cases/reload.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/cases/reload.py b/test/cases/reload.py index 466912f..fd47016 100644 --- a/test/cases/reload.py +++ b/test/cases/reload.py @@ -19,6 +19,7 @@ def bar(): import reload_test reload_test.foo() +reload_test.foo() importlib.reload(reload_test) reload_test.foo() From 30f69659853ca409c392183ccde7f077902951f3 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 24 Jun 2020 10:26:43 +0200 Subject: [PATCH 04/27] working state --- scorep/_instrumenters/scorep_profile.py | 16 ++++------------ src/methods.cpp | 13 +++++++++++-- src/scorepy/events.hpp | 7 ++++--- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/scorep/_instrumenters/scorep_profile.py b/scorep/_instrumenters/scorep_profile.py index 0484950..1af795e 100644 --- a/scorep/_instrumenters/scorep_profile.py +++ b/scorep/_instrumenters/scorep_profile.py @@ -39,26 +39,18 @@ def _globaltrace(self, frame, why, arg): """ if why == 'call': code = frame.f_code - + modulename = get_module_name(frame) + if "foo" in code.co_name: print("#########################################") print("code.co_name == foo", id(frame.f_code)) + print("region name == foo", modulename + ":" + code.co_name) print("#########################################") - #print(frame.f_globals.get('__name__', None)) - #print(code.co_name) - #print(code.__dir__()) - #print(frame.f_locals["self"].__class__.__name__) - #print(frame.f_globals) - #print(frame.f_locals) - #print(dir(frame.f_globals)) - - - modulename = get_module_name(frame) if not code.co_name == "_unsetprofile" and not modulename[:6] == "scorep": full_file_name = get_file_name(frame) line_number = code.co_firstlineno - scorep_bindings.region_begin(modulename, code.co_name, full_file_name, line_number) + scorep_bindings.region_begin(frame.f_code, modulename, code.co_name, full_file_name, line_number) elif why == 'return': code = frame.f_code modulename = get_module_name(frame) diff --git a/src/methods.cpp b/src/methods.cpp index 1d4280b..65d4427 100644 --- a/src/methods.cpp +++ b/src/methods.cpp @@ -4,6 +4,8 @@ #include #include +#include + extern "C" { @@ -30,16 +32,23 @@ extern "C" const char* module; const char* region_name; const char* file_name; + const PyObject* identifier; std::uint64_t line_number = 0; - if (!PyArg_ParseTuple(args, "sssK", &module, ®ion_name, &file_name, &line_number)) + if (!PyArg_ParseTuple(args, "OsssK", &identifier, &module, ®ion_name, &file_name, + &line_number)) return NULL; static std::string region = ""; region = module; region += ":"; region += region_name; - scorepy::region_begin(region, module, file_name, line_number); + scorepy::region_begin(identifier, region, module, file_name, line_number); + if (region == "reload_test:foo") + { + std::cout << "Pointer: " << reinterpret_cast(code_object) << std::endl; + std::cout << "Pointer: " << code_object << std::endl; + } Py_RETURN_NONE; } diff --git a/src/scorepy/events.hpp b/src/scorepy/events.hpp index feb2bf1..42ba814 100644 --- a/src/scorepy/events.hpp +++ b/src/scorepy/events.hpp @@ -1,13 +1,14 @@ #pragma once +#include #include #include namespace scorepy { -void region_begin(const std::string& region_name, std::string module, std::string file_name, - std::uint64_t line_number); -void region_end(const std::string& region_name); +void region_begin(const PyObject*&, const std::string& region_name, std::string module, + std::string file_name, std::uint64_t line_number); +void region_end(const PyObject*&); void rewind_begin(std::string region_name, std::string file_name, std::uint64_t line_number); void rewind_end(std::string region_name, bool value); From d92c5d695a5f7efb1b0795786fe6b1f50320a962 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 24 Jun 2020 11:52:24 +0200 Subject: [PATCH 05/27] working with code pointer on profile --- scorep/_instrumenters/scorep_profile.py | 11 +- src/methods.cpp | 33 ++++-- src/scorepy/events.cpp | 149 ++++++++++++++++-------- src/scorepy/events.hpp | 13 ++- 4 files changed, 139 insertions(+), 67 deletions(-) diff --git a/scorep/_instrumenters/scorep_profile.py b/scorep/_instrumenters/scorep_profile.py index 3ae01ad..a662bd5 100644 --- a/scorep/_instrumenters/scorep_profile.py +++ b/scorep/_instrumenters/scorep_profile.py @@ -41,18 +41,13 @@ def _globaltrace(self, frame, why, arg): code = frame.f_code modulename = get_module_name(frame) - if "foo" in code.co_name: - print("#########################################") - print("code.co_name == foo", id(frame.f_code)) - print("region name == foo", modulename + ":" + code.co_name) - print("#########################################") - + if not code.co_name == "_unsetprofile" and not modulename[:6] == "scorep": full_file_name = get_file_name(frame) line_number = code.co_firstlineno - scorep._bindings.region_begin(frame.f_code, modulename, code.co_name, full_file_name, line_number) + scorep._bindings.region_begin(modulename, code.co_name, full_file_name, line_number, code) elif why == 'return': code = frame.f_code modulename = get_module_name(frame) if not code.co_name == "_unsetprofile" and not modulename[:6] == "scorep": - scorep._bindings.region_end(modulename, code.co_name) + scorep._bindings.region_end(modulename, code.co_name, code) diff --git a/src/methods.cpp b/src/methods.cpp index 06a71f6..9fa695a 100644 --- a/src/methods.cpp +++ b/src/methods.cpp @@ -32,20 +32,25 @@ extern "C" const char* module; const char* region_name; const char* file_name; - const PyObject* identifier; + PyObject* identifier = nullptr; std::uint64_t line_number = 0; - if (!PyArg_ParseTuple(args, "OsssK", &identifier, &module, ®ion_name, &file_name, - &line_number)) + if (!PyArg_ParseTuple(args, "sssK|O", &module, ®ion_name, &file_name, &line_number, + &identifier)) + { return NULL; + } const std::string& region = scorepy::make_region_name(module, region_name); - if (region == "reload_test:foo") + if (identifier == nullptr) { - std::cout << "Pointer: " << reinterpret_cast(code_object) << std::endl; - std::cout << "Pointer: " << code_object << std::endl; + scorepy::region_begin(region, module, file_name, line_number); + } + else + { + scorepy::region_begin(region, module, file_name, line_number, + reinterpret_cast(identifier)); } - scorepy::region_begin(region, module, file_name, line_number); Py_RETURN_NONE; } @@ -57,12 +62,22 @@ extern "C" { const char* module; const char* region_name; + PyObject* identifier = nullptr; - if (!PyArg_ParseTuple(args, "ss", &module, ®ion_name)) + if (!PyArg_ParseTuple(args, "ss|O", &module, ®ion_name, &identifier)) + { return NULL; + } const std::string& region = scorepy::make_region_name(module, region_name); - scorepy::region_end(region); + if (identifier == nullptr) + { + scorepy::region_end(region); + } + else + { + scorepy::region_end(region, reinterpret_cast(identifier)); + } Py_RETURN_NONE; } diff --git a/src/scorepy/events.cpp b/src/scorepy/events.cpp index f3b1a5e..0087a83 100644 --- a/src/scorepy/events.cpp +++ b/src/scorepy/events.cpp @@ -23,25 +23,11 @@ struct region_handle constexpr region_handle uninitialised_region_handle = region_handle(); -static std::unordered_map regions; +static std::unordered_map regions; +static std::unordered_map region_translations; +static std::unordered_map user_regions; static std::unordered_map rewind_regions; -void region_begin(const std::string& region_name, std::string module, std::string file_name, - std::uint64_t line_number) -{ - auto& region_handle = regions[region_name]; - - if (region_handle == uninitialised_region_handle) - { - SCOREP_User_RegionInit(®ion_handle.value, NULL, &SCOREP_User_LastFileHandle, - region_name.c_str(), SCOREP_USER_REGION_TYPE_FUNCTION, - file_name.c_str(), line_number); - SCOREP_User_RegionSetGroup(region_handle.value, - std::string(module, 0, module.find('.')).c_str()); - } - SCOREP_User_RegionEnter(region_handle.value); -} - /// Region names that are known to have no region enter event and should not report an error /// on region exit static const std::array EXIT_REGION_WHITELIST = { @@ -52,45 +38,114 @@ static const std::array EXIT_REGION_WHITELIST = { #endif }; -void region_end(const std::string& region_name) +void region_begin(const std::string& region_name, const std::string module, + const std::string file_name, const std::uint64_t line_number, + const std::uintptr_t& identifier) { - const auto itRegion = regions.find(region_name); - if (itRegion != regions.end()) - { - SCOREP_User_RegionEnd(itRegion->second.value); - } - else + auto& region_handle = regions[identifier]; + + if (region_handle == uninitialised_region_handle) { - static region_handle error_region; - static SCOREP_User_ParameterHandle scorep_param = SCOREP_USER_INVALID_PARAMETER; - static bool error_printed = false; + auto it = user_regions.find(region_name); + if (it == user_regions.end()) + { + SCOREP_User_RegionInit(®ion_handle.value, NULL, &SCOREP_User_LastFileHandle, + region_name.c_str(), SCOREP_USER_REGION_TYPE_FUNCTION, + file_name.c_str(), line_number); - if (std::find(EXIT_REGION_WHITELIST.begin(), EXIT_REGION_WHITELIST.end(), region_name) != - EXIT_REGION_WHITELIST.end()) + SCOREP_User_RegionSetGroup(region_handle.value, + std::string(module, 0, module.find('.')).c_str()); + } + else { - return; + region_handle = it->second; } + region_translations[region_name] = identifier; + } + SCOREP_User_RegionEnter(region_handle.value); +} + +void region_begin(const std::string& region_name, const std::string module, + const std::string file_name, const std::uint64_t line_number) +{ + auto& region_handle = user_regions[region_name]; - if (error_region.value == SCOREP_USER_INVALID_REGION) + if (region_handle == uninitialised_region_handle) + { + auto it_translation = region_translations.find(region_name); + if (it_translation == region_translations.end()) { - SCOREP_User_RegionInit(&error_region.value, NULL, &SCOREP_User_LastFileHandle, - "error_region", SCOREP_USER_REGION_TYPE_FUNCTION, "scorep.cpp", - 0); - SCOREP_User_RegionSetGroup(error_region.value, "error"); - } - SCOREP_User_RegionEnter(error_region.value); - SCOREP_User_ParameterString(&scorep_param, "leave-region", region_name.c_str()); - SCOREP_User_RegionEnd(error_region.value); + SCOREP_User_RegionInit(®ion_handle.value, NULL, &SCOREP_User_LastFileHandle, + region_name.c_str(), SCOREP_USER_REGION_TYPE_FUNCTION, + file_name.c_str(), line_number); - if (!error_printed) + SCOREP_User_RegionSetGroup(region_handle.value, + std::string(module, 0, module.find('.')).c_str()); + } + else { - std::cerr << "SCOREP_BINDING_PYTHON ERROR: There was a region exit without an enter!\n" - << "SCOREP_BINDING_PYTHON ERROR: For details look for \"error_region\" in " - "the trace or profile." - << std::endl; - error_printed = true; + region_handle = regions[it_translation->second]; } } + SCOREP_User_RegionEnter(region_handle.value); +} + +void region_end(const std::string& region_name, const std::uintptr_t& identifier) +{ + const auto it_region = regions.find(identifier); + if (it_region != regions.end()) + { + SCOREP_User_RegionEnd(it_region->second.value); + } + else + { + region_end_error_handling(region_name); + } +} + +void region_end(const std::string& region_name) +{ + const auto it_region = user_regions.find(region_name); + if (it_region != user_regions.end()) + { + SCOREP_User_RegionEnd(it_region->second.value); + } + else + { + region_end_error_handling(region_name); + } +} + +void region_end_error_handling(const std::string& region_name) +{ + static region_handle error_region; + static SCOREP_User_ParameterHandle scorep_param = SCOREP_USER_INVALID_PARAMETER; + static bool error_printed = false; + + if (std::find(EXIT_REGION_WHITELIST.begin(), EXIT_REGION_WHITELIST.end(), region_name) != + EXIT_REGION_WHITELIST.end()) + { + return; + } + + if (error_region.value == SCOREP_USER_INVALID_REGION) + { + SCOREP_User_RegionInit(&error_region.value, NULL, &SCOREP_User_LastFileHandle, + "error_region", SCOREP_USER_REGION_TYPE_FUNCTION, "scorep.cpp", 0); + SCOREP_User_RegionSetGroup(error_region.value, "error"); + } + SCOREP_User_RegionEnter(error_region.value); + SCOREP_User_ParameterString(&scorep_param, "leave-region", region_name.c_str()); + SCOREP_User_RegionEnd(error_region.value); + + if (!error_printed) + { + std::cerr << "SCOREP_BINDING_PYTHON ERROR: There was a region exit without an enter!\n" + << "SCOREP_BINDING_PYTHON ERROR: For details look for \"error_region\" in " + "the trace or profile." + << std::endl; + error_printed = true; + } } void rewind_begin(std::string region_name, std::string file_name, std::uint64_t line_number) @@ -136,7 +191,7 @@ void parameter_string(std::string name, std::string value) void oa_region_begin(std::string region_name, std::string file_name, std::uint64_t line_number) { - auto& handle = regions[region_name]; + auto& handle = user_regions[region_name]; SCOREP_User_OaPhaseBegin(&handle.value, &SCOREP_User_LastFileName, &SCOREP_User_LastFileHandle, region_name.c_str(), SCOREP_USER_REGION_TYPE_FUNCTION, file_name.c_str(), line_number); @@ -144,7 +199,7 @@ void oa_region_begin(std::string region_name, std::string file_name, std::uint64 void oa_region_end(std::string region_name) { - auto& handle = regions[region_name]; + auto& handle = user_regions[region_name]; SCOREP_User_OaPhaseEnd(handle.value); } diff --git a/src/scorepy/events.hpp b/src/scorepy/events.hpp index 48fa91c..baecb63 100644 --- a/src/scorepy/events.hpp +++ b/src/scorepy/events.hpp @@ -17,9 +17,16 @@ inline const std::string& make_region_name(const char* module_name, const char* return region; } -void region_begin(const PyObject*&, const std::string& region_name, std::string module, - std::string file_name, std::uint64_t line_number); -void region_end(const PyObject*&); +void region_begin(const std::string& region_name, const std::string module, + const std::string file_name, const std::uint64_t line_number, + const std::uintptr_t& identifier); +void region_begin(const std::string& region_name, const std::string module, + const std::string file_name, const std::uint64_t line_number); + +void region_end(const std::string& region_name, const std::uintptr_t& identifier); +void region_end(const std::string& region_name); + +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); void rewind_end(std::string region_name, bool value); From df09265feb7305e49a8518037a2d94e15f5e8fc0 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 24 Jun 2020 13:33:36 +0200 Subject: [PATCH 06/27] address user regions with line numbers# --- src/scorepy/events.cpp | 4 ++-- test/cases/classes.py | 15 ++++++++++++--- test/cases/user_regions.py | 7 +++++-- 3 files changed, 19 insertions(+), 7 deletions(-) diff --git a/src/scorepy/events.cpp b/src/scorepy/events.cpp index 0087a83..e83cfa1 100644 --- a/src/scorepy/events.cpp +++ b/src/scorepy/events.cpp @@ -60,7 +60,7 @@ void region_begin(const std::string& region_name, const std::string module, { region_handle = it->second; } - region_translations[region_name] = identifier; + region_translations[region_name + std::to_string(line_number)] = identifier; } SCOREP_User_RegionEnter(region_handle.value); } @@ -72,7 +72,7 @@ void region_begin(const std::string& region_name, const std::string module, if (region_handle == uninitialised_region_handle) { - auto it_translation = region_translations.find(region_name); + auto it_translation = region_translations.find(region_name + std::to_string(line_number)); if (it_translation == region_translations.end()) { SCOREP_User_RegionInit(®ion_handle.value, NULL, &SCOREP_User_LastFileHandle, diff --git a/test/cases/classes.py b/test/cases/classes.py index 2969a11..21bbd9f 100644 --- a/test/cases/classes.py +++ b/test/cases/classes.py @@ -1,4 +1,5 @@ - +import scorep.user +import scorep.instrumenter class TestClass: def foo(self): @@ -7,6 +8,10 @@ def doo(arg): print("doo") arg.foo() +class TestClass2: + @scorep.user.region() + def foo(self): + print("foo-2") def foo(): print("bar") @@ -17,7 +22,11 @@ def doo(arg): if __name__ == "__main__": t = TestClass() + t2 = TestClass2() - + t2.foo() t.doo() - foo() \ No newline at end of file + foo() + + with scorep.instrumenter.disable(): + t2.foo() diff --git a/test/cases/user_regions.py b/test/cases/user_regions.py index 6994dfe..e853d43 100644 --- a/test/cases/user_regions.py +++ b/test/cases/user_regions.py @@ -1,5 +1,5 @@ import scorep.user - +import scorep.instrumenter def foo(): scorep.user.region_begin("test_region") @@ -24,5 +24,8 @@ def foo4(): foo() foo2() -foo3() +with scorep.instrumenter.enable(): + foo3() +with scorep.instrumenter.disable(): + foo3() foo4() From b9e48871d899d502a3ddbe838b42fe9ed8d29623 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 24 Jun 2020 14:29:36 +0200 Subject: [PATCH 07/27] update tests --- test/test_scorep.py | 46 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 42 insertions(+), 4 deletions(-) diff --git a/test/test_scorep.py b/test/test_scorep.py index a6ae672..0802840 100755 --- a/test/test_scorep.py +++ b/test/test_scorep.py @@ -92,7 +92,7 @@ def test_user_regions(scorep_env, instrumenter): env=scorep_env) assert std_err == "" - assert std_out == "hello world\nhello world\nhello world3\nhello world4\n" + assert std_out == "hello world\nhello world\nhello world3\nhello world3\nhello world4\n" std_out, std_err = call(["otf2-print", trace_path]) @@ -100,8 +100,8 @@ def test_user_regions(scorep_env, instrumenter): assert re.search('LEAVE[ ]*[0-9 ]*[0-9 ]*Region: "user:test_region"', std_out) assert re.search('ENTER[ ]*[0-9 ]*[0-9 ]*Region: "user:test_region_2"', std_out) assert re.search('LEAVE[ ]*[0-9 ]*[0-9 ]*Region: "user:test_region_2"', std_out) - assert re.search('ENTER[ ]*[0-9 ]*[0-9 ]*Region: "__main__:foo3"', std_out) - assert re.search('LEAVE[ ]*[0-9 ]*[0-9 ]*Region: "__main__:foo3"', std_out) + assert len(re.findall('ENTER[ ]*[0-9 ]*[0-9 ]*Region: "__main__:foo3"', std_out)) == 2 + assert len(re.findall('LEAVE[ ]*[0-9 ]*[0-9 ]*Region: "__main__:foo3"', std_out)) == 2 assert re.search('ENTER[ ]*[0-9 ]*[0-9 ]*Region: "user:test_region_4"', std_out) assert re.search('LEAVE[ ]*[0-9 ]*[0-9 ]*Region: "user:test_region_4"', std_out) @@ -130,7 +130,7 @@ def test_user_regions_no_scorep(): "cases/user_regions.py"]) assert std_err == "" - assert std_out == "hello world\nhello world\nhello world3\nhello world4\n" + assert std_out == "hello world\nhello world\nhello world3\nhello world3\nhello world4\n" @foreach_instrumenter @@ -276,6 +276,44 @@ def test_call_main(scorep_env, instrumenter): assert re.search(expected_std_err, std_err) assert std_out == expected_std_out +@foreach_instrumenter +def test_classes(scorep_env, instrumenter): + trace_path = get_trace_path(scorep_env) + std_out, std_err = call_with_scorep("cases/classes.py", + ["--nocompiler", "--instrumenter-type=" + instrumenter], + expected_returncode=0, + env=scorep_env) + + expected_std_err = "" + expected_std_out = "foo-2\ndoo\nfoo\nbar\nasdgh\nfoo-2\n" + + assert std_out == expected_std_out + assert std_err == expected_std_err + + std_out, std_err = call(["otf2-print", trace_path]) + + assert std_err == "" + + region_ids = [] + foo_count = 0 + for line in std_out.split("\n"): + m = re.search('ENTER[ ]*[0-9 ]*[0-9 ]*Region: "__main__:foo" <([0-9]*)>', line) + if m is not None: + foo_count += 1 + r_id = m.group(1) + + if foo_count == 1: + region_ids.append(r_id) + continue + + if foo_count < 4: + assert region_ids[-1] < r_id # check if foo regions are different + else: + assert r_id == region_ids[0] # check if last foo is fist foo + region_ids.append(r_id) + + assert len(region_ids) == 4 + def test_dummy(scorep_env): std_out, std_err = call_with_scorep("cases/instrumentation.py", From ecb1fddf6923aca7bca1ade632721476b49d157a Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 24 Jun 2020 14:41:05 +0200 Subject: [PATCH 08/27] only create name when needed --- src/methods.cpp | 10 ++++------ src/scorepy/cInstrumenter.cpp | 6 ++---- src/scorepy/events.cpp | 18 +++++++++++------- src/scorepy/events.hpp | 16 ++++++++-------- 4 files changed, 25 insertions(+), 25 deletions(-) diff --git a/src/methods.cpp b/src/methods.cpp index 9fa695a..d49921a 100644 --- a/src/methods.cpp +++ b/src/methods.cpp @@ -41,14 +41,13 @@ extern "C" return NULL; } - const std::string& region = scorepy::make_region_name(module, region_name); if (identifier == nullptr) { - scorepy::region_begin(region, module, file_name, line_number); + scorepy::region_begin(region_name, module, file_name, line_number); } else { - scorepy::region_begin(region, module, file_name, line_number, + scorepy::region_begin(region_name, module, file_name, line_number, reinterpret_cast(identifier)); } @@ -69,14 +68,13 @@ extern "C" return NULL; } - const std::string& region = scorepy::make_region_name(module, region_name); if (identifier == nullptr) { - scorepy::region_end(region); + scorepy::region_end(module, region_name); } else { - scorepy::region_end(region, reinterpret_cast(identifier)); + scorepy::region_end(module, region_name, reinterpret_cast(identifier)); } Py_RETURN_NONE; diff --git a/src/scorepy/cInstrumenter.cpp b/src/scorepy/cInstrumenter.cpp index 42719f0..1b8e072 100644 --- a/src/scorepy/cInstrumenter.cpp +++ b/src/scorepy/cInstrumenter.cpp @@ -122,9 +122,8 @@ bool CInstrumenter::on_event(PyFrameObject& frame, int what, PyObject*) if (std::string(name) != "_unsetprofile" && std::string(module_name, 0, 6) != "scorep") { const int line_number = code.co_firstlineno; - const auto& region_name = make_region_name(module_name, name); const auto file_name = get_file_name(frame); - region_begin(region_name, module_name, file_name, line_number); + region_begin(name, module_name, file_name, line_number); } break; } @@ -138,8 +137,7 @@ bool CInstrumenter::on_event(PyFrameObject& frame, int what, PyObject*) // TODO: Use string_view/CString comparison? if (std::string(name) != "_unsetprofile" && std::string(module_name, 0, 6) != "scorep") { - const auto& region_name = make_region_name(module_name, name); - region_end(region_name); + region_end(module_name, name); } break; } diff --git a/src/scorepy/events.cpp b/src/scorepy/events.cpp index e83cfa1..598aac6 100644 --- a/src/scorepy/events.cpp +++ b/src/scorepy/events.cpp @@ -38,14 +38,14 @@ static const std::array EXIT_REGION_WHITELIST = { #endif }; -void region_begin(const std::string& region_name, const std::string module, - const std::string file_name, const std::uint64_t line_number, - const std::uintptr_t& identifier) +void region_begin(const std::string& region, const std::string module, const std::string file_name, + const std::uint64_t line_number, const std::uintptr_t& identifier) { auto& region_handle = regions[identifier]; if (region_handle == uninitialised_region_handle) { + auto& region_name = make_region_name(module, region); auto it = user_regions.find(region_name); if (it == user_regions.end()) { @@ -65,9 +65,10 @@ void region_begin(const std::string& region_name, const std::string module, SCOREP_User_RegionEnter(region_handle.value); } -void region_begin(const std::string& region_name, const std::string module, - const std::string file_name, const std::uint64_t line_number) +void region_begin(const std::string& region, const std::string module, const std::string file_name, + const std::uint64_t line_number) { + auto& region_name = make_region_name(module, region); auto& region_handle = user_regions[region_name]; if (region_handle == uninitialised_region_handle) @@ -90,7 +91,8 @@ void region_begin(const std::string& region_name, const std::string module, SCOREP_User_RegionEnter(region_handle.value); } -void region_end(const std::string& region_name, const std::uintptr_t& identifier) +void region_end(const std::string& region, const std::string& module, + const std::uintptr_t& identifier) { const auto it_region = regions.find(identifier); if (it_region != regions.end()) @@ -99,12 +101,14 @@ void region_end(const std::string& region_name, const std::uintptr_t& identifier } else { + auto& region_name = make_region_name(module, region); region_end_error_handling(region_name); } } -void region_end(const std::string& region_name) +void region_end(const std::string& region, const std::string& module) { + auto& region_name = make_region_name(module, region); const auto it_region = user_regions.find(region_name); if (it_region != user_regions.end()) { diff --git a/src/scorepy/events.hpp b/src/scorepy/events.hpp index baecb63..0ad85a5 100644 --- a/src/scorepy/events.hpp +++ b/src/scorepy/events.hpp @@ -8,7 +8,7 @@ namespace scorepy { /// 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(const char* module_name, const char* name) +inline const std::string& make_region_name(const std::string& module_name, const std::string& name) { static std::string region; region = module_name; @@ -17,14 +17,14 @@ inline const std::string& make_region_name(const char* module_name, const char* return region; } -void region_begin(const std::string& region_name, const std::string module, - const std::string file_name, const std::uint64_t line_number, - const std::uintptr_t& identifier); -void region_begin(const std::string& region_name, const std::string module, - const std::string file_name, const std::uint64_t line_number); +void region_begin(const std::string& region, const std::string& module, const std::string file_name, + const std::uint64_t line_number, const std::uintptr_t& identifier); +void region_begin(const std::string& region, const std::string& module, const std::string file_name, + const std::uint64_t line_number); -void region_end(const std::string& region_name, const std::uintptr_t& identifier); -void region_end(const std::string& region_name); +void region_end(const std::string& region, const std::string& module, + const std::uintptr_t& identifier); +void region_end(const std::string& region, const std::string& module); void region_end_error_handling(const std::string& region_name); From 882d8c279c93716a9b7c2309da76014d69363763 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 24 Jun 2020 14:59:18 +0200 Subject: [PATCH 09/27] fix region and module order --- src/methods.cpp | 16 ++++++++-------- src/scorepy/cInstrumenter.cpp | 2 +- src/scorepy/events.cpp | 9 +++++---- src/scorepy/events.hpp | 9 +++++---- 4 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/methods.cpp b/src/methods.cpp index d49921a..2896973 100644 --- a/src/methods.cpp +++ b/src/methods.cpp @@ -30,12 +30,12 @@ extern "C" static PyObject* region_begin(PyObject* self, PyObject* args) { const char* module; - const char* region_name; + const char* region; const char* file_name; PyObject* identifier = nullptr; std::uint64_t line_number = 0; - if (!PyArg_ParseTuple(args, "sssK|O", &module, ®ion_name, &file_name, &line_number, + if (!PyArg_ParseTuple(args, "sssK|O", &module, ®ion, &file_name, &line_number, &identifier)) { return NULL; @@ -43,11 +43,11 @@ extern "C" if (identifier == nullptr) { - scorepy::region_begin(region_name, module, file_name, line_number); + scorepy::region_begin(region, module, file_name, line_number); } else { - scorepy::region_begin(region_name, module, file_name, line_number, + scorepy::region_begin(region, module, file_name, line_number, reinterpret_cast(identifier)); } @@ -60,21 +60,21 @@ extern "C" static PyObject* region_end(PyObject* self, PyObject* args) { const char* module; - const char* region_name; + const char* region; PyObject* identifier = nullptr; - if (!PyArg_ParseTuple(args, "ss|O", &module, ®ion_name, &identifier)) + if (!PyArg_ParseTuple(args, "ss|O", &module, ®ion, &identifier)) { return NULL; } if (identifier == nullptr) { - scorepy::region_end(module, region_name); + scorepy::region_end(region, module); } else { - scorepy::region_end(module, region_name, reinterpret_cast(identifier)); + scorepy::region_end(region, module, reinterpret_cast(identifier)); } Py_RETURN_NONE; diff --git a/src/scorepy/cInstrumenter.cpp b/src/scorepy/cInstrumenter.cpp index 1b8e072..ab3c5f0 100644 --- a/src/scorepy/cInstrumenter.cpp +++ b/src/scorepy/cInstrumenter.cpp @@ -137,7 +137,7 @@ bool CInstrumenter::on_event(PyFrameObject& frame, int what, PyObject*) // TODO: Use string_view/CString comparison? if (std::string(name) != "_unsetprofile" && std::string(module_name, 0, 6) != "scorep") { - region_end(module_name, name); + region_end(name, module_name); } break; } diff --git a/src/scorepy/events.cpp b/src/scorepy/events.cpp index 598aac6..2968a72 100644 --- a/src/scorepy/events.cpp +++ b/src/scorepy/events.cpp @@ -38,8 +38,9 @@ static const std::array EXIT_REGION_WHITELIST = { #endif }; -void region_begin(const std::string& region, const std::string module, const std::string file_name, - const std::uint64_t line_number, const std::uintptr_t& identifier) +void region_begin(const std::string& region, const std::string& module, + const std::string& file_name, const std::uint64_t line_number, + const std::uintptr_t& identifier) { auto& region_handle = regions[identifier]; @@ -65,8 +66,8 @@ void region_begin(const std::string& region, const std::string module, const std SCOREP_User_RegionEnter(region_handle.value); } -void region_begin(const std::string& region, const std::string module, const std::string file_name, - const std::uint64_t line_number) +void region_begin(const std::string& region, const std::string& module, + const std::string& file_name, const std::uint64_t line_number) { auto& region_name = make_region_name(module, region); auto& region_handle = user_regions[region_name]; diff --git a/src/scorepy/events.hpp b/src/scorepy/events.hpp index 0ad85a5..01a7a0a 100644 --- a/src/scorepy/events.hpp +++ b/src/scorepy/events.hpp @@ -17,10 +17,11 @@ inline const std::string& make_region_name(const std::string& module_name, const return region; } -void region_begin(const std::string& region, const std::string& module, const std::string file_name, - const std::uint64_t line_number, const std::uintptr_t& identifier); -void region_begin(const std::string& region, const std::string& module, const std::string file_name, - const std::uint64_t line_number); +void region_begin(const std::string& region, const std::string& module, + const std::string& file_name, const std::uint64_t line_number, + const std::uintptr_t& identifier); +void region_begin(const std::string& region, const std::string& module, + const std::string& file_name, const std::uint64_t line_number); void region_end(const std::string& region, const std::string& module, const std::uintptr_t& identifier); From c01aa1f46550c9536aa3d5f427c6323dd031c67a Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 24 Jun 2020 15:05:46 +0200 Subject: [PATCH 10/27] extend the other instrumenters --- scorep/_instrumenters/scorep_trace.py | 4 ++-- src/scorepy/cInstrumenter.cpp | 16 +++++++++------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/scorep/_instrumenters/scorep_trace.py b/scorep/_instrumenters/scorep_trace.py index 18cb629..bcbef66 100644 --- a/scorep/_instrumenters/scorep_trace.py +++ b/scorep/_instrumenters/scorep_trace.py @@ -40,7 +40,7 @@ def _globaltrace(self, frame, why, arg): if not code.co_name == "_unsettrace" and not modulename[:6] == "scorep": full_file_name = get_file_name(frame) line_number = code.co_firstlineno - scorep._bindings.region_begin(modulename, code.co_name, full_file_name, line_number) + scorep._bindings.region_begin(modulename, code.co_name, full_file_name, line_number, code) return self._localtrace return None @@ -48,5 +48,5 @@ def _localtrace(self, frame, why, arg): if why == 'return': code = frame.f_code modulename = get_module_name(frame) - scorep._bindings.region_end(modulename, code.co_name) + scorep._bindings.region_end(modulename, code.co_name, code) return self._localtrace diff --git a/src/scorepy/cInstrumenter.cpp b/src/scorepy/cInstrumenter.cpp index ab3c5f0..6bfca81 100644 --- a/src/scorepy/cInstrumenter.cpp +++ b/src/scorepy/cInstrumenter.cpp @@ -3,6 +3,7 @@ #include "pythonHelpers.hpp" #include #include +#include #include namespace scorepy @@ -113,31 +114,32 @@ bool CInstrumenter::on_event(PyFrameObject& frame, int what, PyObject*) { case PyTrace_CALL: { - const PyCodeObject& code = *frame.f_code; - const char* name = PyUnicode_AsUTF8(code.co_name); + const PyCodeObject* code = frame.f_code; + const char* name = PyUnicode_AsUTF8(code->co_name); const char* module_name = get_module_name(frame); assert(name); assert(module_name); // TODO: Use string_view/CString comparison? if (std::string(name) != "_unsetprofile" && std::string(module_name, 0, 6) != "scorep") { - const int line_number = code.co_firstlineno; + const int line_number = code->co_firstlineno; const auto file_name = get_file_name(frame); - region_begin(name, module_name, file_name, line_number); + region_begin(name, module_name, file_name, line_number, + reinterpret_cast(code)); } break; } case PyTrace_RETURN: { - const PyCodeObject& code = *frame.f_code; - const char* name = PyUnicode_AsUTF8(code.co_name); + const PyCodeObject* code = frame.f_code; + const char* name = PyUnicode_AsUTF8(code->co_name); const char* module_name = get_module_name(frame); assert(name); assert(module_name); // TODO: Use string_view/CString comparison? if (std::string(name) != "_unsetprofile" && std::string(module_name, 0, 6) != "scorep") { - region_end(name, module_name); + region_end(name, module_name, reinterpret_cast(code)); } break; } From ec407c90a135e4909422d50b827adc1590544117 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 24 Jun 2020 15:17:49 +0200 Subject: [PATCH 11/27] update README.md --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 476074d..9628352 100644 --- a/README.md +++ b/README.md @@ -261,6 +261,7 @@ Please be aware the `--user` is always passed to Score-P, as this is needed for ## Not Working * python multiprocessing * Score-P does currently only support MPI or SHMEM. Any other multiprocessing approach cannot be traced. +* tracking `importlib.reload()` # Acknowledgments The European Union initially supported this work as part of the European Union’s Horizon 2020 project READEX (grant agreement number 671657). From a396c802e9bd95a70158e36dced824edec0c96d8 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 24 Jun 2020 15:22:33 +0200 Subject: [PATCH 12/27] autopep8 --- scorep/_instrumenters/scorep_profile.py | 2 -- test/cases/classes.py | 13 +++++++++---- test/cases/classes2.py | 2 +- test/cases/reload.py | 7 +++---- test/cases/user_regions.py | 1 + test/test_scorep.py | 17 +++++++++-------- 6 files changed, 23 insertions(+), 19 deletions(-) diff --git a/scorep/_instrumenters/scorep_profile.py b/scorep/_instrumenters/scorep_profile.py index a662bd5..479a23f 100644 --- a/scorep/_instrumenters/scorep_profile.py +++ b/scorep/_instrumenters/scorep_profile.py @@ -4,7 +4,6 @@ from scorep._instrumenters.utils import get_module_name, get_file_name from scorep._instrumenters.scorep_instrumenter import ScorepInstrumenter import scorep._bindings -import inspect try: import threading @@ -41,7 +40,6 @@ def _globaltrace(self, frame, why, arg): code = frame.f_code modulename = get_module_name(frame) - if not code.co_name == "_unsetprofile" and not modulename[:6] == "scorep": full_file_name = get_file_name(frame) line_number = code.co_firstlineno diff --git a/test/cases/classes.py b/test/cases/classes.py index 21bbd9f..8789e0e 100644 --- a/test/cases/classes.py +++ b/test/cases/classes.py @@ -1,29 +1,34 @@ import scorep.user import scorep.instrumenter + class TestClass: def foo(self): print("foo") + def doo(arg): print("doo") arg.foo() + class TestClass2: @scorep.user.region() def foo(self): print("foo-2") + def foo(): print("bar") - + def doo(arg): print("asdgh") doo("test") - + + if __name__ == "__main__": t = TestClass() - t2 = TestClass2() - + t2 = TestClass2() + t2.foo() t.doo() foo() diff --git a/test/cases/classes2.py b/test/cases/classes2.py index fcec7e6..1c5ddbc 100644 --- a/test/cases/classes2.py +++ b/test/cases/classes2.py @@ -1,4 +1,4 @@ import classes t = classes.TestClass -t().doo() \ No newline at end of file +t().doo() diff --git a/test/cases/reload.py b/test/cases/reload.py index fd47016..36e4fc3 100644 --- a/test/cases/reload.py +++ b/test/cases/reload.py @@ -1,3 +1,4 @@ +import reload_test import importlib import os @@ -14,10 +15,9 @@ def bar(): """ -with open("reload_test.py","w") as f: +with open("reload_test.py", "w") as f: f.write(data1) -import reload_test reload_test.foo() reload_test.foo() @@ -25,7 +25,7 @@ def bar(): reload_test.foo() -with open("reload_test.py","w") as f: +with open("reload_test.py", "w") as f: f.write(data2) importlib.reload(reload_test) @@ -33,4 +33,3 @@ def bar(): reload_test.bar() os.remove("reload_test.py") - diff --git a/test/cases/user_regions.py b/test/cases/user_regions.py index e853d43..c3a3358 100644 --- a/test/cases/user_regions.py +++ b/test/cases/user_regions.py @@ -1,6 +1,7 @@ import scorep.user import scorep.instrumenter + def foo(): scorep.user.region_begin("test_region") print("hello world") diff --git a/test/test_scorep.py b/test/test_scorep.py index 0802840..9cddf2a 100755 --- a/test/test_scorep.py +++ b/test/test_scorep.py @@ -276,6 +276,7 @@ def test_call_main(scorep_env, instrumenter): assert re.search(expected_std_err, std_err) assert std_out == expected_std_out + @foreach_instrumenter def test_classes(scorep_env, instrumenter): trace_path = get_trace_path(scorep_env) @@ -293,7 +294,7 @@ def test_classes(scorep_env, instrumenter): std_out, std_err = call(["otf2-print", trace_path]) assert std_err == "" - + region_ids = [] foo_count = 0 for line in std_out.split("\n"): @@ -301,19 +302,19 @@ def test_classes(scorep_env, instrumenter): if m is not None: foo_count += 1 r_id = m.group(1) - + if foo_count == 1: region_ids.append(r_id) continue - + if foo_count < 4: - assert region_ids[-1] < r_id # check if foo regions are different + assert region_ids[-1] < r_id # check if foo regions are different else: - assert r_id == region_ids[0] # check if last foo is fist foo + assert r_id == region_ids[0] # check if last foo is fist foo region_ids.append(r_id) - + assert len(region_ids) == 4 - + def test_dummy(scorep_env): std_out, std_err = call_with_scorep("cases/instrumentation.py", @@ -349,7 +350,7 @@ def test_threads(scorep_env, instrumenter): trace_path = get_trace_path(scorep_env) std_out, std_err = call_with_scorep("cases/use_threads.py", - ["--nocompiler", "--instrumenter-type=" + instrumenter], + ["--nocompiler", "--instrumenter-type=" + instrumenter], env=scorep_env) assert std_err == "" From fa506c1dc19111489c4fce18961a5d6c9458e6d8 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Mon, 6 Jul 2020 18:49:37 +0200 Subject: [PATCH 13/27] Apply suggestions from code review a few more `const` --- src/scorepy/events.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/scorepy/events.cpp b/src/scorepy/events.cpp index 2968a72..eff702f 100644 --- a/src/scorepy/events.cpp +++ b/src/scorepy/events.cpp @@ -46,7 +46,7 @@ void region_begin(const std::string& region, const std::string& module, if (region_handle == uninitialised_region_handle) { - auto& region_name = make_region_name(module, region); + const auto& region_name = make_region_name(module, region); auto it = user_regions.find(region_name); if (it == user_regions.end()) { @@ -69,7 +69,7 @@ void region_begin(const std::string& region, const std::string& module, void region_begin(const std::string& region, const std::string& module, const std::string& file_name, const std::uint64_t line_number) { - auto& region_name = make_region_name(module, region); + const auto& region_name = make_region_name(module, region); auto& region_handle = user_regions[region_name]; if (region_handle == uninitialised_region_handle) From fbdcffaf90d651fa3abd4a0da74fd7b90623a9e7 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 15 Jul 2020 11:51:14 +0200 Subject: [PATCH 14/27] sperate user regions and instrumented or decorated regions --- scorep/_instrumenters/base_instrumenter.py | 4 +- scorep/_instrumenters/dummy.py | 4 +- scorep/_instrumenters/scorep_instrumenter.py | 8 ++-- scorep/user.py | 32 ++++++++----- src/methods.cpp | 4 +- src/scorepy/events.cpp | 48 +++++++------------- test/cases/decorator.py | 13 ++++++ test/test_scorep.py | 15 ++++++ 8 files changed, 76 insertions(+), 52 deletions(-) create mode 100644 test/cases/decorator.py diff --git a/scorep/_instrumenters/base_instrumenter.py b/scorep/_instrumenters/base_instrumenter.py index 86db0b7..7e03d76 100644 --- a/scorep/_instrumenters/base_instrumenter.py +++ b/scorep/_instrumenters/base_instrumenter.py @@ -30,11 +30,11 @@ def run(self, cmd, globals=None, locals=None): pass @abc.abstractmethod - def region_begin(self, module_name, function_name, file_name, line_number): + def region_begin(self, module_name, function_name, file_name, line_number, code_object): pass @abc.abstractmethod - def region_end(self, module_name, function_name): + def region_end(self, module_name, function_name, code_object): pass @abc.abstractmethod diff --git a/scorep/_instrumenters/dummy.py b/scorep/_instrumenters/dummy.py index 7a64844..e3fbe85 100644 --- a/scorep/_instrumenters/dummy.py +++ b/scorep/_instrumenters/dummy.py @@ -23,10 +23,10 @@ def run(self, cmd, globals=None, locals=None): locals = {} exec(cmd, globals, locals) - def region_begin(self, module_name, function_name, file_name, line_number): + def region_begin(self, module_name, function_name, file_name, line_number, code_object=None): pass - def region_end(self, module_name, function_name): + def region_end(self, module_name, function_name, code_object=None): pass def rewind_begin(self, name, file_name=None, line_number=None): diff --git a/scorep/_instrumenters/scorep_instrumenter.py b/scorep/_instrumenters/scorep_instrumenter.py index 062194b..75384e7 100644 --- a/scorep/_instrumenters/scorep_instrumenter.py +++ b/scorep/_instrumenters/scorep_instrumenter.py @@ -56,14 +56,14 @@ def run(self, cmd, globals=None, locals=None): finally: self.unregister() - def region_begin(self, module_name, function_name, file_name, line_number): + def region_begin(self, module_name, function_name, file_name, line_number, code_object=None): """Record a region begin event""" scorep._bindings.region_begin( - module_name, function_name, file_name, line_number) + module_name, function_name, file_name, line_number, code_object) - def region_end(self, module_name, function_name): + def region_end(self, module_name, function_name, code_object=None): """Record a region end event""" - scorep._bindings.region_end(module_name, function_name) + scorep._bindings.region_end(module_name, function_name, code_object) def rewind_begin(self, name, file_name=None, line_number=None): """ diff --git a/scorep/user.py b/scorep/user.py index 900e896..8dbf5e9 100644 --- a/scorep/user.py +++ b/scorep/user.py @@ -87,12 +87,11 @@ def __enter__(self): scorep.instrumenter.get_instrumenter().region_begin( self.module_name, self.region_name, full_file_name, line_number) elif(callable(self.func)): - """ - looks like the decorator is invoked - """ + # looks like the decorator is invoked if not initally_registered: self.region_name = self.func.__name__ self.module_name = self.func.__module__ + self.code_obj = self.func.__code__ file_name = self.func.__code__.co_filename line_number = self.func.__code__.co_firstlineno @@ -102,11 +101,11 @@ def __enter__(self): full_file_name = "None" scorep.instrumenter.get_instrumenter().region_begin( - self.module_name, self.region_name, full_file_name, line_number) + self.module_name, self.region_name, full_file_name, line_number, self.code_obj) else: - """ - do not need to decorate a function, when we are registerd. It is instrumented any way. - """ + # do not need to decorate a function, when we are registerd. It is + # instrumented any way. # do not need to decorate a function, when we are + # registerd. It is instrumented any way. pass else: raise RuntimeError("a region name needs to be specified") @@ -117,12 +116,23 @@ def __exit__(self, exc_type, exc_value, traceback): if (callable(self.func) and instrumenter.get_instrumenter().get_registered() and not self.user_region_name): - """ - looks like there is a decorator, we are registered and the name is not specified by the user, - so we do not need to do anything. The Instrumentation will take care. - """ + # * we are a decorator + # * we are registered + # * the name is not specified by the user, + # The Instrumentation will care about the exit region return False + elif (callable(self.func) and not self.user_region_name): + # * we are a decorator + # * we are not registered + # * the name is not specified by the user, + # We need to exit the region, and to pass the code object, as we are a decorator + scorep.instrumenter.get_instrumenter().region_end( + self.module_name, self.region_name, self.code_obj) else: + # * we might be a decorator + # * we are not registered + # * a name is specified by the user, + # We need to exit the region and do not need to pass a code object scorep.instrumenter.get_instrumenter().region_end( self.module_name, self.region_name) return False diff --git a/src/methods.cpp b/src/methods.cpp index 2896973..ceb3d7c 100644 --- a/src/methods.cpp +++ b/src/methods.cpp @@ -41,7 +41,7 @@ extern "C" return NULL; } - if (identifier == nullptr) + if (identifier == nullptr or identifier == Py_None) { scorepy::region_begin(region, module, file_name, line_number); } @@ -68,7 +68,7 @@ extern "C" return NULL; } - if (identifier == nullptr) + if (identifier == nullptr or identifier == Py_None) { scorepy::region_end(region, module); } diff --git a/src/scorepy/events.cpp b/src/scorepy/events.cpp index eff702f..d51e0b0 100644 --- a/src/scorepy/events.cpp +++ b/src/scorepy/events.cpp @@ -24,7 +24,6 @@ struct region_handle constexpr region_handle uninitialised_region_handle = region_handle(); static std::unordered_map regions; -static std::unordered_map region_translations; static std::unordered_map user_regions; static std::unordered_map rewind_regions; @@ -38,6 +37,7 @@ static const std::array EXIT_REGION_WHITELIST = { #endif }; +// Userd for regions, that have an idetifier, aka a code object id void region_begin(const std::string& region, const std::string& module, const std::string& file_name, const std::uint64_t line_number, const std::uintptr_t& identifier) @@ -46,26 +46,18 @@ void region_begin(const std::string& region, const std::string& module, if (region_handle == uninitialised_region_handle) { - const auto& region_name = make_region_name(module, region); - auto it = user_regions.find(region_name); - if (it == user_regions.end()) - { - SCOREP_User_RegionInit(®ion_handle.value, NULL, &SCOREP_User_LastFileHandle, - region_name.c_str(), SCOREP_USER_REGION_TYPE_FUNCTION, - file_name.c_str(), line_number); - - SCOREP_User_RegionSetGroup(region_handle.value, - std::string(module, 0, module.find('.')).c_str()); - } - else - { - region_handle = it->second; - } - region_translations[region_name + std::to_string(line_number)] = identifier; + auto& region_name = make_region_name(module, region); + SCOREP_User_RegionInit(®ion_handle.value, NULL, &SCOREP_User_LastFileHandle, + region_name.c_str(), SCOREP_USER_REGION_TYPE_FUNCTION, + file_name.c_str(), line_number); + + SCOREP_User_RegionSetGroup(region_handle.value, + std::string(module, 0, module.find('.')).c_str()); } SCOREP_User_RegionEnter(region_handle.value); } +// Userd for regions, that only have a function name, a module, a file and a line number void region_begin(const std::string& region, const std::string& module, const std::string& file_name, const std::uint64_t line_number) { @@ -74,24 +66,17 @@ void region_begin(const std::string& region, const std::string& module, if (region_handle == uninitialised_region_handle) { - auto it_translation = region_translations.find(region_name + std::to_string(line_number)); - if (it_translation == region_translations.end()) - { - SCOREP_User_RegionInit(®ion_handle.value, NULL, &SCOREP_User_LastFileHandle, - region_name.c_str(), SCOREP_USER_REGION_TYPE_FUNCTION, - file_name.c_str(), line_number); - - SCOREP_User_RegionSetGroup(region_handle.value, - std::string(module, 0, module.find('.')).c_str()); - } - else - { - region_handle = regions[it_translation->second]; - } + SCOREP_User_RegionInit(®ion_handle.value, NULL, &SCOREP_User_LastFileHandle, + region_name.c_str(), SCOREP_USER_REGION_TYPE_FUNCTION, + file_name.c_str(), line_number); + + SCOREP_User_RegionSetGroup(region_handle.value, + std::string(module, 0, module.find('.')).c_str()); } SCOREP_User_RegionEnter(region_handle.value); } +// Userd for regions, that have an idetifier, aka a code object id void region_end(const std::string& region, const std::string& module, const std::uintptr_t& identifier) { @@ -107,6 +92,7 @@ void region_end(const std::string& region, const std::string& module, } } +// Userd for regions, that only have a function name, a module void region_end(const std::string& region, const std::string& module) { auto& region_name = make_region_name(module, region); diff --git a/test/cases/decorator.py b/test/cases/decorator.py new file mode 100644 index 0000000..c991de6 --- /dev/null +++ b/test/cases/decorator.py @@ -0,0 +1,13 @@ +import scorep.user + + +@scorep.user.region() +def foo(): + print("hello world") + + +foo() +with scorep.instrumenter.disable(): + foo() + with scorep.instrumenter.enable(): + foo() diff --git a/test/test_scorep.py b/test/test_scorep.py index 9cddf2a..e70f85f 100755 --- a/test/test_scorep.py +++ b/test/test_scorep.py @@ -124,6 +124,21 @@ def test_context(scorep_env, instrumenter): assert re.search('ENTER[ ]*[0-9 ]*[0-9 ]*Region: "__main__:foo"', std_out) assert re.search('LEAVE[ ]*[0-9 ]*[0-9 ]*Region: "__main__:foo"', std_out) +@foreach_instrumenter +def test_decorator(scorep_env, instrumenter): + trace_path = get_trace_path(scorep_env) + + std_out, std_err = call_with_scorep("cases/decorator.py", + ["--noinstrumenter", "--instrumenter-type=" + instrumenter], + env=scorep_env) + + assert std_err == "" + assert std_out == "hello world\nhello world\nhello world\n" + + std_out, std_err = call(["otf2-print", "-A", trace_path]) + + assert len(re.findall('REGION[ ]*[0-9 ]*Name: "__main__:foo"', std_out)) == 1 + def test_user_regions_no_scorep(): std_out, std_err = call([sys.executable, From 09ff74576aacaab86f1c94dddbe942392aa09ea9 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 15 Jul 2020 11:53:00 +0200 Subject: [PATCH 15/27] fix style --- test/test_scorep.py | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test_scorep.py b/test/test_scorep.py index e70f85f..87b4514 100755 --- a/test/test_scorep.py +++ b/test/test_scorep.py @@ -124,6 +124,7 @@ def test_context(scorep_env, instrumenter): assert re.search('ENTER[ ]*[0-9 ]*[0-9 ]*Region: "__main__:foo"', std_out) assert re.search('LEAVE[ ]*[0-9 ]*[0-9 ]*Region: "__main__:foo"', std_out) + @foreach_instrumenter def test_decorator(scorep_env, instrumenter): trace_path = get_trace_path(scorep_env) From 5eb191d4def2b8bc37c306d5416e301b2c752bd9 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 15 Jul 2020 12:00:36 +0200 Subject: [PATCH 16/27] back to ref --- src/scorepy/cInstrumenter.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/scorepy/cInstrumenter.cpp b/src/scorepy/cInstrumenter.cpp index 6bfca81..d250566 100644 --- a/src/scorepy/cInstrumenter.cpp +++ b/src/scorepy/cInstrumenter.cpp @@ -114,32 +114,32 @@ bool CInstrumenter::on_event(PyFrameObject& frame, int what, PyObject*) { case PyTrace_CALL: { - const PyCodeObject* code = frame.f_code; - const char* name = PyUnicode_AsUTF8(code->co_name); + const PyCodeObject& code = *frame.f_code; + const char* name = PyUnicode_AsUTF8(code.co_name); const char* module_name = get_module_name(frame); assert(name); assert(module_name); // TODO: Use string_view/CString comparison? if (std::string(name) != "_unsetprofile" && std::string(module_name, 0, 6) != "scorep") { - const int line_number = code->co_firstlineno; + const int line_number = code.co_firstlineno; const auto file_name = get_file_name(frame); region_begin(name, module_name, file_name, line_number, - reinterpret_cast(code)); + reinterpret_cast(&code)); } break; } case PyTrace_RETURN: { - const PyCodeObject* code = frame.f_code; - const char* name = PyUnicode_AsUTF8(code->co_name); + const PyCodeObject& code = *frame.f_code; + const char* name = PyUnicode_AsUTF8(code.co_name); const char* module_name = get_module_name(frame); assert(name); assert(module_name); // TODO: Use string_view/CString comparison? if (std::string(name) != "_unsetprofile" && std::string(module_name, 0, 6) != "scorep") { - region_end(name, module_name, reinterpret_cast(code)); + region_end(name, module_name, reinterpret_cast(&code)); } break; } From ac703fd181174075c8aab307f75f09fd01a644c7 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 15 Jul 2020 12:05:26 +0200 Subject: [PATCH 17/27] region --> function_name --- src/methods.cpp | 17 +++++++++-------- src/scorepy/events.cpp | 16 ++++++++-------- src/scorepy/events.hpp | 8 ++++---- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/src/methods.cpp b/src/methods.cpp index ceb3d7c..390d01d 100644 --- a/src/methods.cpp +++ b/src/methods.cpp @@ -30,12 +30,12 @@ extern "C" static PyObject* region_begin(PyObject* self, PyObject* args) { const char* module; - const char* region; + const char* function_name; const char* file_name; PyObject* identifier = nullptr; std::uint64_t line_number = 0; - if (!PyArg_ParseTuple(args, "sssK|O", &module, ®ion, &file_name, &line_number, + if (!PyArg_ParseTuple(args, "sssK|O", &module, &function_name, &file_name, &line_number, &identifier)) { return NULL; @@ -43,11 +43,11 @@ extern "C" if (identifier == nullptr or identifier == Py_None) { - scorepy::region_begin(region, module, file_name, line_number); + scorepy::region_begin(function_name, module, file_name, line_number); } else { - scorepy::region_begin(region, module, file_name, line_number, + scorepy::region_begin(function_name, module, file_name, line_number, reinterpret_cast(identifier)); } @@ -60,21 +60,22 @@ extern "C" static PyObject* region_end(PyObject* self, PyObject* args) { const char* module; - const char* region; + const char* function_name; PyObject* identifier = nullptr; - if (!PyArg_ParseTuple(args, "ss|O", &module, ®ion, &identifier)) + if (!PyArg_ParseTuple(args, "ss|O", &module, &function_name, &identifier)) { return NULL; } if (identifier == nullptr or identifier == Py_None) { - scorepy::region_end(region, module); + scorepy::region_end(function_name, module); } else { - scorepy::region_end(region, module, reinterpret_cast(identifier)); + scorepy::region_end(function_name, module, + reinterpret_cast(identifier)); } Py_RETURN_NONE; diff --git a/src/scorepy/events.cpp b/src/scorepy/events.cpp index d51e0b0..cbf648d 100644 --- a/src/scorepy/events.cpp +++ b/src/scorepy/events.cpp @@ -38,7 +38,7 @@ static const std::array EXIT_REGION_WHITELIST = { }; // Userd for regions, that have an idetifier, aka a code object id -void region_begin(const std::string& region, const std::string& module, +void region_begin(const std::string& function_name, const std::string& module, const std::string& file_name, const std::uint64_t line_number, const std::uintptr_t& identifier) { @@ -46,7 +46,7 @@ void region_begin(const std::string& region, const std::string& module, if (region_handle == uninitialised_region_handle) { - auto& region_name = make_region_name(module, region); + auto& region_name = make_region_name(module, function_name); SCOREP_User_RegionInit(®ion_handle.value, NULL, &SCOREP_User_LastFileHandle, region_name.c_str(), SCOREP_USER_REGION_TYPE_FUNCTION, file_name.c_str(), line_number); @@ -58,10 +58,10 @@ void region_begin(const std::string& region, const std::string& module, } // Userd for regions, that only have a function name, a module, a file and a line number -void region_begin(const std::string& region, const std::string& module, +void region_begin(const std::string& function_name, const std::string& module, const std::string& file_name, const std::uint64_t line_number) { - const auto& region_name = make_region_name(module, region); + const auto& region_name = make_region_name(module, function_name); auto& region_handle = user_regions[region_name]; if (region_handle == uninitialised_region_handle) @@ -77,7 +77,7 @@ void region_begin(const std::string& region, const std::string& module, } // Userd for regions, that have an idetifier, aka a code object id -void region_end(const std::string& region, const std::string& module, +void region_end(const std::string& function_name, const std::string& module, const std::uintptr_t& identifier) { const auto it_region = regions.find(identifier); @@ -87,15 +87,15 @@ void region_end(const std::string& region, const std::string& module, } else { - auto& region_name = make_region_name(module, region); + auto& region_name = make_region_name(module, function_name); region_end_error_handling(region_name); } } // Userd for regions, that only have a function name, a module -void region_end(const std::string& region, const std::string& module) +void region_end(const std::string& function_name, const std::string& module) { - auto& region_name = make_region_name(module, region); + auto& region_name = make_region_name(module, function_name); const auto it_region = user_regions.find(region_name); if (it_region != user_regions.end()) { diff --git a/src/scorepy/events.hpp b/src/scorepy/events.hpp index 01a7a0a..649842f 100644 --- a/src/scorepy/events.hpp +++ b/src/scorepy/events.hpp @@ -17,15 +17,15 @@ inline const std::string& make_region_name(const std::string& module_name, const return region; } -void region_begin(const std::string& region, const std::string& module, +void region_begin(const std::string& function_name, const std::string& module, const std::string& file_name, const std::uint64_t line_number, const std::uintptr_t& identifier); -void region_begin(const std::string& region, const std::string& module, +void region_begin(const std::string& function_name, const std::string& module, const std::string& file_name, const std::uint64_t line_number); -void region_end(const std::string& region, const std::string& module, +void region_end(const std::string& function_name, const std::string& module, const std::uintptr_t& identifier); -void region_end(const std::string& region, const std::string& module); +void region_end(const std::string& function_name, const std::string& module); void region_end_error_handling(const std::string& region_name); From e2ac3e842be97cb5d9ff4de179b35359bcd01b4d Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 15 Jul 2020 12:07:28 +0200 Subject: [PATCH 18/27] &SCOREP_User_LastFileHandle --> NULL --- src/scorepy/events.cpp | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/scorepy/events.cpp b/src/scorepy/events.cpp index cbf648d..839ad25 100644 --- a/src/scorepy/events.cpp +++ b/src/scorepy/events.cpp @@ -47,9 +47,8 @@ void region_begin(const std::string& function_name, const std::string& module, if (region_handle == uninitialised_region_handle) { auto& region_name = make_region_name(module, function_name); - SCOREP_User_RegionInit(®ion_handle.value, NULL, &SCOREP_User_LastFileHandle, - region_name.c_str(), SCOREP_USER_REGION_TYPE_FUNCTION, - file_name.c_str(), line_number); + SCOREP_User_RegionInit(®ion_handle.value, NULL, NULL, region_name.c_str(), + SCOREP_USER_REGION_TYPE_FUNCTION, file_name.c_str(), line_number); SCOREP_User_RegionSetGroup(region_handle.value, std::string(module, 0, module.find('.')).c_str()); @@ -66,9 +65,8 @@ void region_begin(const std::string& function_name, const std::string& module, if (region_handle == uninitialised_region_handle) { - SCOREP_User_RegionInit(®ion_handle.value, NULL, &SCOREP_User_LastFileHandle, - region_name.c_str(), SCOREP_USER_REGION_TYPE_FUNCTION, - file_name.c_str(), line_number); + SCOREP_User_RegionInit(®ion_handle.value, NULL, NULL, region_name.c_str(), + SCOREP_USER_REGION_TYPE_FUNCTION, file_name.c_str(), line_number); SCOREP_User_RegionSetGroup(region_handle.value, std::string(module, 0, module.find('.')).c_str()); @@ -121,8 +119,8 @@ void region_end_error_handling(const std::string& region_name) if (error_region.value == SCOREP_USER_INVALID_REGION) { - SCOREP_User_RegionInit(&error_region.value, NULL, &SCOREP_User_LastFileHandle, - "error_region", SCOREP_USER_REGION_TYPE_FUNCTION, "scorep.cpp", 0); + SCOREP_User_RegionInit(&error_region.value, NULL, NULL, "error_region", + SCOREP_USER_REGION_TYPE_FUNCTION, "scorep.cpp", 0); SCOREP_User_RegionSetGroup(error_region.value, "error"); } SCOREP_User_RegionEnter(error_region.value); @@ -146,9 +144,8 @@ void rewind_begin(std::string region_name, std::string file_name, std::uint64_t auto& handle = pair.first->second; if (inserted_new) { - SCOREP_User_RegionInit(&handle.value, NULL, &SCOREP_User_LastFileHandle, - region_name.c_str(), SCOREP_USER_REGION_TYPE_FUNCTION, - file_name.c_str(), line_number); + SCOREP_User_RegionInit(&handle.value, NULL, NULL, region_name.c_str(), + SCOREP_USER_REGION_TYPE_FUNCTION, file_name.c_str(), line_number); } SCOREP_User_RewindRegionEnter(handle.value); } @@ -183,9 +180,8 @@ void parameter_string(std::string name, std::string value) void oa_region_begin(std::string region_name, std::string file_name, std::uint64_t line_number) { auto& handle = user_regions[region_name]; - SCOREP_User_OaPhaseBegin(&handle.value, &SCOREP_User_LastFileName, &SCOREP_User_LastFileHandle, - region_name.c_str(), SCOREP_USER_REGION_TYPE_FUNCTION, - file_name.c_str(), line_number); + SCOREP_User_OaPhaseBegin(&handle.value, &SCOREP_User_LastFileName, NULL, region_name.c_str(), + SCOREP_USER_REGION_TYPE_FUNCTION, file_name.c_str(), line_number); } void oa_region_end(std::string region_name) From 3c708abe895f3f3206467ec9491f20f50f8b2d0e Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 15 Jul 2020 15:05:11 +0200 Subject: [PATCH 19/27] Update scorep/user.py --- scorep/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scorep/user.py b/scorep/user.py index 8dbf5e9..67a7c45 100644 --- a/scorep/user.py +++ b/scorep/user.py @@ -104,7 +104,7 @@ def __enter__(self): self.module_name, self.region_name, full_file_name, line_number, self.code_obj) else: # do not need to decorate a function, when we are registerd. It is - # instrumented any way. # do not need to decorate a function, when we are + # instrumented any way. # registerd. It is instrumented any way. pass else: From 8afb236cf06e8e95b403cebb537e8c137ddc344f Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 15 Jul 2020 15:08:48 +0200 Subject: [PATCH 20/27] Update src/scorepy/events.cpp Co-authored-by: Alexander Grund --- src/scorepy/events.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scorepy/events.cpp b/src/scorepy/events.cpp index 839ad25..3c1cc92 100644 --- a/src/scorepy/events.cpp +++ b/src/scorepy/events.cpp @@ -56,7 +56,7 @@ void region_begin(const std::string& function_name, const std::string& module, SCOREP_User_RegionEnter(region_handle.value); } -// Userd for regions, that only have a function name, a module, a file and a line number +// Used for regions, that only have a function name, a module, a file and a line number (user regions) void region_begin(const std::string& function_name, const std::string& module, const std::string& file_name, const std::uint64_t line_number) { From 6cafedea95820e5d0c5c3b3036cf5177ff1e1ff2 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 15 Jul 2020 15:10:49 +0200 Subject: [PATCH 21/27] Update src/scorepy/events.cpp Co-authored-by: Alexander Grund --- src/scorepy/events.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scorepy/events.cpp b/src/scorepy/events.cpp index 3c1cc92..cc74a4e 100644 --- a/src/scorepy/events.cpp +++ b/src/scorepy/events.cpp @@ -180,7 +180,7 @@ void parameter_string(std::string name, std::string value) void oa_region_begin(std::string region_name, std::string file_name, std::uint64_t line_number) { auto& handle = user_regions[region_name]; - SCOREP_User_OaPhaseBegin(&handle.value, &SCOREP_User_LastFileName, NULL, region_name.c_str(), + SCOREP_User_OaPhaseBegin(&handle.value, NULL, NULL, region_name.c_str(), SCOREP_USER_REGION_TYPE_FUNCTION, file_name.c_str(), line_number); } From a609531688574628ac7053268de18ebcf9f34e3c Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 15 Jul 2020 15:11:04 +0200 Subject: [PATCH 22/27] Update src/scorepy/events.hpp Co-authored-by: Alexander Grund --- src/scorepy/events.hpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/scorepy/events.hpp b/src/scorepy/events.hpp index 649842f..43400ca 100644 --- a/src/scorepy/events.hpp +++ b/src/scorepy/events.hpp @@ -1,6 +1,5 @@ #pragma once -#include #include #include From 9274ba90d64bff963c5a5a14e3041055e9cc2ddc Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 15 Jul 2020 15:12:07 +0200 Subject: [PATCH 23/27] Update src/scorepy/events.cpp Co-authored-by: Alexander Grund --- src/scorepy/events.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/scorepy/events.cpp b/src/scorepy/events.cpp index cc74a4e..9cfd278 100644 --- a/src/scorepy/events.cpp +++ b/src/scorepy/events.cpp @@ -90,7 +90,7 @@ void region_end(const std::string& function_name, const std::string& module, } } -// Userd for regions, that only have a function name, a module +// Used for regions, that only have a function name, a module (user regions) void region_end(const std::string& function_name, const std::string& module) { auto& region_name = make_region_name(module, function_name); From 646767b2a814f5733cb3198180079af105e063e3 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 15 Jul 2020 15:58:55 +0200 Subject: [PATCH 24/27] remove optional arguments --- src/methods.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/methods.cpp b/src/methods.cpp index 390d01d..68ed2e3 100644 --- a/src/methods.cpp +++ b/src/methods.cpp @@ -35,7 +35,7 @@ extern "C" PyObject* identifier = nullptr; std::uint64_t line_number = 0; - if (!PyArg_ParseTuple(args, "sssK|O", &module, &function_name, &file_name, &line_number, + if (!PyArg_ParseTuple(args, "sssKO", &module, &function_name, &file_name, &line_number, &identifier)) { return NULL; @@ -63,7 +63,7 @@ extern "C" const char* function_name; PyObject* identifier = nullptr; - if (!PyArg_ParseTuple(args, "ss|O", &module, &function_name, &identifier)) + if (!PyArg_ParseTuple(args, "ssO", &module, &function_name, &identifier)) { return NULL; } From 4793e03841eec8240a312215e0d0bf6c520dd7e6 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 15 Jul 2020 15:59:22 +0200 Subject: [PATCH 25/27] restructure code, to make things more clear. --- scorep/user.py | 74 ++++++++++++++++++++++---------------------------- 1 file changed, 33 insertions(+), 41 deletions(-) diff --git a/scorep/user.py b/scorep/user.py index 67a7c45..5357d06 100644 --- a/scorep/user.py +++ b/scorep/user.py @@ -75,6 +75,7 @@ def __enter__(self): initally_registered = instrumenter.get_instrumenter().get_registered() with scorep.instrumenter.disable(): if(self.user_region_name): + # The user did specify a region name, so its a user_region self.module_name = "user" frame = inspect.currentframe().f_back file_name = frame.f_globals.get('__file__', None) @@ -86,56 +87,47 @@ def __enter__(self): scorep.instrumenter.get_instrumenter().region_begin( self.module_name, self.region_name, full_file_name, line_number) - elif(callable(self.func)): - # looks like the decorator is invoked - if not initally_registered: - self.region_name = self.func.__name__ - self.module_name = self.func.__module__ - self.code_obj = self.func.__code__ - file_name = self.func.__code__.co_filename - line_number = self.func.__code__.co_firstlineno - - if file_name is not None: - full_file_name = os.path.abspath(file_name) - else: - full_file_name = "None" - - scorep.instrumenter.get_instrumenter().region_begin( - self.module_name, self.region_name, full_file_name, line_number, self.code_obj) + elif callable(self.func) and not initally_registered: + # The user did not specify a region name, and it's a callable, so it's a semi instrumented region + self.region_name = self.func.__name__ + self.module_name = self.func.__module__ + self.code_obj = self.func.__code__ + file_name = self.func.__code__.co_filename + line_number = self.func.__code__.co_firstlineno + + if file_name is not None: + full_file_name = os.path.abspath(file_name) else: - # do not need to decorate a function, when we are registerd. It is - # instrumented any way. - # registerd. It is instrumented any way. - pass + full_file_name = "None" + + scorep.instrumenter.get_instrumenter().region_begin( + self.module_name, self.region_name, full_file_name, line_number, self.code_obj) + elif callable(self.func) and initally_registered: + # The user did not specify a region name, and it's a callable, so it's a semi instrumented region. However, the instrumenter is active, so there is nothing to do. + pass else: - raise RuntimeError("a region name needs to be specified") + #The user did not specify a region name, and it's not a callable. So it is a context region without a region name. Throw an error. + raise RuntimeError("A region name needs to be specified.") return self def __exit__(self, exc_type, exc_value, traceback): - if (callable(self.func) - and instrumenter.get_instrumenter().get_registered() - and not self.user_region_name): - # * we are a decorator - # * we are registered - # * the name is not specified by the user, - # The Instrumentation will care about the exit region - return False - elif (callable(self.func) and not self.user_region_name): - # * we are a decorator - # * we are not registered - # * the name is not specified by the user, - # We need to exit the region, and to pass the code object, as we are a decorator + initally_registered = instrumenter.get_instrumenter().get_registered() + if self.user_region_name: + # The user did specify a region name, so its a user_region + scorep.instrumenter.get_instrumenter().region_end( + self.module_name, self.region_name) + elif callable(self.func) and not initally_registered: + # The user did not specify a region name, and it's a callable, so it's a semi instrumented region scorep.instrumenter.get_instrumenter().region_end( self.module_name, self.region_name, self.code_obj) + elif callable(self.func) and initally_registered: + # The user did not specify a region name, and it's a callable, so it's a semi instrumented region. However, the instrumenter is active, so there is nothing to do. + pass else: - # * we might be a decorator - # * we are not registered - # * a name is specified by the user, - # We need to exit the region and do not need to pass a code object - scorep.instrumenter.get_instrumenter().region_end( - self.module_name, self.region_name) - return False + #The user did not specify a region name, and it's not a callable. So it is a context region without a region name. Throw an error. + raise RuntimeError("Something wen't wrong. Please do a Bug Report.") + return False def rewind_begin(name, file_name=None, line_number=None): From 0996309c7486d1c4894abe5862369766b26ca215 Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 15 Jul 2020 16:01:36 +0200 Subject: [PATCH 26/27] codestyle --- scorep/user.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/scorep/user.py b/scorep/user.py index 5357d06..a33ae63 100644 --- a/scorep/user.py +++ b/scorep/user.py @@ -103,10 +103,13 @@ def __enter__(self): scorep.instrumenter.get_instrumenter().region_begin( self.module_name, self.region_name, full_file_name, line_number, self.code_obj) elif callable(self.func) and initally_registered: - # The user did not specify a region name, and it's a callable, so it's a semi instrumented region. However, the instrumenter is active, so there is nothing to do. + # The user did not specify a region name, and it's a callable, so it's a + # semi instrumented region. However, the instrumenter is active, so there + # is nothing to do. pass else: - #The user did not specify a region name, and it's not a callable. So it is a context region without a region name. Throw an error. + # The user did not specify a region name, and it's not a callable. So it + # is a context region without a region name. Throw an error. raise RuntimeError("A region name needs to be specified.") return self @@ -117,17 +120,20 @@ def __exit__(self, exc_type, exc_value, traceback): # The user did specify a region name, so its a user_region scorep.instrumenter.get_instrumenter().region_end( self.module_name, self.region_name) - elif callable(self.func) and not initally_registered: + elif callable(self.func) and not initally_registered: # The user did not specify a region name, and it's a callable, so it's a semi instrumented region scorep.instrumenter.get_instrumenter().region_end( self.module_name, self.region_name, self.code_obj) elif callable(self.func) and initally_registered: - # The user did not specify a region name, and it's a callable, so it's a semi instrumented region. However, the instrumenter is active, so there is nothing to do. + # The user did not specify a region name, and it's a callable, so it's a + # semi instrumented region. However, the instrumenter is active, so there + # is nothing to do. pass else: - #The user did not specify a region name, and it's not a callable. So it is a context region without a region name. Throw an error. + # The user did not specify a region name, and it's not a callable. So it + # is a context region without a region name. Throw an error. raise RuntimeError("Something wen't wrong. Please do a Bug Report.") - return False + return False def rewind_begin(name, file_name=None, line_number=None): From 05a4fe9b772ee31479471c0422aec7ee4beddfde Mon Sep 17 00:00:00 2001 From: Andreas Gocht Date: Wed, 15 Jul 2020 16:03:10 +0200 Subject: [PATCH 27/27] some doc --- src/scorepy/events.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/scorepy/events.cpp b/src/scorepy/events.cpp index 9cfd278..5b2e378 100644 --- a/src/scorepy/events.cpp +++ b/src/scorepy/events.cpp @@ -37,7 +37,8 @@ static const std::array EXIT_REGION_WHITELIST = { #endif }; -// Userd for regions, that have an idetifier, aka a code object id +// Used for regions, that have an identifier, aka a code object id. (instrumenter regions and +// some decorated regions) void region_begin(const std::string& function_name, const std::string& module, const std::string& file_name, const std::uint64_t line_number, const std::uintptr_t& identifier) @@ -56,7 +57,8 @@ void region_begin(const std::string& function_name, const std::string& module, SCOREP_User_RegionEnter(region_handle.value); } -// Used for regions, that only have a function name, a module, a file and a line number (user regions) +// Used for regions, that only have a function name, a module, a file and a line number (user +// regions) void region_begin(const std::string& function_name, const std::string& module, const std::string& file_name, const std::uint64_t line_number) { @@ -74,7 +76,8 @@ void region_begin(const std::string& function_name, const std::string& module, SCOREP_User_RegionEnter(region_handle.value); } -// Userd for regions, that have an idetifier, aka a code object id +// Used for regions, that have an identifier, aka a code object id. (instrumenter regions and +// some decorated regions) void region_end(const std::string& function_name, const std::string& module, const std::uintptr_t& identifier) {