[inspector] introduce limit for amount of stored async stacks

[inspector] introduce limit for amount of stored async stacks

BUG=v8:5738
R=dgozman@chromium.org

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

[kozy, 2016-12-16 22:03:50]

Dmitry, please take a look.

[dgozman, 2016-12-16 22:36:11]

https://codereview.chromium.org/2579403002/diff/20001/src/inspector/v8-debugg...
File src/inspector/v8-debugger.h (right):

https://codereview.chromium.org/2579403002/diff/20001/src/inspector/v8-debugg...
src/inspector/v8-debugger.h:155: std::deque<void*> m_scheduledOrder;
Can we instead have two maps: void* -> task_id, and task_id -> void* ?
Then we can increment the id, and just use idToTask.front(). WDYT?

[kozy, 2016-12-16 22:56:16]

all done, please take another look.

https://codereview.chromium.org/2579403002/diff/20001/src/inspector/v8-debugg...
File src/inspector/v8-debugger.h (right):

https://codereview.chromium.org/2579403002/diff/20001/src/inspector/v8-debugg...
src/inspector/v8-debugger.h:155: std::deque<void*> m_scheduledOrder;
On 2016/12/16 22:36:11, dgozman wrote:
> Can we instead have two maps: void* -> task_id, and task_id -> void* ?
> Then we can increment the id, and just use idToTask.front(). WDYT?

Done.

[dgozman, 2016-12-16 23:59:49]

https://codereview.chromium.org/2579403002/diff/40001/src/inspector/v8-debugg...
File src/inspector/v8-debugger.cc (right):

https://codereview.chromium.org/2579403002/diff/40001/src/inspector/v8-debugg...
src/inspector/v8-debugger.cc:913: while (m_idToRawId.size() >
kMaxAsyncTaskStacks) {
while -> if ?

https://codereview.chromium.org/2579403002/diff/40001/src/inspector/v8-debugg...
src/inspector/v8-debugger.cc:920: void V8Debugger::asyncTaskCanceled(void* task)
{
Note that task can be messing from any of our maps now.

https://codereview.chromium.org/2579403002/diff/40001/src/inspector/v8-debugg...
File src/inspector/v8-debugger.h (right):

https://codereview.chromium.org/2579403002/diff/40001/src/inspector/v8-debugg...
src/inspector/v8-debugger.h:153: std::map<int, void*> m_idToRawId;
m_idToTask

https://codereview.chromium.org/2579403002/diff/40001/src/inspector/v8-debugg...
src/inspector/v8-debugger.h:154: std::unordered_map<void*, int> m_rawIdToId;
m_taskToId

https://codereview.chromium.org/2579403002/diff/40001/src/inspector/v8-debugg...
src/inspector/v8-debugger.h:155: int m_lastId;
m_lastTaskId

[kozy, 2016-12-17 01:12:42]

all done, test added ,please take a look.

https://codereview.chromium.org/2579403002/diff/40001/src/inspector/v8-debugg...
File src/inspector/v8-debugger.cc (right):

https://codereview.chromium.org/2579403002/diff/40001/src/inspector/v8-debugg...
src/inspector/v8-debugger.cc:913: while (m_idToRawId.size() >
kMaxAsyncTaskStacks) {
On 2016/12/16 23:59:49, dgozman wrote:
> while -> if ?

Done.

https://codereview.chromium.org/2579403002/diff/40001/src/inspector/v8-debugg...
src/inspector/v8-debugger.cc:920: void V8Debugger::asyncTaskCanceled(void* task)
{
On 2016/12/16 23:59:49, dgozman wrote:
> Note that task can be messing from any of our maps now.

Done.

https://codereview.chromium.org/2579403002/diff/40001/src/inspector/v8-debugg...
File src/inspector/v8-debugger.h (right):

https://codereview.chromium.org/2579403002/diff/40001/src/inspector/v8-debugg...
src/inspector/v8-debugger.h:153: std::map<int, void*> m_idToRawId;
On 2016/12/16 23:59:49, dgozman wrote:
> m_idToTask

Done.

https://codereview.chromium.org/2579403002/diff/40001/src/inspector/v8-debugg...
src/inspector/v8-debugger.h:154: std::unordered_map<void*, int> m_rawIdToId;
On 2016/12/16 23:59:49, dgozman wrote:
> m_taskToId

Done.

https://codereview.chromium.org/2579403002/diff/40001/src/inspector/v8-debugg...
src/inspector/v8-debugger.h:155: int m_lastId;
On 2016/12/16 23:59:49, dgozman wrote:
> m_lastTaskId

Done.

[kozy, 2016-12-17 03:13:27]

added missing test, ready for review.

[dgozman, 2016-12-18 03:31:02]

Looks good!

https://codereview.chromium.org/2579403002/diff/100001/test/inspector/DEPS
File test/inspector/DEPS (right):

https://codereview.chromium.org/2579403002/diff/100001/test/inspector/DEPS#ne...
test/inspector/DEPS:10: "+src/inspector/v8-inspector-impl.h",
Let's expose src/inspector/test-interface.h, which will do static cast for you.
It can access debugger() as well, without extra method in V8InspectorImpl.

https://codereview.chromium.org/2579403002/diff/100001/test/inspector/debugge...
File test/inspector/debugger/async-stacks-limit.js (right):

https://codereview.chromium.org/2579403002/diff/100001/test/inspector/debugge...
test/inspector/debugger/async-stacks-limit.js:122: function
testTwoLimitTwoSetTimeouts(next) {
Let's add testTenLimitTwentySetTimeouts.

[kozy, 2016-12-18 06:27:38]

all done, please take another look.

https://codereview.chromium.org/2579403002/diff/100001/test/inspector/DEPS
File test/inspector/DEPS (right):

https://codereview.chromium.org/2579403002/diff/100001/test/inspector/DEPS#ne...
test/inspector/DEPS:10: "+src/inspector/v8-inspector-impl.h",
On 2016/12/18 03:31:01, dgozman wrote:
> Let's expose src/inspector/test-interface.h, which will do static cast for
you.
> It can access debugger() as well, without extra method in V8InspectorImpl.

Done.

https://codereview.chromium.org/2579403002/diff/100001/test/inspector/debugge...
File test/inspector/debugger/async-stacks-limit.js (right):

https://codereview.chromium.org/2579403002/diff/100001/test/inspector/debugge...
test/inspector/debugger/async-stacks-limit.js:122: function
testTwoLimitTwoSetTimeouts(next) {
On 2016/12/18 03:31:01, dgozman wrote:
> Let's add testTenLimitTwentySetTimeouts.

Done.

[kozy, 2016-12-18 06:27:44]

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

[commit-bot: I haz the power, 2016-12-18 06:27:53]

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, 2016-12-18 06:55:20]

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

[commit-bot: I haz the power, 2016-12-18 06:55:21]

Dry run: This issue passed the CQ dry run.

[dgozman, 2016-12-18 09:49:22]

Wonderful, lgtm.

[kozy, 2016-12-18 17:02:55]

The CQ bit was checked by kozyatinskiy@chromium.org

[commit-bot: I haz the power, 2016-12-18 17:03:03]

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, 2016-12-18 17:04:40]

CQ is committing da patch.

Bot data: {"patchset_id": 120001, "attempt_start_ts": 1482080575694130,
"parent_rev": "083a5dcdfee3a6b7c3e41e4a2dc4420604e86641", "commit_rev":
"c42915f02d91e6605ce347c9dd798607813d8130"}

[commit-bot: I haz the power, 2016-12-18 17:04:49]

Description was changed from

==========
[inspector] introduce limit for amount of stored async stacks

BUG=v8:5738
R=dgozman@chromium.org
==========

to

==========
[inspector] introduce limit for amount of stored async stacks

BUG=v8:5738
R=dgozman@chromium.org

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

[commit-bot: I haz the power, 2016-12-18 17:04:50]

Committed patchset #7 (id:120001) as
https://chromium.googlesource.com/v8/v8/+/c42915f02d91e6605ce347c9dd798607813...