Skip to content

Commit d7cea2b

Browse files
authored
[lldb] Remove UnwindPlan::Row shared_ptrs (#132370)
The surrounding code doesn't use them anymore. This removes the internal usages. This patch makes the Rows actual values. An alternative would be to make them unique_ptrs. That would make vector resizes faster at the cost of more pointer chasing and heap fragmentation. I don't know which one is better so I picked the simpler option.
1 parent 99ec6f8 commit d7cea2b

File tree

2 files changed

+21
-38
lines changed

2 files changed

+21
-38
lines changed

lldb/include/lldb/Symbol/UnwindPlan.h

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -428,34 +428,17 @@ class UnwindPlan {
428428
bool m_unspecified_registers_are_undefined = false;
429429
}; // class Row
430430

431-
typedef std::shared_ptr<Row> RowSP;
432-
433431
UnwindPlan(lldb::RegisterKind reg_kind)
434432
: m_register_kind(reg_kind), m_return_addr_register(LLDB_INVALID_REGNUM),
435433
m_plan_is_sourced_from_compiler(eLazyBoolCalculate),
436434
m_plan_is_valid_at_all_instruction_locations(eLazyBoolCalculate),
437435
m_plan_is_for_signal_trap(eLazyBoolCalculate) {}
438436

439437
// Performs a deep copy of the plan, including all the rows (expensive).
440-
UnwindPlan(const UnwindPlan &rhs)
441-
: m_plan_valid_ranges(rhs.m_plan_valid_ranges),
442-
m_register_kind(rhs.m_register_kind),
443-
m_return_addr_register(rhs.m_return_addr_register),
444-
m_source_name(rhs.m_source_name),
445-
m_plan_is_sourced_from_compiler(rhs.m_plan_is_sourced_from_compiler),
446-
m_plan_is_valid_at_all_instruction_locations(
447-
rhs.m_plan_is_valid_at_all_instruction_locations),
448-
m_plan_is_for_signal_trap(rhs.m_plan_is_for_signal_trap),
449-
m_lsda_address(rhs.m_lsda_address),
450-
m_personality_func_addr(rhs.m_personality_func_addr) {
451-
m_row_list.reserve(rhs.m_row_list.size());
452-
for (const RowSP &row_sp : rhs.m_row_list)
453-
m_row_list.emplace_back(new Row(*row_sp));
454-
}
438+
UnwindPlan(const UnwindPlan &rhs) = default;
439+
UnwindPlan &operator=(const UnwindPlan &rhs) = default;
440+
455441
UnwindPlan(UnwindPlan &&rhs) = default;
456-
UnwindPlan &operator=(const UnwindPlan &rhs) {
457-
return *this = UnwindPlan(rhs); // NB: moving from a temporary (deep) copy
458-
}
459442
UnwindPlan &operator=(UnwindPlan &&) = default;
460443

461444
~UnwindPlan() = default;
@@ -486,7 +469,7 @@ class UnwindPlan {
486469
uint32_t GetInitialCFARegister() const {
487470
if (m_row_list.empty())
488471
return LLDB_INVALID_REGNUM;
489-
return m_row_list.front()->GetCFAValue().GetRegisterNumber();
472+
return m_row_list.front().GetCFAValue().GetRegisterNumber();
490473
}
491474

492475
// This UnwindPlan may not be valid at every address of the function span.
@@ -569,7 +552,7 @@ class UnwindPlan {
569552
}
570553

571554
private:
572-
std::vector<RowSP> m_row_list;
555+
std::vector<Row> m_row_list;
573556
std::vector<AddressRange> m_plan_valid_ranges;
574557
lldb::RegisterKind m_register_kind; // The RegisterKind these register numbers
575558
// are in terms of - will need to be

lldb/source/Symbol/UnwindPlan.cpp

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -391,29 +391,29 @@ bool UnwindPlan::Row::operator==(const UnwindPlan::Row &rhs) const {
391391
}
392392

393393
void UnwindPlan::AppendRow(Row row) {
394-
if (m_row_list.empty() || m_row_list.back()->GetOffset() != row.GetOffset())
395-
m_row_list.push_back(std::make_shared<Row>(std::move(row)));
394+
if (m_row_list.empty() || m_row_list.back().GetOffset() != row.GetOffset())
395+
m_row_list.push_back(std::move(row));
396396
else
397-
*m_row_list.back() = std::move(row);
397+
m_row_list.back() = std::move(row);
398398
}
399399

400400
struct RowLess {
401-
bool operator()(addr_t a, const UnwindPlan::RowSP &b) const {
402-
return a < b->GetOffset();
401+
bool operator()(addr_t a, const UnwindPlan::Row &b) const {
402+
return a < b.GetOffset();
403403
}
404-
bool operator()(const UnwindPlan::RowSP &a, addr_t b) const {
405-
return a->GetOffset() < b;
404+
bool operator()(const UnwindPlan::Row &a, addr_t b) const {
405+
return a.GetOffset() < b;
406406
}
407407
};
408408

409409
void UnwindPlan::InsertRow(Row row, bool replace_existing) {
410410
auto it = llvm::lower_bound(m_row_list, row.GetOffset(), RowLess());
411-
if (it == m_row_list.end() || it->get()->GetOffset() > row.GetOffset())
412-
m_row_list.insert(it, std::make_shared<Row>(std::move(row)));
411+
if (it == m_row_list.end() || it->GetOffset() > row.GetOffset())
412+
m_row_list.insert(it, std::move(row));
413413
else {
414-
assert(it->get()->GetOffset() == row.GetOffset());
414+
assert(it->GetOffset() == row.GetOffset());
415415
if (replace_existing)
416-
**it = std::move(row);
416+
*it = std::move(row);
417417
}
418418
}
419419

@@ -424,7 +424,7 @@ const UnwindPlan::Row *UnwindPlan::GetRowForFunctionOffset(int offset) const {
424424
return nullptr;
425425
// upper_bound returns the row strictly greater than our desired offset, which
426426
// means that the row before it is a match.
427-
return std::prev(it)->get();
427+
return &*std::prev(it);
428428
}
429429

430430
bool UnwindPlan::IsValidRowIndex(uint32_t idx) const {
@@ -433,7 +433,7 @@ bool UnwindPlan::IsValidRowIndex(uint32_t idx) const {
433433

434434
const UnwindPlan::Row *UnwindPlan::GetRowAtIndex(uint32_t idx) const {
435435
if (idx < m_row_list.size())
436-
return m_row_list[idx].get();
436+
return &m_row_list[idx];
437437
LLDB_LOG(GetLog(LLDBLog::Unwind),
438438
"error: UnwindPlan::GetRowAtIndex(idx = {0}) invalid index "
439439
"(number rows is {1})",
@@ -447,7 +447,7 @@ const UnwindPlan::Row *UnwindPlan::GetLastRow() const {
447447
"UnwindPlan::GetLastRow() when rows are empty");
448448
return nullptr;
449449
}
450-
return m_row_list.back().get();
450+
return &m_row_list.back();
451451
}
452452

453453
bool UnwindPlan::PlanValidAtAddress(Address addr) {
@@ -566,9 +566,9 @@ void UnwindPlan::Dump(Stream &s, Thread *thread, lldb::addr_t base_addr) const {
566566
range.Dump(&s, target_sp.get(), Address::DumpStyleSectionNameOffset);
567567
s.EOL();
568568
}
569-
for (const auto &[index, row_sp] : llvm::enumerate(m_row_list)) {
569+
for (const auto &[index, row] : llvm::enumerate(m_row_list)) {
570570
s.Format("row[{0}]: ", index);
571-
row_sp->Dump(s, this, thread, base_addr);
571+
row.Dump(s, this, thread, base_addr);
572572
s << "\n";
573573
}
574574
}

0 commit comments

Comments
 (0)