The default bug view has changed. See this FAQ.

Use the mac profiler backend again

RESOLVED FIXED in mozilla13

Status

()

Core
Gecko Profiler
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: jrmuizel, Assigned: jrmuizel)

Tracking

unspecified
mozilla13
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 5 obsolete attachments)

(Assignee)

Description

5 years ago
I'm hoping this will fix up the crash problems we've been seeing.
(Assignee)

Comment 1

5 years ago
Created attachment 594277 [details] [diff] [review]
Add an internal stackwalk API

This will be used by the profiler to be able to unwind arbitrary threads.
Attachment #594277 - Flags: review?(ehsan)
(Assignee)

Comment 2

5 years ago
Created attachment 594278 [details] [diff] [review]
Bring the mac backend back
Attachment #594278 - Flags: review?(ehsan)
(Assignee)

Comment 3

5 years ago
Created attachment 594279 [details]
Add an internal stackwalk API v2
Attachment #594277 - Attachment is obsolete: true
Attachment #594277 - Flags: review?(ehsan)
Attachment #594279 - Flags: review?(ehsan)
(Assignee)

Comment 4

5 years ago
Created attachment 594468 [details] [diff] [review]
Add an internal stackwalk API v3
Attachment #594279 - Attachment is obsolete: true
Attachment #594279 - Flags: review?(ehsan)
Attachment #594468 - Flags: review?(ehsan)
(Assignee)

Comment 5

5 years ago
Created attachment 594469 [details] [diff] [review]
Bring the mac backend back v2

This patches actually work now.
Attachment #594278 - Attachment is obsolete: true
Attachment #594278 - Flags: review?(ehsan)
Attachment #594469 - Flags: review?(ehsan)
(Assignee)

Comment 6

5 years ago
(In reply to Jeff Muizelaar [:jrmuizel] from comment #5)
> Created attachment 594469 [details] [diff] [review]
> Bring the mac backend back v2
> 
> This patches actually work now.

I had an unrefreshed change to make this use sample->fp instead of pc
Comment on attachment 594468 [details] [diff] [review]
Add an internal stackwalk API v3

Review of attachment 594468 [details] [diff] [review]:
-----------------------------------------------------------------

::: xpcom/base/StackWalk.h
@@ +12,5 @@
> + * for the specific language governing rights and limitations under the
> + * License.
> + *
> + * The Initial Developer of the Original Code is the Mozilla Foundation.
> + * Portions created by the Initial Developer are Copyright (C) 2011

2012!

::: xpcom/base/nsStackWalk.cpp
@@ +1573,4 @@
>  
>  #else // not __sun-specific
>  
> +#if (defined(__i386) || defined(__ppc__) || defined(PPC) || defined(__amd64)) && defined(__GNUC__)

Please move the X86_OR_PPC definition back up here and change this to use that macro.
Attachment #594468 - Flags: review?(ehsan) → review+
Comment on attachment 594469 [details] [diff] [review]
Bring the mac backend back v2

Review of attachment 594469 [details] [diff] [review]:
-----------------------------------------------------------------

::: tools/profiler/sps/TableTicker.cpp
@@ +497,4 @@
>    }
>  }
>  #endif
> +*/

This function will still be used on Linux right?  Not sure why you're removing it.  I'm pretty sure this breaks Linux builds!

@@ +501,2 @@
>  
> +//#ifdef USE_NS_STACKWALK

This will also break Linux!

@@ +529,5 @@
>      mozilla::ArrayLength(pc_array),
>      0
>    };
> +#ifdef XP_MACOSX
> +  nsresult rv = FramePointerStackWalk(StackWalkCallback, 1, &array, reinterpret_cast<void**>(pc));

How is the size of this buffer communicated to FramePointerStackWalk?  As far as I can tell, we're just hoping that we won't be more than 1000 frames deep in the stack, right?

@@ +542,4 @@
>      }
>    }
>  }
> +//#endif

See above.
Attachment #594469 - Flags: review?(ehsan) → review-
(Assignee)

Comment 9

5 years ago
Created attachment 596052 [details] [diff] [review]
Bring the mac backend back v3

Address review comments
Attachment #594469 - Attachment is obsolete: true
Attachment #596052 - Flags: review?(ehsan)
(Assignee)

Comment 10

5 years ago
Created attachment 596053 [details] [diff] [review]
Add an internal stackwalk API v4
Attachment #594468 - Attachment is obsolete: true
Attachment #596053 - Flags: review?(ehsan)

Updated

5 years ago
Attachment #596053 - Flags: review?(ehsan) → review+

Updated

5 years ago
Attachment #596052 - Flags: review?(ehsan) → review+
https://hg.mozilla.org/mozilla-central/rev/c5574e55a558
https://hg.mozilla.org/mozilla-central/rev/deb7adeda4eb
Assignee: nobody → jmuizelaar
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla13
You need to log in before you can comment on or make changes to this bug.