[inspector] store stack frame in struct instead of JSObject

[inspector] store stack frame in struct instead of JSObject

JSObject is slow: creating strings for keys and storing values by these keys after takes significant amount of time.
With this CL console methods (most of them collect top stack frame to calculate source location) are ~33% faster.
V8Debugger::captureStackTrace is ~50% faster.

BUG=v8:6189
R=yangguo@chromium.org
TBR=bmeurer@chromium.org

Review-Url: https://codereview.chromium.org/2789073002
Cr-Commit-Position: refs/heads/master@{#44344}
Committed: https://chromium.googlesource.com/v8/v8/+/dc662e5b740c46dc6350485efd64bccd4a4d7630

[kozy, 2017-04-02 07:17:27]

The CQ bit was checked by kozyatinskiy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-02 07:17:37]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kozy, 2017-04-02 07:19:50]

Description was changed from

==========
[inspector] store stack frame in struct instead of JSObject

JSObject is slow: creating strings for keys and storing values by these keys
after takes significant amount of time.
With this CL console methods (most of them collect top stack frame to calculate
source location) are ~40% faster.

BUG=v8:6189
R=yangguo@chromium.org
==========

to

==========
[inspector] store stack frame in struct instead of JSObject

JSObject is slow: creating strings for keys and storing values by these keys
after takes significant amount of time.
With this CL console methods (most of them collect top stack frame to calculate
source location) are ~33% faster.

BUG=v8:6189
R=yangguo@chromium.org
==========

[commit-bot: I haz the power, 2017-04-02 07:36:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-02 07:36:13]

Dry run: Try jobs failed on following builders:
  v8_linux64_gyp_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng/build...)
  v8_linux64_gyp_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_gyp_rel_ng_trigg...)

[kozy, 2017-04-02 07:49:26]

The CQ bit was checked by kozyatinskiy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-02 07:49:33]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-02 07:53:02]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-02 07:53:03]

Dry run: Try jobs failed on following builders:
  v8_linux64_avx2_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng/buil...)
  v8_linux64_avx2_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_avx2_rel_ng_trig...)

[kozy, 2017-04-02 08:13:22]

Yang, please take a look.

More details about performance in linked issue.
Roadmap:
- use v8::Local<v8::StackTrace> in V8StackTrace as long as possible (currently
we copy all data from v8::StackFrame into our internal struct when we receive
stack trace),
- symbolize fields in StackFrameInfo lazily.
- deduplicate StackFrameInfo (allow symbolize only once and reduce memory),
- deduplicate StackTrace (if it will give significantly reduce of memory usage
after StackFrameInfo deduplication).

[kozy, 2017-04-02 08:26:05]

Description was changed from

==========
[inspector] store stack frame in struct instead of JSObject

JSObject is slow: creating strings for keys and storing values by these keys
after takes significant amount of time.
With this CL console methods (most of them collect top stack frame to calculate
source location) are ~33% faster.

BUG=v8:6189
R=yangguo@chromium.org
==========

to

==========
[inspector] store stack frame in struct instead of JSObject

JSObject is slow: creating strings for keys and storing values by these keys
after takes significant amount of time.
With this CL console methods (most of them collect top stack frame to calculate
source location) are ~33% faster.
V8Debugger::captureStackTrace is ~50% faster.

BUG=v8:6189
R=yangguo@chromium.org
==========

[kozy, 2017-04-03 05:13:02]

The CQ bit was checked by kozyatinskiy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-03 05:13:12]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-03 05:52:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-03 05:52:30]

Dry run: Try jobs failed on following builders:
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/23640)
  v8_linux_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng_triggered/b...)

[kozy, 2017-04-03 06:14:03]

The CQ bit was checked by kozyatinskiy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-03 06:14:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-03 06:51:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-03 06:51:26]

Dry run: Try jobs failed on following builders:
  v8_linux_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng/builds/23643)
  v8_linux_rel_ng_triggered on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux_rel_ng_triggered/b...)

[kozy, 2017-04-03 07:35:05]

The CQ bit was checked by kozyatinskiy@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-04-03 07:35:19]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Yang, 2017-04-03 07:58:41]

LGTM.

Some things to consider:

- It might be even slightly faster if we do not use a struct for every stack
frame, but put data for all stack frames into a flat FixedArray. To the API, a
StackFrameInfo would simply be a wrapper to the FixedArray and an index into it.
Not sure whether that's worth the effort.
- Computing position information lazily will get you a huge speed boost. You
only need to save the source position table and the code offset upfront. The
first time you need the source positions, you compute it and overwrite the
fields for source position table and code offset.
- We should consider not exposing this to the API.

https://codereview.chromium.org/2789073002/diff/20001/src/isolate.cc
File src/isolate.cc (right):

https://codereview.chromium.org/2789073002/diff/20001/src/isolate.cc#newcode633
src/isolate.cc:633: &info, Script::WITH_OFFSET);
Getting position info is fairly expensive. We can be much faster if we do this
lazily by only saving the source position table and the code offset here.

[commit-bot: I haz the power, 2017-04-03 08:00:07]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-03 08:00:07]

Dry run: This issue passed the CQ dry run.

[kozy, 2017-04-03 14:54:47]

On 2017/04/03 07:58:41, Yang wrote:
> - It might be even slightly faster if we do not use a struct for every stack
> frame, but put data for all stack frames into a flat FixedArray. To the API, a
> StackFrameInfo would simply be a wrapper to the FixedArray and an index into
it.
> Not sure whether that's worth the effort.

I'll try this approach and measure performance. My current idea was that storing
data to struct is much faster then resolving locations, functionName and other
fields and I'd like to introduce cache of all call frames and lookup it by
source_position(code_offset + source_position_table) + script reference and if
call frame is found just return this frame. It should reduce memory usage and
we'll calculate call frame only once.

> - Computing position information lazily will get you a huge speed boost. You
> only need to save the source position table and the code offset upfront. The
> first time you need the source positions, you compute it and overwrite the
> fields for source position table and code offset.
Thanks, I'll use it.

> - We should consider not exposing this to the API.
What do you mean? We currently use v8::StackTrace to get stack trace information
in V8StackTraceImpl and can make it fast too. Should we introduce another data
structure in debug-interface.h? Or we'd like to deprecate and remove StackTrace
API from v8.h and provide only inspector captureStackTrace?

[kozy, 2017-04-03 14:54:56]

https://codereview.chromium.org/2789073002/diff/20001/src/isolate.cc
File src/isolate.cc (right):

https://codereview.chromium.org/2789073002/diff/20001/src/isolate.cc#newcode633
src/isolate.cc:633: &info, Script::WITH_OFFSET);
On 2017/04/03 07:58:40, Yang wrote:
> Getting position info is fairly expensive. We can be much faster if we do this
> lazily by only saving the source position table and the code offset here.

Thanks, I'll upload separate CLfor this because currently we get all information
from v8::StackTrace in V8StackTraceImpl immediately and I'd like to make it lazy
first.

[kozy, 2017-04-03 14:55:04]

The CQ bit was checked by kozyatinskiy@chromium.org

[kozy, 2017-04-03 14:55:13]

The CQ bit was unchecked by kozyatinskiy@chromium.org

[commit-bot: I haz the power, 2017-04-03 14:55:15]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[kozy, 2017-04-03 14:56:01]

Description was changed from

==========
[inspector] store stack frame in struct instead of JSObject

JSObject is slow: creating strings for keys and storing values by these keys
after takes significant amount of time.
With this CL console methods (most of them collect top stack frame to calculate
source location) are ~33% faster.
V8Debugger::captureStackTrace is ~50% faster.

BUG=v8:6189
R=yangguo@chromium.org
==========

to

==========
[inspector] store stack frame in struct instead of JSObject

JSObject is slow: creating strings for keys and storing values by these keys
after takes significant amount of time.
With this CL console methods (most of them collect top stack frame to calculate
source location) are ~33% faster.
V8Debugger::captureStackTrace is ~50% faster.

BUG=v8:6189
R=yangguo@chromium.org
TBR=bmeurer@chromium.org
==========

[kozy, 2017-04-03 14:56:17]

The CQ bit was checked by kozyatinskiy@chromium.org

[commit-bot: I haz the power, 2017-04-03 14:56:24]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-03 14:58:50]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1491231377233020,
"parent_rev": "32d4d8e93dda9f8aa069dd5f3f88ba09462ba89d", "commit_rev":
"dc662e5b740c46dc6350485efd64bccd4a4d7630"}

[commit-bot: I haz the power, 2017-04-03 14:58:56]

Description was changed from

==========
[inspector] store stack frame in struct instead of JSObject

JSObject is slow: creating strings for keys and storing values by these keys
after takes significant amount of time.
With this CL console methods (most of them collect top stack frame to calculate
source location) are ~33% faster.
V8Debugger::captureStackTrace is ~50% faster.

BUG=v8:6189
R=yangguo@chromium.org
TBR=bmeurer@chromium.org
==========

to

==========
[inspector] store stack frame in struct instead of JSObject

JSObject is slow: creating strings for keys and storing values by these keys
after takes significant amount of time.
With this CL console methods (most of them collect top stack frame to calculate
source location) are ~33% faster.
V8Debugger::captureStackTrace is ~50% faster.

BUG=v8:6189
R=yangguo@chromium.org
TBR=bmeurer@chromium.org

Review-Url: https://codereview.chromium.org/2789073002
Cr-Commit-Position: refs/heads/master@{#44344}
Committed:
https://chromium.googlesource.com/v8/v8/+/dc662e5b740c46dc6350485efd64bccd4a4...
==========

[commit-bot: I haz the power, 2017-04-03 14:58:57]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/v8/v8/+/dc662e5b740c46dc6350485efd64bccd4a4...