Browse Source

perf: improve heap snapshot performance (#26229)

Fixes #24509.

cherry-pick 2e2dc98 from v8
Charles Kerr 4 years ago
parent
commit
831e84504d
2 changed files with 135 additions and 0 deletions
  1. 1 0
      patches/v8/.patches
  2. 134 0
      patches/v8/perf_make_getpositioninfoslow_faster.patch

+ 1 - 0
patches/v8/.patches

@@ -7,3 +7,4 @@ workaround_an_undefined_symbol_error.patch
 do_not_export_private_v8_symbols_on_windows.patch
 revert_cleanup_switch_offset_of_to_offsetof_where_possible.patch
 fix_build_deprecated_attirbute_for_older_msvc_versions.patch
+perf_make_getpositioninfoslow_faster.patch

+ 134 - 0
patches/v8/perf_make_getpositioninfoslow_faster.patch

@@ -0,0 +1,134 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: Charles Kerr <[email protected]>
+Date: Mon, 26 Oct 2020 16:53:25 -0500
+Subject: perf: make GetPositionInfoSlow() faster
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+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
+
+Change-Id: I22b034dc1bfe967164d2f8515a9a0c1d7f043c83
+Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2496065
+Commit-Queue: Simon Zünd <[email protected]>
+Reviewed-by: Ulan Degenbaev <[email protected]>
+Reviewed-by: Simon Zünd <[email protected]>
+Cr-Commit-Position: refs/heads/master@{#70783}
+
+diff --git a/AUTHORS b/AUTHORS
+index 7c43c60fc13af2a793547a9e5ef689e380ef565f..31a46b9bec0757c78650124793ba30def5f438f7 100644
+--- a/AUTHORS
++++ b/AUTHORS
+@@ -69,6 +69,7 @@ Bert Belder <[email protected]>
+ Burcu Dogan <[email protected]>
+ Caitlin Potter <[email protected]>
+ Craig Schlenter <[email protected]>
++Charles Kerr <[email protected]>
+ Chengzhong Wu <[email protected]>
+ Choongwoo Han <[email protected]>
+ Chris Nardi <[email protected]>
+diff --git a/src/objects/objects.cc b/src/objects/objects.cc
+index 3ef1067d9f956cba708475aa2beb87dcc5df261b..5e0c831f73b6ccbe40c556497f5ba6fbc9f42b48 100644
+--- a/src/objects/objects.cc
++++ b/src/objects/objects.cc
+@@ -4785,30 +4785,43 @@ bool Script::ContainsAsmModule() {
+ }
+ 
+ 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_start = 0;
+-  int len = source_string.length();
+-  for (int pos = 0; pos <= len; ++pos) {
+-    if (pos == len || source_string.Get(pos) == '\n') {
+-      if (position <= pos) {
+-        info->line = line;
+-        info->column = position - line_start;
+-        info->line_start = line_start;
+-        info->line_end = pos;
+-        return true;
+-      }
+-      line++;
+-      line_start = pos + 1;
++  const auto begin = std::cbegin(source);
++  const auto end = std::cend(source);
++  for (auto line_begin = begin; line_begin < end;) {
++    const auto line_end = std::find(line_begin, end, '\n');
++    if (position <= (line_end - begin)) {
++      info->line = line;
++      info->column = static_cast<int>((begin + position) - line_begin);
++      info->line_start = static_cast<int>(line_begin - begin);
++      info->line_end = static_cast<int>(line_end - begin);
++      return true;
+     }
++    ++line;
++    line_begin = line_end + 1;
+   }
+   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
+ 
+ bool Script::GetPositionInfo(int position, PositionInfo* info,
+@@ -4830,7 +4843,9 @@ bool Script::GetPositionInfo(int position, PositionInfo* info,
+ 
+   if (line_ends().IsUndefined()) {
+     // 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 {
+     DCHECK(line_ends().IsFixedArray());
+     FixedArray ends = FixedArray::cast(line_ends());
+diff --git a/src/profiler/heap-snapshot-generator.cc b/src/profiler/heap-snapshot-generator.cc
+index 2fa4f2e5e841bbbf98580dee241fac00899e780e..8f98451488052ba34a59635b707aa5bd3d086161 100644
+--- a/src/profiler/heap-snapshot-generator.cc
++++ b/src/profiler/heap-snapshot-generator.cc
+@@ -573,9 +573,9 @@ void V8HeapExplorer::ExtractLocationForJSFunction(HeapEntry* entry,
+   Script script = Script::cast(func.shared().script());
+   int scriptId = script.id();
+   int start = func.shared().StartPosition();
+-  int line = script.GetLineNumber(start);
+-  int col = script.GetColumnNumber(start);
+-  snapshot_->AddLocation(entry, scriptId, line, col);
++  Script::PositionInfo info;
++  script.GetPositionInfo(start, &info, Script::WITH_OFFSET);
++  snapshot_->AddLocation(entry, scriptId, info.line, info.column);
+ }
+ 
+ HeapEntry* V8HeapExplorer::AddEntry(HeapObject object) {