Skip to content

Commit

Permalink
[Courgette] Refactor: Add AssemblyProgram::DispatchInstructionLabels(…
Browse files Browse the repository at this point in the history
…) to hide InstructionVector usage.

To reduce Courgette memory usage (and possibly solve installer crashes),
we plan to make AssemblyProgram store instructions more efficiently. We
start by hiding AssemblyProgram's InstructionVector usage from
AdjustmentMethod and AdjustmentMethod2.

Previously AdjustmentMethod[2] get AssemblyProgram's instruction vector,
and loop over it to extract labels from abs32/rel32 instructions. This
CL moves the loop into AssemblyProgram::DispatchInstructionLabels().
Callers now specify a map of handlers, without needing to know how
instructions are stored in AssemblyProgram.

Review-Url: https://codereview.chromium.org/2457133002
Cr-Commit-Position: refs/heads/master@{#428522}
  • Loading branch information
samuelhuang authored and Commit bot committed Oct 28, 2016
1 parent 0d496dc commit 99a5a8c
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 58 deletions.
23 changes: 13 additions & 10 deletions courgette/adjustment_method.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <string>
#include <vector>

#include "base/bind.h"
#include "base/logging.h"
#include "base/macros.h"
#include "base/strings/string_number_conversions.h"
Expand Down Expand Up @@ -592,17 +593,19 @@ class GraphAdjuster : public AdjustmentMethod {
}

private:

void CollectTraces(const AssemblyProgram* program, Trace* abs32, Trace* rel32,
bool is_model) {
const InstructionVector& instructions = program->instructions();
for (size_t i = 0; i < instructions.size(); ++i) {
Instruction* instruction = instructions[i];
if (Label* label = program->InstructionAbs32Label(instruction))
ReferenceLabel(abs32, label, is_model);
if (Label* label = program->InstructionRel32Label(instruction))
ReferenceLabel(rel32, label, is_model);
}
AssemblyProgram::LabelHandler abs32_handler =
base::Bind(&GraphAdjuster::ReferenceLabel, base::Unretained(this),
abs32, is_model);
AssemblyProgram::LabelHandler rel32_handler =
base::Bind(&GraphAdjuster::ReferenceLabel, base::Unretained(this),
rel32, is_model);

program->HandleInstructionLabels({{ABS32, abs32_handler},
{REL32, rel32_handler},
{REL32ARM, rel32_handler}});

// TODO(sra): we could simply append all the labels in index order to
// incorporate some costing for entropy (bigger deltas) that will be
// introduced into the label address table by non-monotonic ordering. This
Expand Down Expand Up @@ -634,7 +637,7 @@ class GraphAdjuster : public AdjustmentMethod {
}
}

void ReferenceLabel(Trace* trace, Label* label, bool is_model) {
void ReferenceLabel(Trace* trace, bool is_model, Label* label) {
trace->push_back(
MakeLabelInfo(label, is_model, static_cast<uint32_t>(trace->size())));
}
Expand Down
24 changes: 13 additions & 11 deletions courgette/adjustment_method_2.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include <string>
#include <vector>

#include "base/bind.h"
#include "base/format_macros.h"
#include "base/logging.h"
#include "base/macros.h"
Expand Down Expand Up @@ -1252,16 +1253,17 @@ class Adjuster : public AdjustmentMethod {
void CollectTraces(const AssemblyProgram* program, Trace* abs32, Trace* rel32,
bool is_model) {
label_info_maker_.ResetDebugLabel();
const InstructionVector& instructions = program->instructions();
for (size_t i = 0; i < instructions.size(); ++i) {
Instruction* instruction = instructions[i];
if (Label* label = program->InstructionAbs32Label(instruction))
ReferenceLabel(abs32, label, is_model);
if (Label* label = program->InstructionAbs64Label(instruction))
ReferenceLabel(abs32, label, is_model);
if (Label* label = program->InstructionRel32Label(instruction))
ReferenceLabel(rel32, label, is_model);
}

AssemblyProgram::LabelHandler abs32_handler = base::Bind(
&Adjuster::ReferenceLabel, base::Unretained(this), abs32, is_model);
AssemblyProgram::LabelHandler rel32_handler = base::Bind(
&Adjuster::ReferenceLabel, base::Unretained(this), rel32, is_model);

program->HandleInstructionLabels({{ABS32, abs32_handler},
{ABS64, abs32_handler},
{REL32, rel32_handler},
{REL32ARM, rel32_handler}});

// TODO(sra): we could simply append all the labels in index order to
// incorporate some costing for entropy (bigger deltas) that will be
// introduced into the label address table by non-monotonic ordering. This
Expand All @@ -1277,7 +1279,7 @@ class Adjuster : public AdjustmentMethod {
<< (base::Time::Now() - start_time).InSecondsF();
}

void ReferenceLabel(Trace* trace, Label* label, bool is_model) {
void ReferenceLabel(Trace* trace, bool is_model, Label* label) {
trace->push_back(label_info_maker_.MakeLabelInfo(
label, is_model, static_cast<uint32_t>(trace->size())));
}
Expand Down
29 changes: 8 additions & 21 deletions courgette/assembly_program.cc
Original file line number Diff line number Diff line change
Expand Up @@ -217,28 +217,15 @@ Label* AssemblyProgram::FindRel32Label(RVA rva) {
return rel32_label_manager_.Find(rva);
}

Label* AssemblyProgram::InstructionAbs32Label(
const Instruction* instruction) const {
if (instruction->op() == ABS32)
return static_cast<const InstructionWithLabel*>(instruction)->label();
return NULL;
}

Label* AssemblyProgram::InstructionAbs64Label(
const Instruction* instruction) const {
if (instruction->op() == ABS64)
return static_cast<const InstructionWithLabel*>(instruction)->label();
return NULL;
}

Label* AssemblyProgram::InstructionRel32Label(
const Instruction* instruction) const {
if (instruction->op() == REL32 || instruction->op() == REL32ARM) {
Label* label =
static_cast<const InstructionWithLabel*>(instruction)->label();
return label;
void AssemblyProgram::HandleInstructionLabels(
const AssemblyProgram::LabelHandlerMap& handler_map) const {
for (const Instruction* instruction : instructions_) {
LabelHandlerMap::const_iterator it = handler_map.find(instruction->op());
if (it != handler_map.end()) {
it->second.Run(
static_cast<const InstructionWithLabel*>(instruction)->label());
}
}
return NULL;
}

CheckBool AssemblyProgram::Emit(ScopedInstruction instruction) {
Expand Down
25 changes: 9 additions & 16 deletions courgette/assembly_program.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <set>
#include <vector>

#include "base/bind.h"
#include "base/macros.h"
#include "base/memory/free_deleter.h"
#include "courgette/courgette.h"
Expand Down Expand Up @@ -79,6 +80,9 @@ typedef NoThrowBuffer<Instruction*> InstructionVector;
//
class AssemblyProgram {
public:
using LabelHandler = base::Callback<void(Label*)>;
using LabelHandlerMap = std::map<OP, LabelHandler>;

explicit AssemblyProgram(ExecutableType kind);
~AssemblyProgram();

Expand Down Expand Up @@ -142,22 +146,11 @@ class AssemblyProgram {

std::unique_ptr<EncodedProgram> Encode() const;

// Accessor for instruction list.
const InstructionVector& instructions() const {
return instructions_;
}

// Returns the label if the instruction contains an absolute 32-bit address,
// otherwise returns NULL.
Label* InstructionAbs32Label(const Instruction* instruction) const;

// Returns the label if the instruction contains an absolute 64-bit address,
// otherwise returns NULL.
Label* InstructionAbs64Label(const Instruction* instruction) const;

// Returns the label if the instruction contains a rel32 offset,
// otherwise returns NULL.
Label* InstructionRel32Label(const Instruction* instruction) const;
// For each |instruction| in |instructions_|, looks up its opcode from
// |handler_map| for a handler. If a handler exists, invoke it by passing the
// |instruction|'s label. We assume that |handler_map| has correct keys, i.e.,
// opcodes for an instruction that have label.
void HandleInstructionLabels(const LabelHandlerMap& handler_map) const;

private:
using ScopedInstruction =
Expand Down

0 comments on commit 99a5a8c

Please sign in to comment.