[codegen] Move SourcePositionTableBuilder to Zone
The SourcePositionTableBuilder unconditionally allocates heap memory for every Liftoff compilation. This shows up with 1-2% of compilation time in profiles. Hence move the vector contained in the SourcePositionTableBuilder into the compilation zone. Such a zone already exists for both Liftoff and TurboFan, so we can easily save allocations this way. R=thibaudm@chromium.org Bug: v8:10576 Change-Id: Ia83d05cc8c36c775ebff6ec2064e9c3f8cc4d384 Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2224221 Commit-Queue: Clemens Backes <clemensb@chromium.org> Reviewed-by: Thibaud Michaud <thibaudm@chromium.org> Reviewed-by: Ross McIlroy <rmcilroy@chromium.org> Cr-Commit-Position: refs/heads/master@{#68112}
This commit is contained in:
parent
f62fc2e1a8
commit
447ad5ccbe
@ -49,10 +49,10 @@ void SubtractFromEntry(PositionTableEntry* value,
|
||||
|
||||
// Helper: Encode an integer.
|
||||
template <typename T>
|
||||
void EncodeInt(std::vector<byte>* bytes, T value) {
|
||||
void EncodeInt(ZoneVector<byte>* bytes, T value) {
|
||||
using unsigned_type = typename std::make_unsigned<T>::type;
|
||||
// Zig-zag encoding.
|
||||
static const int kShift = sizeof(T) * kBitsPerByte - 1;
|
||||
static constexpr int kShift = sizeof(T) * kBitsPerByte - 1;
|
||||
value = ((static_cast<unsigned_type>(value) << 1) ^ (value >> kShift));
|
||||
DCHECK_GE(value, 0);
|
||||
unsigned_type encoded = static_cast<unsigned_type>(value);
|
||||
@ -67,7 +67,7 @@ void EncodeInt(std::vector<byte>* bytes, T value) {
|
||||
}
|
||||
|
||||
// Encode a PositionTableEntry.
|
||||
void EncodeEntry(std::vector<byte>* bytes, const PositionTableEntry& entry) {
|
||||
void EncodeEntry(ZoneVector<byte>* bytes, const PositionTableEntry& entry) {
|
||||
// We only accept ascending code offsets.
|
||||
DCHECK_GE(entry.code_offset, 0);
|
||||
// Since code_offset is not negative, we use sign to encode is_statement.
|
||||
@ -115,7 +115,7 @@ Vector<const byte> VectorFromByteArray(ByteArray byte_array) {
|
||||
}
|
||||
|
||||
#ifdef ENABLE_SLOW_DCHECKS
|
||||
void CheckTableEquals(const std::vector<PositionTableEntry>& raw_entries,
|
||||
void CheckTableEquals(const ZoneVector<PositionTableEntry>& raw_entries,
|
||||
SourcePositionTableIterator* encoded) {
|
||||
// Brute force testing: Record all positions and decode
|
||||
// the entire table to verify they are identical.
|
||||
@ -133,8 +133,14 @@ void CheckTableEquals(const std::vector<PositionTableEntry>& raw_entries,
|
||||
} // namespace
|
||||
|
||||
SourcePositionTableBuilder::SourcePositionTableBuilder(
|
||||
SourcePositionTableBuilder::RecordingMode mode)
|
||||
: mode_(mode), previous_() {}
|
||||
Zone* zone, SourcePositionTableBuilder::RecordingMode mode)
|
||||
: mode_(mode),
|
||||
bytes_(zone),
|
||||
#ifdef ENABLE_SLOW_DCHECKS
|
||||
raw_entries_(zone),
|
||||
#endif
|
||||
previous_() {
|
||||
}
|
||||
|
||||
void SourcePositionTableBuilder::AddPosition(size_t code_offset,
|
||||
SourcePosition source_position,
|
||||
|
@ -49,7 +49,7 @@ class V8_EXPORT_PRIVATE SourcePositionTableBuilder {
|
||||
};
|
||||
|
||||
explicit SourcePositionTableBuilder(
|
||||
RecordingMode mode = RECORD_SOURCE_POSITIONS);
|
||||
Zone* zone, RecordingMode mode = RECORD_SOURCE_POSITIONS);
|
||||
|
||||
void AddPosition(size_t code_offset, SourcePosition source_position,
|
||||
bool is_statement);
|
||||
@ -66,9 +66,9 @@ class V8_EXPORT_PRIVATE SourcePositionTableBuilder {
|
||||
void AddEntry(const PositionTableEntry& entry);
|
||||
|
||||
RecordingMode mode_;
|
||||
std::vector<byte> bytes_;
|
||||
ZoneVector<byte> bytes_;
|
||||
#ifdef ENABLE_SLOW_DCHECKS
|
||||
std::vector<PositionTableEntry> raw_entries_;
|
||||
ZoneVector<PositionTableEntry> raw_entries_;
|
||||
#endif
|
||||
PositionTableEntry previous_; // Previously written entry, to compute delta.
|
||||
};
|
||||
|
@ -55,19 +55,20 @@ CodeGenerator::CodeGenerator(
|
||||
frame_access_state_(nullptr),
|
||||
linkage_(linkage),
|
||||
instructions_(instructions),
|
||||
unwinding_info_writer_(zone()),
|
||||
unwinding_info_writer_(codegen_zone),
|
||||
info_(info),
|
||||
labels_(zone()->NewArray<Label>(instructions->InstructionBlockCount())),
|
||||
labels_(
|
||||
codegen_zone->NewArray<Label>(instructions->InstructionBlockCount())),
|
||||
current_block_(RpoNumber::Invalid()),
|
||||
start_source_position_(start_source_position),
|
||||
current_source_position_(SourcePosition::Unknown()),
|
||||
tasm_(isolate, options, CodeObjectRequired::kNo, std::move(buffer)),
|
||||
resolver_(this),
|
||||
safepoints_(zone()),
|
||||
handlers_(zone()),
|
||||
deoptimization_exits_(zone()),
|
||||
deoptimization_literals_(zone()),
|
||||
translations_(zone()),
|
||||
safepoints_(codegen_zone),
|
||||
handlers_(codegen_zone),
|
||||
deoptimization_exits_(codegen_zone),
|
||||
deoptimization_literals_(codegen_zone),
|
||||
translations_(codegen_zone),
|
||||
max_unoptimized_frame_height_(max_unoptimized_frame_height),
|
||||
max_pushed_argument_count_(max_pushed_argument_count),
|
||||
caller_registers_saved_(false),
|
||||
@ -77,12 +78,12 @@ CodeGenerator::CodeGenerator(
|
||||
osr_pc_offset_(-1),
|
||||
optimized_out_literal_id_(-1),
|
||||
source_position_table_builder_(
|
||||
SourcePositionTableBuilder::RECORD_SOURCE_POSITIONS),
|
||||
protected_instructions_(zone()),
|
||||
codegen_zone, SourcePositionTableBuilder::RECORD_SOURCE_POSITIONS),
|
||||
protected_instructions_(codegen_zone),
|
||||
result_(kSuccess),
|
||||
poisoning_level_(poisoning_level),
|
||||
block_starts_(zone()),
|
||||
instr_starts_(zone()) {
|
||||
block_starts_(codegen_zone),
|
||||
instr_starts_(codegen_zone) {
|
||||
for (int i = 0; i < instructions->InstructionBlockCount(); ++i) {
|
||||
new (&labels_[i]) Label;
|
||||
}
|
||||
|
@ -1003,7 +1003,8 @@ bool CanRestartFrame(
|
||||
|
||||
void TranslateSourcePositionTable(Isolate* isolate, Handle<BytecodeArray> code,
|
||||
const std::vector<SourceChangeRange>& diffs) {
|
||||
SourcePositionTableBuilder builder;
|
||||
Zone zone(isolate->allocator(), ZONE_NAME);
|
||||
SourcePositionTableBuilder builder(&zone);
|
||||
|
||||
Handle<ByteArray> source_position_table(code->SourcePositionTable(), isolate);
|
||||
for (SourcePositionTableIterator iterator(*source_position_table);
|
||||
|
@ -27,7 +27,7 @@ BytecodeArrayWriter::BytecodeArrayWriter(
|
||||
SourcePositionTableBuilder::RecordingMode source_position_mode)
|
||||
: bytecodes_(zone),
|
||||
unbound_jumps_(0),
|
||||
source_position_table_builder_(source_position_mode),
|
||||
source_position_table_builder_(zone, source_position_mode),
|
||||
constant_array_builder_(constant_array_builder),
|
||||
last_bytecode_(Bytecode::kIllegal),
|
||||
last_bytecode_offset_(0),
|
||||
|
@ -339,6 +339,7 @@ class LiftoffCompiler {
|
||||
debug_sidetable_builder_(debug_sidetable_builder),
|
||||
for_debugging_(for_debugging),
|
||||
out_of_line_code_(compilation_zone),
|
||||
source_position_table_builder_(compilation_zone),
|
||||
protected_instructions_(compilation_zone),
|
||||
compilation_zone_(compilation_zone),
|
||||
safepoint_table_builder_(compilation_zone_),
|
||||
|
@ -14,12 +14,18 @@ namespace interpreter {
|
||||
|
||||
class SourcePositionTableTest : public TestWithIsolate {
|
||||
public:
|
||||
SourcePositionTableTest() = default;
|
||||
SourcePositionTableTest() : zone_(isolate()->allocator(), ZONE_NAME) {}
|
||||
~SourcePositionTableTest() override = default;
|
||||
|
||||
SourcePosition toPos(int offset) {
|
||||
return SourcePosition(offset, offset % 10 - 1);
|
||||
}
|
||||
|
||||
SourcePositionTableBuilder* builder() { return &builder_; }
|
||||
|
||||
private:
|
||||
Zone zone_;
|
||||
SourcePositionTableBuilder builder_{&zone_};
|
||||
};
|
||||
|
||||
// Some random offsets, mostly at 'suspicious' bit boundaries.
|
||||
@ -28,48 +34,43 @@ static int offsets[] = {0, 1, 2, 3, 4, 30, 31, 32,
|
||||
129, 250, 1000, 9999, 12000, 31415926};
|
||||
|
||||
TEST_F(SourcePositionTableTest, EncodeStatement) {
|
||||
SourcePositionTableBuilder builder;
|
||||
for (size_t i = 0; i < arraysize(offsets); i++) {
|
||||
builder.AddPosition(offsets[i], toPos(offsets[i]), true);
|
||||
builder()->AddPosition(offsets[i], toPos(offsets[i]), true);
|
||||
}
|
||||
|
||||
// To test correctness, we rely on the assertions in ToSourcePositionTable().
|
||||
// (Also below.)
|
||||
CHECK(!builder.ToSourcePositionTable(isolate()).is_null());
|
||||
CHECK(!builder()->ToSourcePositionTable(isolate()).is_null());
|
||||
}
|
||||
|
||||
TEST_F(SourcePositionTableTest, EncodeStatementDuplicates) {
|
||||
SourcePositionTableBuilder builder;
|
||||
for (size_t i = 0; i < arraysize(offsets); i++) {
|
||||
builder.AddPosition(offsets[i], toPos(offsets[i]), true);
|
||||
builder.AddPosition(offsets[i], toPos(offsets[i] + 1), true);
|
||||
builder()->AddPosition(offsets[i], toPos(offsets[i]), true);
|
||||
builder()->AddPosition(offsets[i], toPos(offsets[i] + 1), true);
|
||||
}
|
||||
|
||||
// To test correctness, we rely on the assertions in ToSourcePositionTable().
|
||||
// (Also below.)
|
||||
CHECK(!builder.ToSourcePositionTable(isolate()).is_null());
|
||||
CHECK(!builder()->ToSourcePositionTable(isolate()).is_null());
|
||||
}
|
||||
|
||||
TEST_F(SourcePositionTableTest, EncodeExpression) {
|
||||
SourcePositionTableBuilder builder;
|
||||
for (size_t i = 0; i < arraysize(offsets); i++) {
|
||||
builder.AddPosition(offsets[i], toPos(offsets[i]), false);
|
||||
builder()->AddPosition(offsets[i], toPos(offsets[i]), false);
|
||||
}
|
||||
CHECK(!builder.ToSourcePositionTable(isolate()).is_null());
|
||||
CHECK(!builder()->ToSourcePositionTable(isolate()).is_null());
|
||||
}
|
||||
|
||||
TEST_F(SourcePositionTableTest, EncodeAscending) {
|
||||
SourcePositionTableBuilder builder;
|
||||
|
||||
int code_offset = 0;
|
||||
int source_position = 0;
|
||||
for (size_t i = 0; i < arraysize(offsets); i++) {
|
||||
code_offset += offsets[i];
|
||||
source_position += offsets[i];
|
||||
if (i % 2) {
|
||||
builder.AddPosition(code_offset, toPos(source_position), true);
|
||||
builder()->AddPosition(code_offset, toPos(source_position), true);
|
||||
} else {
|
||||
builder.AddPosition(code_offset, toPos(source_position), false);
|
||||
builder()->AddPosition(code_offset, toPos(source_position), false);
|
||||
}
|
||||
}
|
||||
|
||||
@ -78,13 +79,13 @@ TEST_F(SourcePositionTableTest, EncodeAscending) {
|
||||
code_offset += offsets[i];
|
||||
source_position -= offsets[i];
|
||||
if (i % 2) {
|
||||
builder.AddPosition(code_offset, toPos(source_position), true);
|
||||
builder()->AddPosition(code_offset, toPos(source_position), true);
|
||||
} else {
|
||||
builder.AddPosition(code_offset, toPos(source_position), false);
|
||||
builder()->AddPosition(code_offset, toPos(source_position), false);
|
||||
}
|
||||
}
|
||||
|
||||
CHECK(!builder.ToSourcePositionTable(isolate()).is_null());
|
||||
CHECK(!builder()->ToSourcePositionTable(isolate()).is_null());
|
||||
}
|
||||
|
||||
} // namespace interpreter
|
||||
|
Loading…
Reference in New Issue
Block a user