Closed Bug 1452204 Opened Last year Closed Last year

Does synchronous stackwalking in profiler_get_backtrace not work on Windows?

Categories

(Core :: mozglue, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla62
Tracking Status
firefox61 --- wontfix
firefox62 --- fixed

People

(Reporter: mstange, Assigned: xidorn)

References

(Blocks 1 open bug)

Details

Attachments

(2 files)

In this profile: https://perfht.ml/2GZCPDA
there are a few styles and reflow markers which have cause callstacks. However, the C++ stacks in there look rather odd. They are all only two functions deep:

profiler_get_backtrace()   xul.pdb
RtlAllocateMemoryBlockLookaside   ntdll.pdb

Is synchronous unwinding of the native stack just broken on Windows at the moment?
I think Xidron said he would try to take a look at this when he has some free cycles?
Flags: needinfo?(xidorn+moz)
Priority: -- → P2
OK, I guess I know what the problem is.
Assignee: nobody → xidorn+moz
Flags: needinfo?(xidorn+moz)
Component: Gecko Profiler → mozglue
Comment on attachment 8983010 [details]
Bug 1452204 part 1 - Correctly set walkCallingThread.

https://reviewboard.mozilla.org/r/248864/#review255274
Attachment #8983010 - Flags: review?(mh+mozilla) → review+
Comment on attachment 8983011 [details]
Bug 1452204 part 2 - Use RtlCaptureContext to capture context for current thread and remove walker thread.

https://reviewboard.mozilla.org/r/248866/#review255282
Attachment #8983011 - Flags: review?(mh+mozilla) → review+
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/72fc40daf6cd
part 1 - Correctly set walkCallingThread. r=glandium
https://hg.mozilla.org/integration/autoland/rev/4431cecd4c2d
part 2 - Use RtlCaptureContext to capture context for current thread. r=glandium
Backout by ebalazs@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a47957da1983
Backed out 2 changesets for perma failing in memory/replace/dmd/test/test_dmd.js
Backed out 2 changesets (bug 1452204) for perma failing in memory/replace/dmd/test/test_dmd.js

Push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=4431cecd4c2d43fedc0acfa5016ef89a583d87b6&filter-resultStatus=testfailed&filter-resultStatus=busted&filter-resultStatus=exception&filter-resultStatus=retry&filter-resultStatus=usercancel&filter-resultStatus=running&filter-resultStatus=pending&filter-resultStatus=runnable&selectedJob=181841156

Backout: Backed out 2 changesets (bug 1452204) for perma failing in memory/replace/dmd/test/test_dmd.js

Failure log: Backed out 2 changesets (bug 1452204) for perma failing in memory/replace/dmd/test/test_dmd.js
Flags: needinfo?(xidorn+moz)
OK, eventually I figured out what's going on.

It seems we can remove quite a bit of code for walking the current thread.
Flags: needinfo?(xidorn+moz)
Comment on attachment 8983011 [details]
Bug 1452204 part 2 - Use RtlCaptureContext to capture context for current thread and remove walker thread.

This change is non-trivial and definitely needs another review.
Attachment #8983011 - Flags: review+ → review?(mh+mozilla)
Comment on attachment 8983011 [details]
Bug 1452204 part 2 - Use RtlCaptureContext to capture context for current thread and remove walker thread.

https://reviewboard.mozilla.org/r/248866/#review256098

::: commit-message-4087b:8
(Diff revision 2)
> +GetThreadContext() returns a context pointing to its own frame when it
> +gets called with the current thread handle. That frame can go away after
> +it returns. This patch instead uses RtlCaptureContext(), which captures
> +the context of its caller, when walking the current thread.
> +
> +In the past, we also use a walker thread when nullptr is passed in for

s/use/used/

::: commit-message-4087b:10
(Diff revision 2)
> +it returns. This patch instead uses RtlCaptureContext(), which captures
> +the context of its caller, when walking the current thread.
> +
> +In the past, we also use a walker thread when nullptr is passed in for
> +aThread, but the check doesn't cover all the cases, and having another
> +thread is apparently more complicated then this approach.

s/then/than/
Attachment #8983011 - Flags: review?(mh+mozilla) → review+
Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/6df1a71c5fe2
part 1 - Correctly set walkCallingThread. r=glandium
https://hg.mozilla.org/integration/autoland/rev/12b8065ee9fc
part 2 - Use RtlCaptureContext to capture context for current thread and remove walker thread. r=glandium
https://hg.mozilla.org/mozilla-central/rev/6df1a71c5fe2
https://hg.mozilla.org/mozilla-central/rev/12b8065ee9fc
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Depends on: 1468129
You need to log in before you can comment on or make changes to this bug.