cmListFileCache: Refactor cmListFileBacktrace internals

Replace use of raw pointers and explicit reference counting with
`std::shared_ptr<>`.  Use a discriminated union to store either the
bottom level or a call/file context in each heap-allocated entry.
This avoids storing a copy of the bottom in every `cmListFileBacktrace`
instance and shrinks the structure to a single `shared_ptr`.
This commit is contained in:
Brad King
2018-09-24 09:21:29 -04:00
parent 1fe0d72eb6
commit 22aa6b67b4
2 changed files with 97 additions and 107 deletions
+82 -96
View File
@@ -11,6 +11,7 @@
#include <algorithm> #include <algorithm>
#include <assert.h> #include <assert.h>
#include <memory>
#include <sstream> #include <sstream>
cmCommandContext::cmCommandName& cmCommandContext::cmCommandName::operator=( cmCommandContext::cmCommandName& cmCommandContext::cmCommandName::operator=(
@@ -285,91 +286,66 @@ bool cmListFileParser::AddArgument(cmListFileLexer_Token* token,
return true; return true;
} }
struct cmListFileBacktrace::Entry : public cmListFileContext // We hold either the bottom scope of a directory or a call/file context.
// Discriminate these cases via the parent pointer.
struct cmListFileBacktrace::Entry
{ {
Entry(cmListFileContext const& lfc, Entry* up) Entry(cmStateSnapshot bottom)
: cmListFileContext(lfc) : Bottom(bottom)
, Up(up)
, RefCount(0)
{ {
if (this->Up) {
this->Up->Ref();
}
} }
Entry(std::shared_ptr<Entry const> parent, cmListFileContext lfc)
: Context(std::move(lfc))
, Parent(std::move(parent))
{
}
~Entry() ~Entry()
{ {
if (this->Up) { if (this->Parent) {
this->Up->Unref(); this->Context.~cmListFileContext();
} else {
this->Bottom.~cmStateSnapshot();
} }
} }
void Ref() { ++this->RefCount; }
void Unref() bool IsBottom() const { return !this->Parent; }
union
{ {
if (--this->RefCount == 0) { cmStateSnapshot Bottom;
delete this; cmListFileContext Context;
} };
} std::shared_ptr<Entry const> Parent;
Entry* Up;
unsigned int RefCount;
}; };
cmListFileBacktrace::cmListFileBacktrace(cmStateSnapshot const& bottom,
Entry* up,
cmListFileContext const& lfc)
: Bottom(bottom)
, Cur(new Entry(lfc, up))
{
assert(this->Bottom.IsValid());
this->Cur->Ref();
}
cmListFileBacktrace::cmListFileBacktrace(cmStateSnapshot const& bottom,
Entry* cur)
: Bottom(bottom)
, Cur(cur)
{
if (this->Cur) {
assert(this->Bottom.IsValid());
this->Cur->Ref();
}
}
cmListFileBacktrace::cmListFileBacktrace()
: Bottom()
, Cur(nullptr)
{
}
cmListFileBacktrace::cmListFileBacktrace(cmStateSnapshot const& snapshot) cmListFileBacktrace::cmListFileBacktrace(cmStateSnapshot const& snapshot)
: Bottom(snapshot.GetCallStackBottom()) : TopEntry(std::make_shared<Entry const>(snapshot.GetCallStackBottom()))
, Cur(nullptr)
{ {
} }
cmListFileBacktrace::cmListFileBacktrace(cmListFileBacktrace const& r) cmListFileBacktrace::cmListFileBacktrace(std::shared_ptr<Entry const> parent,
: Bottom(r.Bottom) cmListFileContext const& lfc)
, Cur(r.Cur) : TopEntry(std::make_shared<Entry const>(std::move(parent), lfc))
{ {
if (this->Cur) { }
assert(this->Bottom.IsValid());
this->Cur->Ref(); cmListFileBacktrace::cmListFileBacktrace(std::shared_ptr<Entry const> top)
} : TopEntry(std::move(top))
} {
}
cmListFileBacktrace& cmListFileBacktrace::operator=(
cmListFileBacktrace const& r) cmStateSnapshot cmListFileBacktrace::GetBottom() const
{ {
cmListFileBacktrace tmp(r); cmStateSnapshot bottom;
std::swap(this->Cur, tmp.Cur); if (Entry const* cur = this->TopEntry.get()) {
std::swap(this->Bottom, tmp.Bottom); while (Entry const* parent = cur->Parent.get()) {
return *this; cur = parent;
} }
bottom = cur->Bottom;
cmListFileBacktrace::~cmListFileBacktrace()
{
if (this->Cur) {
this->Cur->Unref();
} }
return bottom;
} }
cmListFileBacktrace cmListFileBacktrace::Push(std::string const& file) const cmListFileBacktrace cmListFileBacktrace::Push(std::string const& file) const
@@ -380,54 +356,61 @@ cmListFileBacktrace cmListFileBacktrace::Push(std::string const& file) const
// skipped during call stack printing. // skipped during call stack printing.
cmListFileContext lfc; cmListFileContext lfc;
lfc.FilePath = file; lfc.FilePath = file;
return cmListFileBacktrace(this->Bottom, this->Cur, lfc); return this->Push(lfc);
} }
cmListFileBacktrace cmListFileBacktrace::Push( cmListFileBacktrace cmListFileBacktrace::Push(
cmListFileContext const& lfc) const cmListFileContext const& lfc) const
{ {
return cmListFileBacktrace(this->Bottom, this->Cur, lfc); assert(this->TopEntry);
assert(!this->TopEntry->IsBottom() || this->TopEntry->Bottom.IsValid());
return cmListFileBacktrace(this->TopEntry, lfc);
} }
cmListFileBacktrace cmListFileBacktrace::Pop() const cmListFileBacktrace cmListFileBacktrace::Pop() const
{ {
assert(this->Cur); assert(this->TopEntry);
return cmListFileBacktrace(this->Bottom, this->Cur->Up); assert(!this->TopEntry->IsBottom());
return cmListFileBacktrace(this->TopEntry->Parent);
} }
cmListFileContext const& cmListFileBacktrace::Top() const cmListFileContext const& cmListFileBacktrace::Top() const
{ {
if (this->Cur) { assert(this->TopEntry);
return *this->Cur; return this->TopEntry->Context;
}
static cmListFileContext const empty;
return empty;
} }
void cmListFileBacktrace::PrintTitle(std::ostream& out) const void cmListFileBacktrace::PrintTitle(std::ostream& out) const
{ {
if (!this->Cur) { // The title exists only if we have a call on top of the bottom.
if (!this->TopEntry || this->TopEntry->IsBottom()) {
return; return;
} }
cmOutputConverter converter(this->Bottom); cmListFileContext lfc = this->TopEntry->Context;
cmListFileContext lfc = *this->Cur; cmStateSnapshot bottom = this->GetBottom();
if (!this->Bottom.GetState()->GetIsInTryCompile()) { cmOutputConverter converter(bottom);
if (!bottom.GetState()->GetIsInTryCompile()) {
lfc.FilePath = converter.ConvertToRelativePath( lfc.FilePath = converter.ConvertToRelativePath(
this->Bottom.GetState()->GetSourceDirectory(), lfc.FilePath); bottom.GetState()->GetSourceDirectory(), lfc.FilePath);
} }
out << (lfc.Line ? " at " : " in ") << lfc; out << (lfc.Line ? " at " : " in ") << lfc;
} }
void cmListFileBacktrace::PrintCallStack(std::ostream& out) const void cmListFileBacktrace::PrintCallStack(std::ostream& out) const
{ {
if (!this->Cur || !this->Cur->Up) { // The call stack exists only if we have at least two calls on top
// of the bottom.
if (!this->TopEntry || this->TopEntry->IsBottom() ||
this->TopEntry->Parent->IsBottom()) {
return; return;
} }
bool first = true; bool first = true;
cmOutputConverter converter(this->Bottom); cmStateSnapshot bottom = this->GetBottom();
for (Entry* i = this->Cur->Up; i; i = i->Up) { cmOutputConverter converter(bottom);
if (i->Name.empty()) { for (Entry const* cur = this->TopEntry->Parent.get(); !cur->IsBottom();
cur = cur->Parent.get()) {
if (cur->Context.Name.empty()) {
// Skip this whole-file scope. When we get here we already will // Skip this whole-file scope. When we get here we already will
// have printed a more-specific context within the file. // have printed a more-specific context within the file.
continue; continue;
@@ -436,10 +419,10 @@ void cmListFileBacktrace::PrintCallStack(std::ostream& out) const
first = false; first = false;
out << "Call Stack (most recent call first):\n"; out << "Call Stack (most recent call first):\n";
} }
cmListFileContext lfc = *i; cmListFileContext lfc = cur->Context;
if (!this->Bottom.GetState()->GetIsInTryCompile()) { if (!bottom.GetState()->GetIsInTryCompile()) {
lfc.FilePath = converter.ConvertToRelativePath( lfc.FilePath = converter.ConvertToRelativePath(
this->Bottom.GetState()->GetSourceDirectory(), lfc.FilePath); bottom.GetState()->GetSourceDirectory(), lfc.FilePath);
} }
out << " " << lfc << "\n"; out << " " << lfc << "\n";
} }
@@ -448,16 +431,19 @@ void cmListFileBacktrace::PrintCallStack(std::ostream& out) const
size_t cmListFileBacktrace::Depth() const size_t cmListFileBacktrace::Depth() const
{ {
size_t depth = 0; size_t depth = 0;
if (this->Cur == nullptr) { if (Entry const* cur = this->TopEntry.get()) {
return 0; for (; !cur->IsBottom(); cur = cur->Parent.get()) {
} ++depth;
}
for (Entry* i = this->Cur->Up; i; i = i->Up) {
depth++;
} }
return depth; return depth;
} }
bool cmListFileBacktrace::Empty() const
{
return !this->TopEntry || this->TopEntry->IsBottom();
}
std::ostream& operator<<(std::ostream& os, cmListFileContext const& lfc) std::ostream& operator<<(std::ostream& os, cmListFileContext const& lfc)
{ {
os << lfc.FilePath; os << lfc.FilePath;
+15 -11
View File
@@ -6,6 +6,7 @@
#include "cmConfigure.h" // IWYU pragma: keep #include "cmConfigure.h" // IWYU pragma: keep
#include <iosfwd> #include <iosfwd>
#include <memory> // IWYU pragma: keep
#include <stddef.h> #include <stddef.h>
#include <string> #include <string>
#include <vector> #include <vector>
@@ -115,18 +116,20 @@ public:
// Default-constructed backtrace may not be used until after // Default-constructed backtrace may not be used until after
// set via assignment from a backtrace constructed with a // set via assignment from a backtrace constructed with a
// valid snapshot. // valid snapshot.
cmListFileBacktrace(); cmListFileBacktrace() = default;
// Construct an empty backtrace whose bottom sits in the directory // Construct an empty backtrace whose bottom sits in the directory
// indicated by the given valid snapshot. // indicated by the given valid snapshot.
cmListFileBacktrace(cmStateSnapshot const& snapshot); cmListFileBacktrace(cmStateSnapshot const& snapshot);
// Backtraces may be copied and assigned as values. // Backtraces may be copied, moved, and assigned as values.
cmListFileBacktrace(cmListFileBacktrace const& r); cmListFileBacktrace(cmListFileBacktrace const&) = default;
cmListFileBacktrace& operator=(cmListFileBacktrace const& r); cmListFileBacktrace(cmListFileBacktrace&&) noexcept = default;
~cmListFileBacktrace(); cmListFileBacktrace& operator=(cmListFileBacktrace const&) = default;
cmListFileBacktrace& operator=(cmListFileBacktrace&&) noexcept = default;
~cmListFileBacktrace() = default;
cmStateSnapshot GetBottom() const { return this->Bottom; } cmStateSnapshot GetBottom() const;
// Get a backtrace with the given file scope added to the top. // Get a backtrace with the given file scope added to the top.
// May not be called until after construction with a valid snapshot. // May not be called until after construction with a valid snapshot.
@@ -153,14 +156,15 @@ public:
// Get the number of 'frames' in this backtrace // Get the number of 'frames' in this backtrace
size_t Depth() const; size_t Depth() const;
// Return true if this backtrace is empty.
bool Empty() const;
private: private:
struct Entry; struct Entry;
std::shared_ptr<Entry const> TopEntry;
cmStateSnapshot Bottom; cmListFileBacktrace(std::shared_ptr<Entry const> parent,
Entry* Cur;
cmListFileBacktrace(cmStateSnapshot const& bottom, Entry* up,
cmListFileContext const& lfc); cmListFileContext const& lfc);
cmListFileBacktrace(cmStateSnapshot const& bottom, Entry* cur); cmListFileBacktrace(std::shared_ptr<Entry const> top);
}; };
struct cmListFile struct cmListFile