Commit 2e2dc986 authored by Charles Kerr's avatar Charles Kerr Committed by Commit Bot

perf: make GetPositionInfoSlow() faster

Halve the number of lookups in ExtractLocationForJSFunction() by calling
GetPositionInfo() directly instead of making separate calls for column
and line number.

Improve the efficiency of position lookups in slow mode. The current
code does a linear walk through the source by calling String::Get() for
each character. This PR also does a linear walk, but avoids the overhead
of multiple Get() calls by pulling the String's flat content into a
local vector and walking through that.

Downstream Electron discussion of this can be found at
https://github.com/electron/electron/issues/24509

Apologies in advance if I've missed anything; this is my first V8 CL...

Change-Id: I22b034dc1bfe967164d2f8515a9a0c1d7f043c83
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2496065
Commit-Queue: Simon Zünd <szuend@chromium.org>
Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
Reviewed-by: 's avatarSimon Zünd <szuend@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70783}
parent f89869a2
...@@ -72,6 +72,7 @@ Bert Belder <bertbelder@gmail.com> ...@@ -72,6 +72,7 @@ Bert Belder <bertbelder@gmail.com>
Burcu Dogan <burcujdogan@gmail.com> Burcu Dogan <burcujdogan@gmail.com>
Caitlin Potter <caitpotter88@gmail.com> Caitlin Potter <caitpotter88@gmail.com>
Craig Schlenter <craig.schlenter@gmail.com> Craig Schlenter <craig.schlenter@gmail.com>
Charles Kerr <charles@charleskerr.com>
Chengzhong Wu <legendecas@gmail.com> Chengzhong Wu <legendecas@gmail.com>
Choongwoo Han <cwhan.tunz@gmail.com> Choongwoo Han <cwhan.tunz@gmail.com>
Chris Nardi <hichris123@gmail.com> Chris Nardi <hichris123@gmail.com>
......
...@@ -4790,30 +4790,43 @@ bool Script::ContainsAsmModule() { ...@@ -4790,30 +4790,43 @@ bool Script::ContainsAsmModule() {
} }
namespace { namespace {
bool GetPositionInfoSlow(const Script script, int position,
Script::PositionInfo* info) {
if (!script.source().IsString()) return false;
if (position < 0) position = 0;
String source_string = String::cast(script.source()); template <typename Char>
bool GetPositionInfoSlowImpl(const Vector<Char>& source, int position,
Script::PositionInfo* info) {
if (position < 0) {
position = 0;
}
int line = 0; int line = 0;
int line_start = 0; const auto begin = std::cbegin(source);
int len = source_string.length(); const auto end = std::cend(source);
for (int pos = 0; pos <= len; ++pos) { for (auto line_begin = begin; line_begin < end;) {
if (pos == len || source_string.Get(pos) == '\n') { const auto line_end = std::find(line_begin, end, '\n');
if (position <= pos) { if (position <= (line_end - begin)) {
info->line = line; info->line = line;
info->column = position - line_start; info->column = static_cast<int>((begin + position) - line_begin);
info->line_start = line_start; info->line_start = static_cast<int>(line_begin - begin);
info->line_end = pos; info->line_end = static_cast<int>(line_end - begin);
return true; return true;
}
line++;
line_start = pos + 1;
} }
++line;
line_begin = line_end + 1;
} }
return false; return false;
} }
bool GetPositionInfoSlow(const Script script, int position,
const DisallowHeapAllocation& no_gc,
Script::PositionInfo* info) {
if (!script.source().IsString()) {
return false;
}
auto source = String::cast(script.source());
const auto flat = source.GetFlatContent(no_gc);
return flat.IsOneByte()
? GetPositionInfoSlowImpl(flat.ToOneByteVector(), position, info)
: GetPositionInfoSlowImpl(flat.ToUC16Vector(), position, info);
}
} // namespace } // namespace
bool Script::GetPositionInfo(int position, PositionInfo* info, bool Script::GetPositionInfo(int position, PositionInfo* info,
...@@ -4835,7 +4848,9 @@ bool Script::GetPositionInfo(int position, PositionInfo* info, ...@@ -4835,7 +4848,9 @@ bool Script::GetPositionInfo(int position, PositionInfo* info,
if (line_ends().IsUndefined()) { if (line_ends().IsUndefined()) {
// Slow mode: we do not have line_ends. We have to iterate through source. // Slow mode: we do not have line_ends. We have to iterate through source.
if (!GetPositionInfoSlow(*this, position, info)) return false; if (!GetPositionInfoSlow(*this, position, no_allocation, info)) {
return false;
}
} else { } else {
DCHECK(line_ends().IsFixedArray()); DCHECK(line_ends().IsFixedArray());
FixedArray ends = FixedArray::cast(line_ends()); FixedArray ends = FixedArray::cast(line_ends());
......
...@@ -579,9 +579,9 @@ void V8HeapExplorer::ExtractLocationForJSFunction(HeapEntry* entry, ...@@ -579,9 +579,9 @@ void V8HeapExplorer::ExtractLocationForJSFunction(HeapEntry* entry,
Script script = Script::cast(func.shared().script()); Script script = Script::cast(func.shared().script());
int scriptId = script.id(); int scriptId = script.id();
int start = func.shared().StartPosition(); int start = func.shared().StartPosition();
int line = script.GetLineNumber(start); Script::PositionInfo info;
int col = script.GetColumnNumber(start); script.GetPositionInfo(start, &info, Script::WITH_OFFSET);
snapshot_->AddLocation(entry, scriptId, line, col); snapshot_->AddLocation(entry, scriptId, info.line, info.column);
} }
HeapEntry* V8HeapExplorer::AddEntry(HeapObject object) { HeapEntry* V8HeapExplorer::AddEntry(HeapObject object) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment