Skip to content

Commit

Permalink
Redo our ExpressionPiece system.
Browse files Browse the repository at this point in the history
While looking at the memory allocation patterns under OSX's Instruments,
I noticed that everything about ExpressionPieces spams the allocator. We
create a really large number of temporary objects, and because of the
way ObjRangeAdapter and friends work, we also clone these things for
every subcommand we send.

This changes ExpressionPiece from being an object hierarchy to one single
object implemented with a union underneath. We can now have vectors of
ExpressionPieces which should help locality. Allocating ExpressionPieces
and vector<>s of ExpressionPieces no longer shows up as a hotspot on
Instruments.
  • Loading branch information
eglaysher committed May 2, 2015
1 parent e1f3137 commit 727f68b
Show file tree
Hide file tree
Showing 19 changed files with 942 additions and 900 deletions.
31 changes: 16 additions & 15 deletions src/libreallive/bytecode.cc
Expand Up @@ -144,8 +144,8 @@ void PrintParameterString(std::ostream& oss,
// Take the binary stuff and try to get usefull, printable values.
const char* start = param.c_str();
try {
std::unique_ptr<ExpressionPiece> piece(GetData(start));
oss << piece->GetDebugString();
ExpressionPiece piece(GetData(start));
oss << piece.GetDebugString();
}
catch (libreallive::Error& e) {
// Any error throw here is a parse error.
Expand Down Expand Up @@ -364,7 +364,8 @@ void TextoutElement::RunOnMachine(RLMachine& machine) const {
// ExpressionElement
// -----------------------------------------------------------------------

ExpressionElement::ExpressionElement(const char* src) {
ExpressionElement::ExpressionElement(const char* src)
: parsed_expression_(invalid_expression_piece_t()) {
// Don't parse the expression, just isolate it.
const char* end = src;
end += NextToken(end);
Expand All @@ -375,31 +376,32 @@ ExpressionElement::ExpressionElement(const char* src) {
repr.assign(src, end);
}

ExpressionElement::ExpressionElement(const long val) {
ExpressionElement::ExpressionElement(const long val)
: parsed_expression_(invalid_expression_piece_t()) {
repr.resize(6, '$');
repr[1] = 0xff;
insert_i32(repr, 2, val);
}

ExpressionElement::ExpressionElement(const ExpressionElement& rhs)
: parsed_expression_(nullptr) {
: parsed_expression_(rhs.parsed_expression_) {
}

ExpressionElement::~ExpressionElement() {}

int ExpressionElement::GetValueOnly(RLMachine& machine) const {
const char* location = repr.c_str();
std::unique_ptr<ExpressionPiece> e(GetExpression(location));
return e->GetIntegerValue(machine);
ExpressionPiece e(GetExpression(location));
return e.GetIntegerValue(machine);
}

const ExpressionPiece& ExpressionElement::ParsedExpression() const {
if (parsed_expression_.get() == 0) {
if (!parsed_expression_.is_valid()) {
const char* location = repr.c_str();
parsed_expression_ = GetAssignment(location);
}

return *parsed_expression_;
return parsed_expression_;
}

void ExpressionElement::PrintSourceRepresentation(std::ostream& oss) const {
Expand Down Expand Up @@ -435,8 +437,7 @@ bool CommandElement::AreParametersParsed() const {
}

void CommandElement::SetParsedParameters(
ExpressionPiecesVector& parsedParameters) const {
parsed_parameters_.clear();
ExpressionPiecesVector parsedParameters) const {
parsed_parameters_ = std::move(parsedParameters);
}

Expand Down Expand Up @@ -619,8 +620,8 @@ std::string FunctionElement::GetSerializedCommand(RLMachine& machine) const {
rv.push_back('(');
for (string const& param : params) {
const char* data = param.c_str();
std::unique_ptr<ExpressionPiece> expression(GetData(data));
rv.append(expression->GetSerializedExpression(machine));
ExpressionPiece expression(GetData(data));
rv.append(expression.GetSerializedExpression(machine));
}
rv.push_back(')');
}
Expand Down Expand Up @@ -679,8 +680,8 @@ std::string SingleArgFunctionElement::GetSerializedCommand(RLMachine& machine)
rv.push_back(command[i]);
rv.push_back('(');
const char* data = arg_.c_str();
std::unique_ptr<ExpressionPiece> expression(GetData(data));
rv.append(expression->GetSerializedExpression(machine));
ExpressionPiece expression(GetData(data));
rv.append(expression.GetSerializedExpression(machine));
rv.push_back(')');
return rv;
}
Expand Down
6 changes: 3 additions & 3 deletions src/libreallive/bytecode.h
Expand Up @@ -212,7 +212,7 @@ class ExpressionElement : public BytecodeElement {

// Storage for the parsed expression so we only have to calculate
// it once (and so we can return it by const reference)
mutable std::unique_ptr<ExpressionPiece> parsed_expression_;
mutable ExpressionPiece parsed_expression_;
};

// Command elements.
Expand All @@ -236,7 +236,7 @@ class CommandElement : public BytecodeElement {
bool AreParametersParsed() const;

// Gets/Sets the cached parameters.
void SetParsedParameters(ExpressionPiecesVector& p) const;
void SetParsedParameters(ExpressionPiecesVector p) const;
const ExpressionPiecesVector& GetParsedParameters() const;

// Returns the number of parameters.
Expand All @@ -259,7 +259,7 @@ class CommandElement : public BytecodeElement {
static const int COMMAND_SIZE = 8;
unsigned char command[COMMAND_SIZE];

mutable std::vector<std::unique_ptr<ExpressionPiece> > parsed_parameters_;
mutable std::vector<ExpressionPiece> parsed_parameters_;
};

class SelectElement : public CommandElement {
Expand Down

0 comments on commit 727f68b

Please sign in to comment.