1171654 - Record stacks for pending protocol.js requests when testing

Status: RESOLVED FIXED

(DevTools :: Framework, defect)

Description

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Record the pending request stack as a debugging aid during tests.

Comment 1

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

For the moment, I plan to move |testing| off gDevTools and onto DevToolsUtils, so that it's available in toolkit.

We can then undo workarounds like bug 1160837 comment 34.

Comment 2

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d7f49d95f6ed

Comment 3

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1171654 - Report stacks for pending requests. r=ochameau

Bug 1171654 - Record stacks of pending requests. r=ochameau

Comment 4

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Attachment: MozReview Request: Bug 1171654 - Move testing flag to DevToolsUtils. r?ochameau

Bug 1171654 - Move testing flag to DevToolsUtils. r=ochameau

Comment 5

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b7f2a32a59ed

Comment 6

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8620515 [details]
MozReview Request: Bug 1171654 - Report stacks for pending requests. r=ochameau

Bug 1171654 - Record stacks of pending requests. r?ochameau

Comment 7

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8620516 [details]
MozReview Request: Bug 1171654 - Move testing flag to DevToolsUtils. r?ochameau

Bug 1171654 - Move testing flag to DevToolsUtils. r?ochameau

Comment 8

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8620515 [details]
MozReview Request: Bug 1171654 - Report stacks for pending requests. r=ochameau

Bug 1171654 - Record stacks of pending requests. r?ochameau

Comment 9

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8620516 [details]
MozReview Request: Bug 1171654 - Move testing flag to DevToolsUtils. r?ochameau

Bug 1171654 - Move testing flag to DevToolsUtils. r?ochameau

Comment 10

[Alexandre Poirot [:ochameau]]

Comment on attachment 8620515 [details]
MozReview Request: Bug 1171654 - Report stacks for pending requests. r=ochameau

https://reviewboard.mozilla.org/r/10785/#review10741

Comment 11

[Alexandre Poirot [:ochameau]]

Comment on attachment 8620516 [details]
MozReview Request: Bug 1171654 - Move testing flag to DevToolsUtils. r?ochameau

https://reviewboard.mozilla.org/r/10787/#review10739

Could you spawn some more instances of mochitest-devtools, just to be sure there is no hidden intermittent?

::: browser/devtools/styleeditor/test/browser_styleeditor_fetch-from-cache.js
(Diff revision 3)
> -  gDevTools.testing = isTesting;

Do you know why this test used to toggle testing and no longer needs to? (was that just copy paste?)

Comment 12

[Alexandre Poirot [:ochameau]]

I'm lost now, it looks like you did the same patch in bug 1151414, but without using testing flag?!

Comment 13

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Nick, maybe you can provide some guidance here.  This bug (which I wrote patches for, but then I forgot about while working on RDP async stacks which you reviewed recently - bug 1151414) wanted to record just "regular" stacks when a protocol.js request is sent.

At the time that Alex and I discussed this particular bug, it seemed logical to only do the stack recording during testing, in case there was speed or memory cost worth avoiding for every RDP request.

So, my question is: is stack recording in general an expensive operation in time and/or space to be avoided for the case of every RDP request?  What about in the async stack case?  I am aware of bug 1152893 (async performance regressions), but I was not sure if I should be worried about that here.

Comment 14

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #13)
> So, my question is: is stack recording in general an expensive operation in
> time and/or space to be avoided for the case of every RDP request?  What
> about in the async stack case?  I am aware of bug 1152893 (async performance
> regressions), but I was not sure if I should be worried about that here.

I wouldn't worry much about bug 1152893; that is when you are microbenchmarking an operation that captures an async stack in a loop some thousand of times vs one an operation that does not. We never do anything like that with RDP protocol requests, but unfortunately stuff on the web does do that with promises. Either way, I wouldn't worry about it here. And we are improving the situation in bug 1028418 and bug 1177508.

TLDR: I wouldn't worry about it.

Comment 15

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Oh, and regarding space: can't get around the fact that you are capturing stacks and need to store them somewhere, when before you were not, however the js::SavedStacks machinery was designed to be compact: http://fitzgeraldnick.com/weblog/61/

Comment 16

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Thanks Nick.  So, it seems like we can leave async stacks always on for RDP requests.

The original focus of this bug is no longer relevant, but I'll proceed with the testing flag move in a new bug.

Comment 17

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Will continue the testing flag work in bug 1178851.

Comment 18

[Alexandre Poirot [:ochameau]]

Actually, I was looking at a pending request failure, and bug 1151414 only shows stack of request failure, not *pending* rejection.

Comment 19

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a8f1d8235d0a

Comment 20

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Comment on attachment 8620515 [details]
MozReview Request: Bug 1171654 - Report stacks for pending requests. r=ochameau

Bug 1171654 - Report stacks for pending requests. r=ochameau

Comment 21

[Alexandre Poirot [:ochameau]]

Comment on attachment 8620515 [details]
MozReview Request: Bug 1171654 - Report stacks for pending requests. r=ochameau

I'm able to debug pending now, thanks!

Comment 22

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/5819373caa6e

Comment 23

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/5819373caa6e