Skip to content

Commit 5aa1d81

Browse files
jankratochviljankratochvil
committed
Code cleanup: Change FormattersContainer::KeyType from SP to rvalue
There is now std::shared_ptr passed around which is expensive for manycore CPUs. Most of the times (except for 3 cases) it is now just std::moved with no CPU locks needed. It also makes it possible to sort the keys (which is now not needed much after D66398). Differential revision: https://reviews.llvm.org/D67049 llvm-svn: 370863
1 parent 915f978 commit 5aa1d81

File tree

13 files changed

+106
-108
lines changed

13 files changed

+106
-108
lines changed

‎lldb/include/lldb/DataFormatters/FormattersContainer.h

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ template <typename KeyType, typename ValueType> class FormatMap {
6767
typedef typename ValueType::SharedPointer ValueSP;
6868
typedef std::map<KeyType, ValueSP> MapType;
6969
typedef typename MapType::iterator MapIterator;
70-
typedef std::function<bool(KeyType, const ValueSP &)> ForEachCallback;
70+
typedef std::function<bool(const KeyType &, const ValueSP &)> ForEachCallback;
7171

7272
FormatMap(IFormatChangeListener *lst)
7373
: m_map(), m_map_mutex(), listener(lst) {}
@@ -79,7 +79,7 @@ template <typename KeyType, typename ValueType> class FormatMap {
7979
entry->GetRevision() = 0;
8080

8181
std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
82-
m_map[name] = entry;
82+
m_map[std::move(name)] = entry;
8383
if (listener)
8484
listener->Changed();
8585
}
@@ -116,7 +116,7 @@ template <typename KeyType, typename ValueType> class FormatMap {
116116
std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
117117
MapIterator pos, end = m_map.end();
118118
for (pos = m_map.begin(); pos != end; pos++) {
119-
KeyType type = pos->first;
119+
const KeyType &type = pos->first;
120120
if (!callback(type, pos->second))
121121
break;
122122
}
@@ -138,6 +138,7 @@ template <typename KeyType, typename ValueType> class FormatMap {
138138
return iter->second;
139139
}
140140

141+
// If caller holds the mutex we could return a reference without copy ctor.
141142
KeyType GetKeyAtIndex(size_t index) {
142143
std::lock_guard<std::recursive_mutex> guard(m_map_mutex);
143144
MapIterator iter = m_map.begin();
@@ -182,8 +183,8 @@ template <typename KeyType, typename ValueType> class FormattersContainer {
182183
FormattersContainer(std::string name, IFormatChangeListener *lst)
183184
: m_format_map(lst), m_name(name) {}
184185

185-
void Add(const MapKeyType &type, const MapValueType &entry) {
186-
Add_Impl(type, entry, static_cast<KeyType *>(nullptr));
186+
void Add(MapKeyType type, const MapValueType &entry) {
187+
Add_Impl(std::move(type), entry, static_cast<KeyType *>(nullptr));
187188
}
188189

189190
bool Delete(ConstString type) {
@@ -233,9 +234,9 @@ template <typename KeyType, typename ValueType> class FormattersContainer {
233234

234235
DISALLOW_COPY_AND_ASSIGN(FormattersContainer);
235236

236-
void Add_Impl(const MapKeyType &type, const MapValueType &entry,
237-
lldb::RegularExpressionSP *dummy) {
238-
m_format_map.Add(type, entry);
237+
void Add_Impl(MapKeyType type, const MapValueType &entry,
238+
RegularExpression *dummy) {
239+
m_format_map.Add(std::move(type), entry);
239240
}
240241

241242
void Add_Impl(ConstString type, const MapValueType &entry,
@@ -247,12 +248,12 @@ template <typename KeyType, typename ValueType> class FormattersContainer {
247248
return m_format_map.Delete(type);
248249
}
249250

250-
bool Delete_Impl(ConstString type, lldb::RegularExpressionSP *dummy) {
251+
bool Delete_Impl(ConstString type, RegularExpression *dummy) {
251252
std::lock_guard<std::recursive_mutex> guard(m_format_map.mutex());
252253
MapIterator pos, end = m_format_map.map().end();
253254
for (pos = m_format_map.map().begin(); pos != end; pos++) {
254-
lldb::RegularExpressionSP regex = pos->first;
255-
if (type.GetStringRef() == regex->GetText()) {
255+
const RegularExpression &regex = pos->first;
256+
if (type.GetStringRef() == regex.GetText()) {
256257
m_format_map.map().erase(pos);
257258
if (m_format_map.listener)
258259
m_format_map.listener->Changed();
@@ -282,23 +283,22 @@ template <typename KeyType, typename ValueType> class FormattersContainer {
282283
}
283284

284285
lldb::TypeNameSpecifierImplSP
285-
GetTypeNameSpecifierAtIndex_Impl(size_t index,
286-
lldb::RegularExpressionSP *dummy) {
287-
lldb::RegularExpressionSP regex = m_format_map.GetKeyAtIndex(index);
288-
if (regex.get() == nullptr)
286+
GetTypeNameSpecifierAtIndex_Impl(size_t index, RegularExpression *dummy) {
287+
RegularExpression regex = m_format_map.GetKeyAtIndex(index);
288+
if (regex == RegularExpression())
289289
return lldb::TypeNameSpecifierImplSP();
290290
return lldb::TypeNameSpecifierImplSP(
291-
new TypeNameSpecifierImpl(regex->GetText().str().c_str(), true));
291+
new TypeNameSpecifierImpl(regex.GetText().str().c_str(), true));
292292
}
293293

294294
bool Get_Impl(ConstString key, MapValueType &value,
295-
lldb::RegularExpressionSP *dummy) {
295+
RegularExpression *dummy) {
296296
llvm::StringRef key_str = key.GetStringRef();
297297
std::lock_guard<std::recursive_mutex> guard(m_format_map.mutex());
298298
MapIterator pos, end = m_format_map.map().end();
299299
for (pos = m_format_map.map().begin(); pos != end; pos++) {
300-
lldb::RegularExpressionSP regex = pos->first;
301-
if (regex->Execute(key_str)) {
300+
const RegularExpression &regex = pos->first;
301+
if (regex.Execute(key_str)) {
302302
value = pos->second;
303303
return true;
304304
}
@@ -307,12 +307,12 @@ template <typename KeyType, typename ValueType> class FormattersContainer {
307307
}
308308

309309
bool GetExact_Impl(ConstString key, MapValueType &value,
310-
lldb::RegularExpressionSP *dummy) {
310+
RegularExpression *dummy) {
311311
std::lock_guard<std::recursive_mutex> guard(m_format_map.mutex());
312312
MapIterator pos, end = m_format_map.map().end();
313313
for (pos = m_format_map.map().begin(); pos != end; pos++) {
314-
lldb::RegularExpressionSP regex = pos->first;
315-
if (regex->GetText() == key.GetStringRef()) {
314+
const RegularExpression &regex = pos->first;
315+
if (regex.GetText() == key.GetStringRef()) {
316316
value = pos->second;
317317
return true;
318318
}

‎lldb/include/lldb/DataFormatters/TypeCategory.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ namespace lldb_private {
2626
template <typename FormatterImpl> class FormatterContainerPair {
2727
public:
2828
typedef FormattersContainer<ConstString, FormatterImpl> ExactMatchContainer;
29-
typedef FormattersContainer<lldb::RegularExpressionSP, FormatterImpl>
29+
typedef FormattersContainer<RegularExpression, FormatterImpl>
3030
RegexMatchContainer;
3131

3232
typedef typename ExactMatchContainer::MapType ExactMatchMap;

‎lldb/include/lldb/Target/Target.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -598,7 +598,7 @@ class Target : public std::enable_shared_from_this<Target>,
598598
const FileSpecList *containingModules,
599599
const FileSpecList *source_file_list,
600600
const std::unordered_set<std::string> &function_names,
601-
RegularExpression &source_regex, bool internal, bool request_hardware,
601+
RegularExpression source_regex, bool internal, bool request_hardware,
602602
LazyBool move_to_nearest_code);
603603

604604
// Use this to create a breakpoint from a load address
@@ -621,7 +621,7 @@ class Target : public std::enable_shared_from_this<Target>,
621621
// target setting, else we use the values passed in
622622
lldb::BreakpointSP CreateFuncRegexBreakpoint(
623623
const FileSpecList *containingModules,
624-
const FileSpecList *containingSourceFiles, RegularExpression &func_regexp,
624+
const FileSpecList *containingSourceFiles, RegularExpression func_regexp,
625625
lldb::LanguageType requested_language, LazyBool skip_prologue,
626626
bool internal, bool request_hardware);
627627

‎lldb/include/lldb/Utility/RegularExpression.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,14 @@ class RegularExpression {
7878
/// otherwise.
7979
llvm::Error GetError() const;
8080

81+
bool operator<(const RegularExpression &rhs) const {
82+
return GetText() < rhs.GetText();
83+
}
84+
85+
bool operator==(const RegularExpression &rhs) const {
86+
return GetText() == rhs.GetText();
87+
}
88+
8189
private:
8290
/// A copy of the original regular expression text.
8391
std::string m_regex_text;

‎lldb/source/API/SBTarget.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -960,8 +960,8 @@ lldb::SBBreakpoint SBTarget::BreakpointCreateByRegex(
960960
const LazyBool skip_prologue = eLazyBoolCalculate;
961961

962962
sb_bp = target_sp->CreateFuncRegexBreakpoint(
963-
module_list.get(), comp_unit_list.get(), regexp, symbol_language,
964-
skip_prologue, internal, hardware);
963+
module_list.get(), comp_unit_list.get(), std::move(regexp),
964+
symbol_language, skip_prologue, internal, hardware);
965965
}
966966

967967
return LLDB_RECORD_RESULT(sb_bp);
@@ -1061,8 +1061,8 @@ lldb::SBBreakpoint SBTarget::BreakpointCreateBySourceRegex(
10611061
}
10621062

10631063
sb_bp = target_sp->CreateSourceRegexBreakpoint(
1064-
module_list.get(), source_file_list.get(), func_names_set, regexp,
1065-
false, hardware, move_to_nearest_code);
1064+
module_list.get(), source_file_list.get(), func_names_set,
1065+
std::move(regexp), false, hardware, move_to_nearest_code);
10661066
}
10671067

10681068
return LLDB_RECORD_RESULT(sb_bp);

‎lldb/source/API/SBTypeCategory.cpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -364,8 +364,8 @@ bool SBTypeCategory::AddTypeFormat(SBTypeNameSpecifier type_name,
364364

365365
if (type_name.IsRegex())
366366
m_opaque_sp->GetRegexTypeFormatsContainer()->Add(
367-
lldb::RegularExpressionSP(new RegularExpression(
368-
llvm::StringRef::withNullAsEmpty(type_name.GetName()))),
367+
RegularExpression(
368+
llvm::StringRef::withNullAsEmpty(type_name.GetName())),
369369
format.GetSP());
370370
else
371371
m_opaque_sp->GetTypeFormatsContainer()->Add(
@@ -443,8 +443,8 @@ bool SBTypeCategory::AddTypeSummary(SBTypeNameSpecifier type_name,
443443

444444
if (type_name.IsRegex())
445445
m_opaque_sp->GetRegexTypeSummariesContainer()->Add(
446-
lldb::RegularExpressionSP(new RegularExpression(
447-
llvm::StringRef::withNullAsEmpty(type_name.GetName()))),
446+
RegularExpression(
447+
llvm::StringRef::withNullAsEmpty(type_name.GetName())),
448448
summary.GetSP());
449449
else
450450
m_opaque_sp->GetTypeSummariesContainer()->Add(
@@ -488,8 +488,8 @@ bool SBTypeCategory::AddTypeFilter(SBTypeNameSpecifier type_name,
488488

489489
if (type_name.IsRegex())
490490
m_opaque_sp->GetRegexTypeFiltersContainer()->Add(
491-
lldb::RegularExpressionSP(new RegularExpression(
492-
llvm::StringRef::withNullAsEmpty(type_name.GetName()))),
491+
RegularExpression(
492+
llvm::StringRef::withNullAsEmpty(type_name.GetName())),
493493
filter.GetSP());
494494
else
495495
m_opaque_sp->GetTypeFiltersContainer()->Add(
@@ -567,8 +567,8 @@ bool SBTypeCategory::AddTypeSynthetic(SBTypeNameSpecifier type_name,
567567

568568
if (type_name.IsRegex())
569569
m_opaque_sp->GetRegexTypeSyntheticsContainer()->Add(
570-
lldb::RegularExpressionSP(new RegularExpression(
571-
llvm::StringRef::withNullAsEmpty(type_name.GetName()))),
570+
RegularExpression(
571+
llvm::StringRef::withNullAsEmpty(type_name.GetName())),
572572
synth.GetSP());
573573
else
574574
m_opaque_sp->GetTypeSyntheticsContainer()->Add(

‎lldb/source/Breakpoint/BreakpointResolverName.cpp

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -126,9 +126,8 @@ BreakpointResolver *BreakpointResolverName::CreateFromStructuredData(
126126
success = options_dict.GetValueForKeyAsString(
127127
GetKey(OptionNames::RegexString), regex_text);
128128
if (success) {
129-
RegularExpression regex(regex_text);
130-
return new BreakpointResolverName(bkpt, regex, language, offset,
131-
skip_prologue);
129+
return new BreakpointResolverName(bkpt, RegularExpression(regex_text),
130+
language, offset, skip_prologue);
132131
} else {
133132
StructuredData::Array *names_array;
134133
success = options_dict.GetValueForKeyAsArray(

‎lldb/source/Commands/CommandObjectBreakpoint.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -668,7 +668,7 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
668668
}
669669

670670
bp_sp = target.CreateFuncRegexBreakpoint(
671-
&(m_options.m_modules), &(m_options.m_filenames), regexp,
671+
&(m_options.m_modules), &(m_options.m_filenames), std::move(regexp),
672672
m_options.m_language, m_options.m_skip_prologue, internal,
673673
m_options.m_hardware);
674674
}
@@ -699,7 +699,7 @@ class CommandObjectBreakpointSet : public CommandObjectParsed {
699699
}
700700
bp_sp = target.CreateSourceRegexBreakpoint(
701701
&(m_options.m_modules), &(m_options.m_filenames),
702-
m_options.m_source_regex_func_names, regexp, internal,
702+
m_options.m_source_regex_func_names, std::move(regexp), internal,
703703
m_options.m_hardware, m_options.m_move_to_nearest_code);
704704
} break;
705705
case eSetTypeException: {

‎lldb/source/Commands/CommandObjectType.cpp

Lines changed: 21 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -687,17 +687,18 @@ pointers to floats. Nor will it change the default display for Afloat and Bfloa
687687

688688
ConstString typeCS(arg_entry.ref);
689689
if (m_command_options.m_regex) {
690-
RegularExpressionSP typeRX(new RegularExpression(arg_entry.ref));
691-
if (!typeRX->IsValid()) {
690+
RegularExpression typeRX(arg_entry.ref);
691+
if (!typeRX.IsValid()) {
692692
result.AppendError(
693693
"regex format error (maybe this is not really a regex?)");
694694
result.SetStatus(eReturnStatusFailed);
695695
return false;
696696
}
697697
category_sp->GetRegexTypeSummariesContainer()->Delete(typeCS);
698-
category_sp->GetRegexTypeFormatsContainer()->Add(typeRX, entry);
698+
category_sp->GetRegexTypeFormatsContainer()->Add(std::move(typeRX),
699+
entry);
699700
} else
700-
category_sp->GetTypeFormatsContainer()->Add(typeCS, entry);
701+
category_sp->GetTypeFormatsContainer()->Add(std::move(typeCS), entry);
701702
}
702703

703704
result.SetStatus(eReturnStatusSuccessFinishNoResult);
@@ -1089,13 +1090,13 @@ class CommandObjectTypeFormatterList : public CommandObjectParsed {
10891090

10901091
foreach
10911092
.SetWithRegex([&result, &formatter_regex, &any_printed](
1092-
RegularExpressionSP regex_sp,
1093+
const RegularExpression &regex,
10931094
const FormatterSharedPointer &format_sp) -> bool {
10941095
if (formatter_regex) {
10951096
bool escape = true;
1096-
if (regex_sp->GetText() == formatter_regex->GetText()) {
1097+
if (regex.GetText() == formatter_regex->GetText()) {
10971098
escape = false;
1098-
} else if (formatter_regex->Execute(regex_sp->GetText())) {
1099+
} else if (formatter_regex->Execute(regex.GetText())) {
10991100
escape = false;
11001101
}
11011102

@@ -1105,7 +1106,7 @@ class CommandObjectTypeFormatterList : public CommandObjectParsed {
11051106

11061107
any_printed = true;
11071108
result.GetOutputStream().Printf("%s: %s\n",
1108-
regex_sp->GetText().str().c_str(),
1109+
regex.GetText().str().c_str(),
11091110
format_sp->GetDescription().c_str());
11101111
return true;
11111112
});
@@ -1619,24 +1620,24 @@ bool CommandObjectTypeSummaryAdd::AddSummary(ConstString type_name,
16191620
}
16201621

16211622
if (type == eRegexSummary) {
1622-
RegularExpressionSP typeRX(new RegularExpression(type_name.GetStringRef()));
1623-
if (!typeRX->IsValid()) {
1623+
RegularExpression typeRX(type_name.GetStringRef());
1624+
if (!typeRX.IsValid()) {
16241625
if (error)
16251626
error->SetErrorString(
16261627
"regex format error (maybe this is not really a regex?)");
16271628
return false;
16281629
}
16291630

16301631
category->GetRegexTypeSummariesContainer()->Delete(type_name);
1631-
category->GetRegexTypeSummariesContainer()->Add(typeRX, entry);
1632+
category->GetRegexTypeSummariesContainer()->Add(std::move(typeRX), entry);
16321633

16331634
return true;
16341635
} else if (type == eNamedSummary) {
16351636
// system named summaries do not exist (yet?)
16361637
DataVisualization::NamedSummaryFormats::Add(type_name, entry);
16371638
return true;
16381639
} else {
1639-
category->GetTypeSummariesContainer()->Add(type_name, entry);
1640+
category->GetTypeSummariesContainer()->Add(std::move(type_name), entry);
16401641
return true;
16411642
}
16421643
}
@@ -2353,20 +2354,20 @@ bool CommandObjectTypeSynthAdd::AddSynth(ConstString type_name,
23532354
}
23542355

23552356
if (type == eRegexSynth) {
2356-
RegularExpressionSP typeRX(new RegularExpression(type_name.GetStringRef()));
2357-
if (!typeRX->IsValid()) {
2357+
RegularExpression typeRX(type_name.GetStringRef());
2358+
if (!typeRX.IsValid()) {
23582359
if (error)
23592360
error->SetErrorString(
23602361
"regex format error (maybe this is not really a regex?)");
23612362
return false;
23622363
}
23632364

23642365
category->GetRegexTypeSyntheticsContainer()->Delete(type_name);
2365-
category->GetRegexTypeSyntheticsContainer()->Add(typeRX, entry);
2366+
category->GetRegexTypeSyntheticsContainer()->Add(std::move(typeRX), entry);
23662367

23672368
return true;
23682369
} else {
2369-
category->GetTypeSyntheticsContainer()->Add(type_name, entry);
2370+
category->GetTypeSyntheticsContainer()->Add(std::move(type_name), entry);
23702371
return true;
23712372
}
23722373
}
@@ -2479,21 +2480,20 @@ class CommandObjectTypeFilterAdd : public CommandObjectParsed {
24792480
}
24802481

24812482
if (type == eRegexFilter) {
2482-
RegularExpressionSP typeRX(
2483-
new RegularExpression(type_name.GetStringRef()));
2484-
if (!typeRX->IsValid()) {
2483+
RegularExpression typeRX(type_name.GetStringRef());
2484+
if (!typeRX.IsValid()) {
24852485
if (error)
24862486
error->SetErrorString(
24872487
"regex format error (maybe this is not really a regex?)");
24882488
return false;
24892489
}
24902490

24912491
category->GetRegexTypeFiltersContainer()->Delete(type_name);
2492-
category->GetRegexTypeFiltersContainer()->Add(typeRX, entry);
2492+
category->GetRegexTypeFiltersContainer()->Add(std::move(typeRX), entry);
24932493

24942494
return true;
24952495
} else {
2496-
category->GetTypeFiltersContainer()->Add(type_name, entry);
2496+
category->GetTypeFiltersContainer()->Add(std::move(type_name), entry);
24972497
return true;
24982498
}
24992499
}

0 commit comments

Comments
 (0)