Skip to content

Commit 2e31125

Browse files
daboehmedaboehme
authored
Don't convert Variant to std::string for region filtering (LLNL#461)
1 parent e206acf commit 2e31125

File tree

4 files changed

+43
-33
lines changed

4 files changed

+43
-33
lines changed

‎src/caliper/RegionFilter.cpp

+10-4
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
#include "RegionFilter.h"
55

6+
#include "caliper/common/Variant.h"
7+
68
#include "../common/util/parse_util.h"
79

810
#include <iostream>
@@ -120,18 +122,22 @@ RegionFilter::parse_filter_config(std::istream& is)
120122
}
121123

122124
bool
123-
RegionFilter::match(const std::string& str, const Filter& filter)
125+
RegionFilter::match(const Variant& val, const Filter& filter)
124126
{
127+
// We assume val is a string. Variant strings aren't
128+
// 0-terminated, hence the more complicated comparisons
129+
const char* strp = static_cast<const char*>(val.data());
130+
125131
for (const auto &w : filter.startswith)
126-
if (str.compare(0, w.size(), w) == 0)
132+
if (val.size() >= w.size() && w.compare(0, w.size(), strp, w.size()) == 0)
127133
return true;
128134

129135
for (const auto &w : filter.match)
130-
if (str == w)
136+
if (val.size() == w.size() && w.compare(0, w.size(), strp, w.size()) == 0)
131137
return true;
132138

133139
for (const auto &r : filter.regex)
134-
if (std::regex_match(str, r) == true)
140+
if (std::regex_match(std::string(strp, val.size()), r) == true)
135141
return true;
136142

137143
return false;

‎src/caliper/RegionFilter.h

+6-4
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414
namespace cali
1515
{
1616

17+
class Variant;
18+
1719
/// \brief Implements region (string) filtering
1820
class RegionFilter
1921
{
@@ -28,7 +30,7 @@ class RegionFilter
2830

2931
static std::pair<std::shared_ptr<Filter>, std::string> parse_filter_config(std::istream& is);
3032

31-
static bool match(const std::string& str, const Filter&);
33+
static bool match(const Variant& val, const Filter&);
3234

3335
RegionFilter(std::shared_ptr<Filter> iflt, std::shared_ptr<Filter> eflt)
3436
: m_include_filters { iflt },
@@ -37,12 +39,12 @@ class RegionFilter
3739

3840
public:
3941

40-
bool pass(const char* str) const {
42+
bool pass(const Variant& val) const {
4143
if (m_exclude_filters)
42-
if (match(str, *m_exclude_filters))
44+
if (match(val, *m_exclude_filters))
4345
return false;
4446
if (m_include_filters)
45-
return match(str, *m_include_filters);
47+
return match(val, *m_include_filters);
4648

4749
return true;
4850
}

‎src/caliper/test/test_regionfilter.cpp

+24-22
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
#include "../RegionFilter.h"
22

3+
#include "caliper/common/Variant.h"
4+
35
#include <gtest/gtest.h>
46

57
using namespace cali;
@@ -14,14 +16,14 @@ TEST(RegionFilterTest, IncludeExclude) {
1416

1517
RegionFilter f(p.first);
1618

17-
EXPECT_TRUE (f.pass( " exact match" ));
18-
EXPECT_FALSE (f.pass( "some random string" ));
19-
EXPECT_TRUE (f.pass( "matchme" ));
20-
EXPECT_TRUE (f.pass( "starts with the magic word" ));
21-
EXPECT_FALSE (f.pass( "start exclude" ));
22-
EXPECT_TRUE (f.pass( "mpi_include_me" ));
23-
EXPECT_FALSE (f.pass( "mpi_exclude_me" ));
24-
EXPECT_FALSE (f.pass( "sta" ));
19+
EXPECT_TRUE (f.pass( Variant(" exact match" )));
20+
EXPECT_FALSE (f.pass( Variant("some random string" )));
21+
EXPECT_TRUE (f.pass( Variant("matchme" )));
22+
EXPECT_TRUE (f.pass( Variant("starts with the magic word" )));
23+
EXPECT_FALSE (f.pass( Variant("start exclude" )));
24+
EXPECT_TRUE (f.pass( Variant("mpi_include_me" )));
25+
EXPECT_FALSE (f.pass( Variant("mpi_exclude_me" )));
26+
EXPECT_FALSE (f.pass( Variant("sta" )));
2527
}
2628

2729
TEST(RegionFilterTest, IncludeOnly) {
@@ -34,12 +36,12 @@ TEST(RegionFilterTest, IncludeOnly) {
3436

3537
RegionFilter f(p.first);
3638

37-
EXPECT_TRUE (f.pass( " exact match" ));
38-
EXPECT_FALSE (f.pass( "some random string" ));
39-
EXPECT_TRUE (f.pass( "matchme" ));
40-
EXPECT_TRUE (f.pass( "starts with the magic word" ));
41-
EXPECT_TRUE (f.pass( "mpi_include_me" ));
42-
EXPECT_FALSE (f.pass( "sta" ));
39+
EXPECT_TRUE (f.pass( Variant(" exact match" )));
40+
EXPECT_FALSE (f.pass( Variant("some random string" )));
41+
EXPECT_TRUE (f.pass( Variant("matchme" )));
42+
EXPECT_TRUE (f.pass( Variant("starts with the magic word" )));
43+
EXPECT_TRUE (f.pass( Variant("mpi_include_me" )));
44+
EXPECT_FALSE (f.pass( Variant("sta" )));
4345
}
4446

4547
TEST(RegionFilterTest, ExcludeOnly) {
@@ -52,11 +54,11 @@ TEST(RegionFilterTest, ExcludeOnly) {
5254

5355
RegionFilter f(p.first);
5456

55-
EXPECT_TRUE (f.pass( "some random string" ));
56-
EXPECT_FALSE (f.pass( " exclude" ));
57-
EXPECT_TRUE (f.pass( "mpi_include_me" ));
58-
EXPECT_FALSE (f.pass( "mpi_exclude_me" ));
59-
EXPECT_TRUE (f.pass( "mpi" ));
57+
EXPECT_TRUE (f.pass( Variant("some random string" )));
58+
EXPECT_FALSE (f.pass( Variant(" exclude" )));
59+
EXPECT_TRUE (f.pass( Variant("mpi_include_me" )));
60+
EXPECT_FALSE (f.pass( Variant("mpi_exclude_me" )));
61+
EXPECT_TRUE (f.pass( Variant("mpi" )));
6062
}
6163

6264
TEST(RegionFilterTest, IncludeRegex) {
@@ -66,9 +68,9 @@ TEST(RegionFilterTest, IncludeRegex) {
6668

6769
RegionFilter f(p.first);
6870

69-
EXPECT_TRUE (f.pass( "i should match" ));
70-
EXPECT_FALSE (f.pass( "i should match not" ));
71-
EXPECT_FALSE (f.pass( "me neither" ));
71+
EXPECT_TRUE (f.pass( Variant("i should match" )));
72+
EXPECT_FALSE (f.pass( Variant("i should match not" )));
73+
EXPECT_FALSE (f.pass( Variant("me neither" )));
7274
}
7375

7476
TEST(RegionFilterTest, ParseError) {

‎src/services/event/EventTrigger.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ class EventTrigger
129129

130130
if (!marker_node)
131131
return;
132-
if (attr.type() == CALI_TYPE_STRING && !region_filter.pass(static_cast<const char*>(value.data())))
132+
if (attr.type() == CALI_TYPE_STRING && !region_filter.pass(value))
133133
return;
134134

135135
if (enable_snapshot_info) {
@@ -163,7 +163,7 @@ class EventTrigger
163163

164164
if (!marker_node)
165165
return;
166-
if (attr.type() == CALI_TYPE_STRING && !region_filter.pass(static_cast<const char*>(value.data())))
166+
if (attr.type() == CALI_TYPE_STRING && !region_filter.pass(value))
167167
return;
168168

169169
if (enable_snapshot_info) {
@@ -197,7 +197,7 @@ class EventTrigger
197197

198198
if (!marker_node)
199199
return;
200-
if (attr.type() == CALI_TYPE_STRING && !region_filter.pass(static_cast<const char*>(value.data())))
200+
if (attr.type() == CALI_TYPE_STRING && !region_filter.pass(value))
201201
return;
202202

203203
if (enable_snapshot_info) {

0 commit comments

Comments
 (0)