Skip to content

Commit 88e53e5

Browse files
ckerrminiak
ckerr
and
miniak
authored
perf: improve heap snapshot performance (electron#26227)
Fixes electron#24509. cherry-pick 2e2dc98 from v8 Co-authored-by: Milan Burda <milan.burda@gmail.com>
1 parent 86e4cce commit 88e53e5

File tree

2 files changed

+135
-0
lines changed

2 files changed

+135
-0
lines changed

‎patches/v8/.patches

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,4 @@ backport_986051.patch
1919
cherry-pick-7e5c7b5964.patch
2020
cherry-pick-6aa1e71fbd09.patch
2121
cherry-pick-6a4cd97d6691.patch
22+
perf_improve_heap_snapshot_performance.patch

‎patches/v8/perf_improve_heap_snapshot_performance.patch

Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
2+
From: Charles Kerr <charles@charleskerr.com>
3+
Date: Wed, 28 Oct 2020 12:11:10 -0500
4+
Subject: perf: improve heap snapshot performance
5+
MIME-Version: 1.0
6+
Content-Type: text/plain; charset=UTF-8
7+
Content-Transfer-Encoding: 8bit
8+
9+
Halve the number of lookups in ExtractLocationForJSFunction() by calling
10+
GetPositionInfo() directly instead of making separate calls for column
11+
and line number.
12+
13+
Improve the efficiency of position lookups in slow mode. The current
14+
code does a linear walk through the source by calling String::Get() for
15+
each character. This PR also does a linear walk, but avoids the overhead
16+
of multiple Get() calls by pulling the String's flat content into a
17+
local vector and walking through that.
18+
19+
Downstream Electron discussion of this can be found at
20+
https://github.com/electron/electron/issues/24509
21+
22+
Change-Id: I22b034dc1bfe967164d2f8515a9a0c1d7f043c83
23+
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2496065
24+
Commit-Queue: Simon Zünd <szuend@chromium.org>
25+
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
26+
Reviewed-by: Simon Zünd <szuend@chromium.org>
27+
Cr-Commit-Position: refs/heads/master@{#70783}
28+
29+
diff --git a/AUTHORS b/AUTHORS
30+
index 79569706c747b9cea475f34a89f5a7ac6d7836e8..8477fb33e573ea78e4eab89956372d8c29562a00 100644
31+
--- a/AUTHORS
32+
+++ b/AUTHORS
33+
@@ -68,6 +68,7 @@ Bert Belder <bertbelder@gmail.com>
34+
Burcu Dogan <burcujdogan@gmail.com>
35+
Caitlin Potter <caitpotter88@gmail.com>
36+
Craig Schlenter <craig.schlenter@gmail.com>
37+
+Charles Kerr <charles@charleskerr.com>
38+
Choongwoo Han <cwhan.tunz@gmail.com>
39+
Chris Nardi <hichris123@gmail.com>
40+
Christopher A. Taylor <chris@gameclosure.com>
41+
diff --git a/src/objects/objects.cc b/src/objects/objects.cc
42+
index 947141a5e6be4793c8915663e7f4cd7b7ae11486..8a446bb109fd248a42a750bcb2f6747c46833cec 100644
43+
--- a/src/objects/objects.cc
44+
+++ b/src/objects/objects.cc
45+
@@ -4674,30 +4674,43 @@ bool Script::ContainsAsmModule() {
46+
}
47+
48+
namespace {
49+
-bool GetPositionInfoSlow(const Script script, int position,
50+
- Script::PositionInfo* info) {
51+
- if (!script.source().IsString()) return false;
52+
- if (position < 0) position = 0;
53+
54+
- String source_string = String::cast(script.source());
55+
+template <typename Char>
56+
+bool GetPositionInfoSlowImpl(const Vector<Char>& source, int position,
57+
+ Script::PositionInfo* info) {
58+
+ if (position < 0) {
59+
+ position = 0;
60+
+ }
61+
int line = 0;
62+
- int line_start = 0;
63+
- int len = source_string.length();
64+
- for (int pos = 0; pos <= len; ++pos) {
65+
- if (pos == len || source_string.Get(pos) == '\n') {
66+
- if (position <= pos) {
67+
- info->line = line;
68+
- info->column = position - line_start;
69+
- info->line_start = line_start;
70+
- info->line_end = pos;
71+
- return true;
72+
- }
73+
- line++;
74+
- line_start = pos + 1;
75+
+ const auto begin = std::cbegin(source);
76+
+ const auto end = std::cend(source);
77+
+ for (auto line_begin = begin; line_begin < end;) {
78+
+ const auto line_end = std::find(line_begin, end, '\n');
79+
+ if (position <= (line_end - begin)) {
80+
+ info->line = line;
81+
+ info->column = static_cast<int>((begin + position) - line_begin);
82+
+ info->line_start = static_cast<int>(line_begin - begin);
83+
+ info->line_end = static_cast<int>(line_end - begin);
84+
+ return true;
85+
}
86+
+ ++line;
87+
+ line_begin = line_end + 1;
88+
}
89+
return false;
90+
}
91+
+bool GetPositionInfoSlow(const Script script, int position,
92+
+ const DisallowHeapAllocation& no_gc,
93+
+ Script::PositionInfo* info) {
94+
+ if (!script.source().IsString()) {
95+
+ return false;
96+
+ }
97+
+ auto source = String::cast(script.source());
98+
+ const auto flat = source.GetFlatContent(no_gc);
99+
+ return flat.IsOneByte()
100+
+ ? GetPositionInfoSlowImpl(flat.ToOneByteVector(), position, info)
101+
+ : GetPositionInfoSlowImpl(flat.ToUC16Vector(), position, info);
102+
+}
103+
+
104+
} // namespace
105+
106+
bool Script::GetPositionInfo(int position, PositionInfo* info,
107+
@@ -4719,7 +4732,9 @@ bool Script::GetPositionInfo(int position, PositionInfo* info,
108+
109+
if (line_ends().IsUndefined()) {
110+
// Slow mode: we do not have line_ends. We have to iterate through source.
111+
- if (!GetPositionInfoSlow(*this, position, info)) return false;
112+
+ if (!GetPositionInfoSlow(*this, position, no_allocation, info)) {
113+
+ return false;
114+
+ }
115+
} else {
116+
DCHECK(line_ends().IsFixedArray());
117+
FixedArray ends = FixedArray::cast(line_ends());
118+
diff --git a/src/profiler/heap-snapshot-generator.cc b/src/profiler/heap-snapshot-generator.cc
119+
index 5c13d64f196067b70377ce321d5de0cb05db6ff7..1f2f1da22ead9dbffe0be7205757c9831a7aa9fc 100644
120+
--- a/src/profiler/heap-snapshot-generator.cc
121+
+++ b/src/profiler/heap-snapshot-generator.cc
122+
@@ -570,9 +570,9 @@ void V8HeapExplorer::ExtractLocationForJSFunction(HeapEntry* entry,
123+
Script script = Script::cast(func.shared().script());
124+
int scriptId = script.id();
125+
int start = func.shared().StartPosition();
126+
- int line = script.GetLineNumber(start);
127+
- int col = script.GetColumnNumber(start);
128+
- snapshot_->AddLocation(entry, scriptId, line, col);
129+
+ Script::PositionInfo info;
130+
+ script.GetPositionInfo(start, &info, Script::WITH_OFFSET);
131+
+ snapshot_->AddLocation(entry, scriptId, info.line, info.column);
132+
}
133+
134+
HeapEntry* V8HeapExplorer::AddEntry(HeapObject object) {

0 commit comments

Comments
 (0)