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

View File

@@ -11,6 +11,7 @@
#include <algorithm>
#include <assert.h>
#include <memory>
#include <sstream>
cmCommandContext::cmCommandName& cmCommandContext::cmCommandName::operator=(
@@ -285,91 +286,66 @@ bool cmListFileParser::AddArgument(cmListFileLexer_Token* token,
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)
: cmListFileContext(lfc)
, Up(up)
, RefCount(0)
Entry(cmStateSnapshot bottom)
: Bottom(bottom)
{
if (this->Up) {
this->Up->Ref();
}
}
Entry(std::shared_ptr<Entry const> parent, cmListFileContext lfc)
: Context(std::move(lfc))
, Parent(std::move(parent))
{
}
~Entry()
{
if (this->Up) {
this->Up->Unref();
if (this->Parent) {
this->Context.~cmListFileContext();
} else {
this->Bottom.~cmStateSnapshot();
}
}
void Ref() { ++this->RefCount; }
void Unref()
bool IsBottom() const { return !this->Parent; }
union
{
if (--this->RefCount == 0) {
delete this;
}
}
Entry* Up;
unsigned int RefCount;
cmStateSnapshot Bottom;
cmListFileContext Context;
};
std::shared_ptr<Entry const> Parent;
};
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)
: Bottom(snapshot.GetCallStackBottom())
, Cur(nullptr)
: TopEntry(std::make_shared<Entry const>(snapshot.GetCallStackBottom()))
{
}
cmListFileBacktrace::cmListFileBacktrace(cmListFileBacktrace const& r)
: Bottom(r.Bottom)
, Cur(r.Cur)
cmListFileBacktrace::cmListFileBacktrace(std::shared_ptr<Entry const> parent,
cmListFileContext const& lfc)
: TopEntry(std::make_shared<Entry const>(std::move(parent), lfc))
{
if (this->Cur) {
assert(this->Bottom.IsValid());
this->Cur->Ref();
}
}
cmListFileBacktrace& cmListFileBacktrace::operator=(
cmListFileBacktrace const& r)
{
cmListFileBacktrace tmp(r);
std::swap(this->Cur, tmp.Cur);
std::swap(this->Bottom, tmp.Bottom);
return *this;
}
cmListFileBacktrace::~cmListFileBacktrace()
{
if (this->Cur) {
this->Cur->Unref();
}
cmListFileBacktrace::cmListFileBacktrace(std::shared_ptr<Entry const> top)
: TopEntry(std::move(top))
{
}
cmStateSnapshot cmListFileBacktrace::GetBottom() const
{
cmStateSnapshot bottom;
if (Entry const* cur = this->TopEntry.get()) {
while (Entry const* parent = cur->Parent.get()) {
cur = parent;
}
bottom = cur->Bottom;
}
return bottom;
}
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.
cmListFileContext lfc;
lfc.FilePath = file;
return cmListFileBacktrace(this->Bottom, this->Cur, lfc);
return this->Push(lfc);
}
cmListFileBacktrace cmListFileBacktrace::Push(
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
{
assert(this->Cur);
return cmListFileBacktrace(this->Bottom, this->Cur->Up);
assert(this->TopEntry);
assert(!this->TopEntry->IsBottom());
return cmListFileBacktrace(this->TopEntry->Parent);
}
cmListFileContext const& cmListFileBacktrace::Top() const
{
if (this->Cur) {
return *this->Cur;
}
static cmListFileContext const empty;
return empty;
assert(this->TopEntry);
return this->TopEntry->Context;
}
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;
}
cmOutputConverter converter(this->Bottom);
cmListFileContext lfc = *this->Cur;
if (!this->Bottom.GetState()->GetIsInTryCompile()) {
cmListFileContext lfc = this->TopEntry->Context;
cmStateSnapshot bottom = this->GetBottom();
cmOutputConverter converter(bottom);
if (!bottom.GetState()->GetIsInTryCompile()) {
lfc.FilePath = converter.ConvertToRelativePath(
this->Bottom.GetState()->GetSourceDirectory(), lfc.FilePath);
bottom.GetState()->GetSourceDirectory(), lfc.FilePath);
}
out << (lfc.Line ? " at " : " in ") << lfc;
}
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;
}
bool first = true;
cmOutputConverter converter(this->Bottom);
for (Entry* i = this->Cur->Up; i; i = i->Up) {
if (i->Name.empty()) {
cmStateSnapshot bottom = this->GetBottom();
cmOutputConverter converter(bottom);
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
// have printed a more-specific context within the file.
continue;
@@ -436,10 +419,10 @@ void cmListFileBacktrace::PrintCallStack(std::ostream& out) const
first = false;
out << "Call Stack (most recent call first):\n";
}
cmListFileContext lfc = *i;
if (!this->Bottom.GetState()->GetIsInTryCompile()) {
cmListFileContext lfc = cur->Context;
if (!bottom.GetState()->GetIsInTryCompile()) {
lfc.FilePath = converter.ConvertToRelativePath(
this->Bottom.GetState()->GetSourceDirectory(), lfc.FilePath);
bottom.GetState()->GetSourceDirectory(), lfc.FilePath);
}
out << " " << lfc << "\n";
}
@@ -448,16 +431,19 @@ void cmListFileBacktrace::PrintCallStack(std::ostream& out) const
size_t cmListFileBacktrace::Depth() const
{
size_t depth = 0;
if (this->Cur == nullptr) {
return 0;
}
for (Entry* i = this->Cur->Up; i; i = i->Up) {
depth++;
if (Entry const* cur = this->TopEntry.get()) {
for (; !cur->IsBottom(); cur = cur->Parent.get()) {
++depth;
}
}
return depth;
}
bool cmListFileBacktrace::Empty() const
{
return !this->TopEntry || this->TopEntry->IsBottom();
}
std::ostream& operator<<(std::ostream& os, cmListFileContext const& lfc)
{
os << lfc.FilePath;

View File

@@ -6,6 +6,7 @@
#include "cmConfigure.h" // IWYU pragma: keep
#include <iosfwd>
#include <memory> // IWYU pragma: keep
#include <stddef.h>
#include <string>
#include <vector>
@@ -115,18 +116,20 @@ public:
// Default-constructed backtrace may not be used until after
// set via assignment from a backtrace constructed with a
// valid snapshot.
cmListFileBacktrace();
cmListFileBacktrace() = default;
// Construct an empty backtrace whose bottom sits in the directory
// indicated by the given valid snapshot.
cmListFileBacktrace(cmStateSnapshot const& snapshot);
// Backtraces may be copied and assigned as values.
cmListFileBacktrace(cmListFileBacktrace const& r);
cmListFileBacktrace& operator=(cmListFileBacktrace const& r);
~cmListFileBacktrace();
// Backtraces may be copied, moved, and assigned as values.
cmListFileBacktrace(cmListFileBacktrace const&) = default;
cmListFileBacktrace(cmListFileBacktrace&&) noexcept = default;
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.
// May not be called until after construction with a valid snapshot.
@@ -153,14 +156,15 @@ public:
// Get the number of 'frames' in this backtrace
size_t Depth() const;
// Return true if this backtrace is empty.
bool Empty() const;
private:
struct Entry;
cmStateSnapshot Bottom;
Entry* Cur;
cmListFileBacktrace(cmStateSnapshot const& bottom, Entry* up,
std::shared_ptr<Entry const> TopEntry;
cmListFileBacktrace(std::shared_ptr<Entry const> parent,
cmListFileContext const& lfc);
cmListFileBacktrace(cmStateSnapshot const& bottom, Entry* cur);
cmListFileBacktrace(std::shared_ptr<Entry const> top);
};
struct cmListFile