1287871 - Talos profiler symbolication is broken

Status: RESOLVED FIXED

(Testing :: Talos, defect)

Description

[Mike Conley (:mconley) (:⚙️)]

Symbols are missing from my local talos runs when running with --spsProfile.

It looks like the symbolication is mostly being skipped. It looks like this is fallout from bug 1282398. Backing that out, I get symbolication occurring again (although the XUL symbols still appear to be missing, strangely).

Comment 1

[Joel Maher ( :jmaher ) (UTC -8)]

interested in investigating this?

Comment 2

[Vishnu (:malayaleecoder)]

Alright, so I ran |mach talcs-test -a ts_paint --spsProfile| before and after the patch, and kept aside the .zip file related to that.

The .zip file is called profile_ts_paint.sps.zip, which on extraction yields a directory with 20 files named cycle_0.sps upto cycle_19.sps

Attached are the cycle_0.sps file before and after the patch.

:mconley, what can I look for in these files? Also am I going in the right path for this bug?

Comment 3

[Vishnu (:malayaleecoder)]

Attachment: afterpatch_cycle_0.sps

Comment 4

[Vishnu (:malayaleecoder)]

Attachment: beforepatch_cycle_0.sps

Comment 5

[Mike Conley (:mconley) (:⚙️)]

Hey malayaleecoder,

Thanks for looking at this! I can see the problem in the afterpatch sps file, and not in the beforepatch sps file.

Here's how I'm checking:

1) Visit cleopatra.io
2) In the "Upload your profile here" file input, I browse to the sps file I'm testing
3) Wait for Cleopatra to process the profile
4) In the right-most column of the resulting view, I see this in the working case:

http://i.imgur.com/KAJduZx.png

and in the not-working case:

http://i.imgur.com/rsScKtt.png

The not-working case is showing raw memory addresses, whereas the working case has successfully mapped the raw memory addresses to actual function names that were being called within the profile. This mapping of memory addresses to function names - that's what we call symbolication.

Comment 6

[Vishnu (:malayaleecoder)]

Attachment: [WIP] Patch

Alright, we have managed to get basestring to work in python3, preserving the symbolication, by defining it as (str, bytes)

The working of the entire code snippet is not known to me. If someone can help me out over here, it would be great.

The possible path we would be traversing will be considering the profiling data to be of bytes[] type ignoring the existing basestring type.

Comment 7

[Vishnu (:malayaleecoder)]

Attachment: Bug1287871_v1.diff

Comment 8

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8780580 [details] [diff] [review]
Bug1287871_v1.diff

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

the question I have here is will this work for python 2.7 for valid files?  I assume so- then the following question is with python 3.0+ will we work fine?  If the answer to both is *yes*, then I am fine with this.

::: testing/talos/talos/profiler/symbolication.py
@@ +18,5 @@
>  
> +try:
> +    basestring
> +except NameError:
> +    basestring = str

for this block of code, I would like a comment above it briefly explaining what problem we are solving by doing this.

Comment 9

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8780580 [details] [diff] [review]
Bug1287871_v1.diff

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

::: testing/talos/talos/profiler/symbolication.py
@@ +18,5 @@
>  
> +try:
> +    basestring
> +except NameError:
> +    basestring = str

I agree.

Will this break with Python 3 though, since we're essentially keeping the status quo if Python 3 is being used (since basestring is missing)? And won't that still be broken?

Comment 10

[Vishnu (:malayaleecoder)]

Attachment: Bug1287871_v2.diff

Comment 11

[Vishnu (:malayaleecoder)]

Attachment: Bug1287871_v3.diff

Comment 12

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8781625 [details] [diff] [review]
Bug1287871_v3.diff

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

thanks, this is easier to understand in the future when we hack on this file for whatever reasons.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/fx-team/rev/40a6c45a56e0
Talos profiler symbolication is broken. r=jmaher

Comment 14

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

https://hg.mozilla.org/mozilla-central/rev/40a6c45a56e0