Skip to content

Commit 132d21c

Browse files
FlamefireFlamefire
committed
Add CString class to avoid allocations
1 parent 8446c11 commit 132d21c

7 files changed

Lines changed: 152 additions & 61 deletions

File tree

‎src/methods.cpp‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
#include "methods.hpp"
22
#include "scorepy/events.hpp"
33
#include "scorepy/pathUtils.hpp"
4+
#include "scorepy/pythonHelpers.hpp"
45
#include <Python.h>
56
#include <cstdint>
67
#include <scorep/SCOREP_User_Functions.h>
@@ -30,19 +31,20 @@ extern "C"
3031
*/
3132
static PyObject* region_begin(PyObject* self, PyObject* args)
3233
{
33-
const char* module;
34-
const char* function_name;
35-
const char* file_name;
34+
scorepy::PythonCString module;
35+
scorepy::PythonCString function_name;
36+
scorepy::PythonCString file_name;
3637
PyObject* identifier = nullptr;
3738
std::uint64_t line_number = 0;
3839

39-
if (!PyArg_ParseTuple(args, "sssKO", &module, &function_name, &file_name, &line_number,
40+
if (!PyArg_ParseTuple(args, "s#s#s#KO", &module.s, &module.l, &function_name.s,
41+
&function_name.l, &file_name.s, &file_name.l, &line_number,
4042
&identifier))
4143
{
4244
return NULL;
4345
}
4446

45-
if (identifier == nullptr or identifier == Py_None)
47+
if (identifier == Py_None)
4648
{
4749
scorepy::region_begin(function_name, module, file_name, line_number);
4850
}
@@ -60,16 +62,17 @@ extern "C"
6062
*/
6163
static PyObject* region_end(PyObject* self, PyObject* args)
6264
{
63-
const char* module;
64-
const char* function_name;
65+
scorepy::PythonCString module;
66+
scorepy::PythonCString function_name;
6567
PyObject* identifier = nullptr;
6668

67-
if (!PyArg_ParseTuple(args, "ssO", &module, &function_name, &identifier))
69+
if (!PyArg_ParseTuple(args, "s#s#O", &module.s, &module.l, &function_name.s,
70+
&function_name.l, &identifier))
6871
{
6972
return NULL;
7073
}
7174

72-
if (identifier == nullptr or identifier == Py_None)
75+
if (identifier == Py_None)
7376
{
7477
scorepy::region_end(function_name, module);
7578
}

‎src/scorepy/cInstrumenter.cpp‎

Lines changed: 8 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include "cInstrumenter.hpp"
2+
#include "cstring.h"
23
#include "events.hpp"
34
#include "pythonHelpers.hpp"
45
#include <algorithm>
@@ -115,15 +116,12 @@ bool CInstrumenter::on_event(PyFrameObject& frame, int what, PyObject*)
115116
case PyTrace_CALL:
116117
{
117118
const PyCodeObject& code = *frame.f_code;
118-
const char* name = PyUnicode_AsUTF8(code.co_name);
119-
const char* module_name = get_module_name(frame);
120-
assert(name);
121-
assert(module_name);
122-
// TODO: Use string_view/CString comparison?
123-
if (std::string(name) != "_unsetprofile" && std::string(module_name, 0, 6) != "scorep")
119+
const CString name = get_string_from_python(*code.co_name);
120+
const CString module_name = get_module_name(frame);
121+
if (name != "_unsetprofile" && !module_name.starts_with("scorep"))
124122
{
125123
const int line_number = code.co_firstlineno;
126-
const auto file_name = get_file_name(frame);
124+
const CString file_name = get_file_name(frame);
127125
region_begin(name, module_name, file_name, line_number,
128126
reinterpret_cast<std::uintptr_t>(&code));
129127
}
@@ -132,12 +130,9 @@ bool CInstrumenter::on_event(PyFrameObject& frame, int what, PyObject*)
132130
case PyTrace_RETURN:
133131
{
134132
const PyCodeObject& code = *frame.f_code;
135-
const char* name = PyUnicode_AsUTF8(code.co_name);
136-
const char* module_name = get_module_name(frame);
137-
assert(name);
138-
assert(module_name);
139-
// TODO: Use string_view/CString comparison?
140-
if (std::string(name) != "_unsetprofile" && std::string(module_name, 0, 6) != "scorep")
133+
const CString name = get_string_from_python(*code.co_name);
134+
const CString module_name = get_module_name(frame);
135+
if (name != "_unsetprofile" && !module_name.starts_with("scorep"))
141136
{
142137
region_end(name, module_name, reinterpret_cast<std::uintptr_t>(&code));
143138
}

‎src/scorepy/cstring.h‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
#pragma once
2+
3+
#include <cassert>
4+
#include <cstring>
5+
6+
namespace scorepy
7+
{
8+
9+
/// Thin wrapper around a C-String (NULL-terminated sequence of chars)
10+
class CString
11+
{
12+
const char* s_;
13+
size_t len_;
14+
15+
public:
16+
template <size_t N>
17+
constexpr CString(const char (&s)[N]) : s_(s), len_(N - 1u)
18+
{
19+
static_assert(N > 0, "Cannot handle empty char array");
20+
}
21+
22+
explicit CString(const char* s, size_t len) : s_(s), len_(len)
23+
{
24+
assert(s_);
25+
}
26+
27+
explicit CString(const char* s) : s_(s), len_(std::strlen(s_))
28+
{
29+
assert(s_);
30+
}
31+
32+
constexpr const char* c_str() const
33+
{
34+
return s_;
35+
}
36+
/// Find the first occurrence of the character and return a pointer to it or NULL if not found
37+
const char* find(char c) const
38+
{
39+
return static_cast<const char*>(std::memchr(s_, c, len_));
40+
}
41+
template <size_t N>
42+
bool starts_with(const char (&prefix)[N]) const
43+
{
44+
return (len_ >= N - 1u) && (std::memcmp(s_, prefix, N - 1u) == 0);
45+
}
46+
47+
friend bool operator==(const CString& lhs, const CString& rhs)
48+
{
49+
return (lhs.len_ == rhs.len_) && (std::memcmp(lhs.s_, rhs.s_, rhs.len_) == 0);
50+
}
51+
friend bool operator!=(const CString& lhs, const CString& rhs)
52+
{
53+
return !(lhs == rhs);
54+
}
55+
};
56+
57+
} // namespace scorepy

‎src/scorepy/events.cpp‎

Lines changed: 30 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -37,49 +37,60 @@ static const std::array<std::string, 2> EXIT_REGION_WHITELIST = {
3737
#endif
3838
};
3939

40+
static region_handle create_user_region(const std::string& region_name, const CString module,
41+
const CString file_name, const std::uint64_t line_number)
42+
{
43+
region_handle handle;
44+
SCOREP_User_RegionInit(&handle.value, NULL, NULL, region_name.c_str(),
45+
SCOREP_USER_REGION_TYPE_FUNCTION, file_name.c_str(), line_number);
46+
47+
// Extract main module name if module is like "mainmodule.submodule.subsubmodule"
48+
const char* dot_pos = module.find('.');
49+
if (dot_pos)
50+
{
51+
const std::string main_module(module.c_str(), dot_pos);
52+
SCOREP_User_RegionSetGroup(handle.value, main_module.c_str());
53+
}
54+
else
55+
{
56+
SCOREP_User_RegionSetGroup(handle.value, module.c_str());
57+
}
58+
return handle;
59+
}
60+
4061
// Used for regions, that have an identifier, aka a code object id. (instrumenter regions and
4162
// some decorated regions)
42-
void region_begin(const std::string& function_name, const std::string& module,
43-
const std::string& file_name, const std::uint64_t line_number,
44-
const std::uintptr_t& identifier)
63+
void region_begin(const CString function_name, const CString module, const CString file_name,
64+
const std::uint64_t line_number, const std::uintptr_t& identifier)
4565
{
4666
auto& region_handle = regions[identifier];
4767

4868
if (region_handle == uninitialised_region_handle)
4969
{
50-
auto& region_name = make_region_name(module, function_name);
51-
SCOREP_User_RegionInit(&region_handle.value, NULL, NULL, region_name.c_str(),
52-
SCOREP_USER_REGION_TYPE_FUNCTION, file_name.c_str(), line_number);
53-
54-
SCOREP_User_RegionSetGroup(region_handle.value,
55-
std::string(module, 0, module.find('.')).c_str());
70+
const auto& region_name = make_region_name(module, function_name);
71+
region_handle = create_user_region(region_name, module, file_name, line_number);
5672
}
5773
SCOREP_User_RegionEnter(region_handle.value);
5874
}
5975

6076
// Used for regions, that only have a function name, a module, a file and a line number (user
6177
// regions)
62-
void region_begin(const std::string& function_name, const std::string& module,
63-
const std::string& file_name, const std::uint64_t line_number)
78+
void region_begin(const CString function_name, const CString module, const CString file_name,
79+
const std::uint64_t line_number)
6480
{
6581
const auto& region_name = make_region_name(module, function_name);
6682
auto& region_handle = user_regions[region_name];
6783

6884
if (region_handle == uninitialised_region_handle)
6985
{
70-
SCOREP_User_RegionInit(&region_handle.value, NULL, NULL, region_name.c_str(),
71-
SCOREP_USER_REGION_TYPE_FUNCTION, file_name.c_str(), line_number);
72-
73-
SCOREP_User_RegionSetGroup(region_handle.value,
74-
std::string(module, 0, module.find('.')).c_str());
86+
region_handle = create_user_region(region_name, module, file_name, line_number);
7587
}
7688
SCOREP_User_RegionEnter(region_handle.value);
7789
}
7890

7991
// Used for regions, that have an identifier, aka a code object id. (instrumenter regions and
8092
// some decorated regions)
81-
void region_end(const std::string& function_name, const std::string& module,
82-
const std::uintptr_t& identifier)
93+
void region_end(const CString function_name, const CString module, const std::uintptr_t& identifier)
8394
{
8495
const auto it_region = regions.find(identifier);
8596
if (it_region != regions.end())
@@ -94,7 +105,7 @@ void region_end(const std::string& function_name, const std::string& module,
94105
}
95106

96107
// Used for regions, that only have a function name, a module (user regions)
97-
void region_end(const std::string& function_name, const std::string& module)
108+
void region_end(const CString function_name, const CString module)
98109
{
99110
auto& region_name = make_region_name(module, function_name);
100111
const auto it_region = user_regions.find(region_name);

‎src/scorepy/events.hpp‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,29 @@
11
#pragma once
22

3+
#include "cstring.h"
34
#include <cstdint>
45
#include <string>
56

67
namespace scorepy
78
{
89
/// Combine the arguments into a region name
910
/// Return value is a statically allocated string to avoid memory (re)allocations
10-
inline const std::string& make_region_name(const std::string& module_name, const std::string& name)
11+
inline const std::string& make_region_name(const CString module_name, const CString name)
1112
{
1213
static std::string region;
13-
region = module_name;
14+
region = module_name.c_str();
1415
region += ":";
15-
region += name;
16+
region += name.c_str();
1617
return region;
1718
}
1819

19-
void region_begin(const std::string& function_name, const std::string& module,
20-
const std::string& file_name, const std::uint64_t line_number,
21-
const std::uintptr_t& identifier);
22-
void region_begin(const std::string& function_name, const std::string& module,
23-
const std::string& file_name, const std::uint64_t line_number);
20+
void region_begin(CString function_name, CString module, CString file_name,
21+
const std::uint64_t line_number, const std::uintptr_t& identifier);
22+
void region_begin(CString function_name, CString module, CString file_name,
23+
const std::uint64_t line_number);
2424

25-
void region_end(const std::string& function_name, const std::string& module,
26-
const std::uintptr_t& identifier);
27-
void region_end(const std::string& function_name, const std::string& module);
25+
void region_end(CString function_name, CString module, const std::uintptr_t& identifier);
26+
void region_end(CString function_name, CString module);
2827

2928
void region_end_error_handling(const std::string& region_name);
3029

‎src/scorepy/pythonHelpers.cpp‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,29 +4,32 @@
44

55
namespace scorepy
66
{
7-
const char* get_module_name(const PyFrameObject& frame)
7+
CString get_module_name(const PyFrameObject& frame)
88
{
99
PyObject* module_name = PyDict_GetItemString(frame.f_globals, "__name__");
1010
if (module_name)
11-
return PyUnicode_AsUTF8(module_name);
11+
return get_string_from_python(*module_name);
1212

1313
// this is a NUMPY special situation, see NEP-18, and Score-P issue #63
14-
// TODO: Use string_view/C-String to avoid creating 2 std::strings
15-
const char* filename = PyUnicode_AsUTF8(frame.f_code->co_filename);
16-
if (filename && (std::string(filename) == "<__array_function__ internals>"))
14+
const CString filename = get_string_from_python(*frame.f_code->co_filename);
15+
if (filename == "<__array_function__ internals>")
16+
{
1717
return "numpy.__array_function__";
18+
}
1819
else
20+
{
1921
return "unkown";
22+
}
2023
}
2124

22-
std::string get_file_name(const PyFrameObject& frame)
25+
CString get_file_name(const PyFrameObject& frame)
2326
{
2427
PyObject* filename = frame.f_code->co_filename;
2528
if (filename == Py_None)
2629
{
2730
return "None";
2831
}
2932
const std::string full_file_name = abspath(PyUnicode_AsUTF8(filename));
30-
return !full_file_name.empty() ? full_file_name : "ErrorPath";
33+
return !full_file_name.empty() ? CStrubg(full_file_name) : CString("ErrorPath");
3134
}
3235
} // namespace scorepy

‎src/scorepy/pythonHelpers.hpp‎

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
#pragma once
22

3+
#include "cstring.h"
34
#include <Python.h>
5+
#include <cassert>
46
#include <frameobject.h>
57
#include <string>
68
#include <type_traits>
@@ -71,11 +73,32 @@ auto cast_to_PyFunc(TFunc* func) -> detail::ReplaceArgsToPyObject_t<TFunc>*
7173
return reinterpret_cast<detail::ReplaceArgsToPyObject_t<TFunc>*>(func);
7274
}
7375

76+
inline CString get_string_from_python(PyObject& o)
77+
{
78+
Py_ssize_t len;
79+
const char* s = PyUnicode_AsUTF8AndSize(&o, &len);
80+
return CString(s, len);
81+
}
82+
83+
/// Pair of a C-String and it's length useful for PyArg_ParseTuple with 's#'
84+
/// Implicitely converts to CString.
85+
struct PythonCString
86+
{
87+
const char* s;
88+
Py_ssize_t l;
89+
operator CString() const
90+
{
91+
assert(s);
92+
return CString(s, l);
93+
}
94+
};
95+
7496
/// Return the module name the frame belongs to.
7597
/// The pointer is valid for the lifetime of the frame
76-
const char* get_module_name(const PyFrameObject& frame);
98+
CString get_module_name(const PyFrameObject& frame);
7799
/// Return the file name the frame belongs to
78-
std::string get_file_name(const PyFrameObject& frame);
100+
/// The returned CString is valid until the next call to this function
101+
CString get_file_name(const PyFrameObject& frame);
79102

80103
// Implementation details
81104
namespace detail

0 commit comments

Comments
 (0)