1166126 - Frame parsing issues with giant URLs

Status: RESOLVED FIXED

(DevTools :: Performance Tools (Profiler/Timeline), defect)

Description

[Jordan Santell [:jsantell] [@jsantell]]

When location does not contain a function, like a long URL, the following becomes its "functionName", meaning we print this in the call tree:

https://www.yahoo.com/sy/zz/combo?nn/lib/metro/g/uicontrib/rapid_2.9.11.js&nn/lib/metro/g/yui/oop_3.8.3.js&nn/lib/metro/g/yui/event-custom-base_3.8.3.js&nn/lib/metro/g/yui/event-base_3.8.3.js&nn/lib/metro/g/yui/io-base_3.8.3.js&nn/lib/metro/g/yui/json-parse_3.8.3.js&nn/lib/metro/g/yui/json-stringify_3.8.3.js&nn/lib/metro/g/yui/cookie_3.8.3.js&nn/lib/metro/g/yui/dom-core_3.8.3.js&nn/lib/metro/g/yui/dom-base_3.8.3.js&nn/lib/metro/g/yui/dom-style_3.8.3.js&nn/lib/metro/g/yui/dom-screen_3.8.3.js&nn/lib/metro/g/

Sometimes giant URLs can also have a function, but we do not parse it out correctly, resulting in another giant URL, I believe the last parentheses is not making its way from the profiler, and it's getting cut off.

g (https://www.yahoo.com/sy/zz/combo?nn/lib/metro/g/uicontrib/rapid_2.9.11.js&nn/lib/metro/g/yui/oop_3.8.3.js&nn/lib/metro/g/yui/event-custom-base_3.8.3.js&nn/lib/metro/g/yui/event-base_3.8.3.js&nn/lib/metro/g/yui/io-base_3.8.3.js&nn/lib/metro/g/yui/json-parse_3.8.3.js&nn/lib/metro/g/yui/json-stringify_3.8.3.js&nn/lib/metro/g/yui/cookie_3.8.3.js&nn/lib/metro/g/yui/dom-core_3.8.3.js&nn/lib/metro/g/yui/dom-base_3.8.3.js&nn/lib/metro/g/yui/dom-style_3.8.3.js&nn/lib/metro/g/yui/dom-screen_3.8.3.js&nn/lib/metro

Comment 1

[Jordan Santell [:jsantell] [@jsantell]]

Hopefully removing the queries from the URLs cleans this up a bit in bug 1049865, but I think we'll need to do some more magic so these giant strings never appear anywhere.

Comment 2

[Jordan Santell [:jsantell] [@jsantell]]

Yap, profiler only contains 512 characters for the location, which means when we parse, we're going to get an ugly response either way

Comment 3

[Jordan Santell [:jsantell] [@jsantell]]

Attachment: Screen Shot 2015-05-18 at 6.53.41 PM.png

Example of what gets rendered.

Comment 4

[Victor Porof [:vporof][:vp]]

(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #2)
> Yap, profiler only contains 512 characters for the location, which means
> when we parse, we're going to get an ugly response either way

oh god

Comment 5

[Victor Porof [:vporof][:vp]]

Can we just not bake in those strings in the profile json? AFAIK, we have those strings separate in the first place, but weld them together at some point, then breaking them apart in the frontend. This is terrible.

Comment 6

[Jordan Santell [:jsantell] [@jsantell]]

Probably a question for Mr. Shu-yu Guo

Comment 7

[Shu-yu Guo [:shu]]

Do you have a raw JSON I could look at?

Comment 8

[Shu-yu Guo [:shu]]

Attachment: Increase the size of the tag buffer in the profiler.

Comment 9

[Markus Stange [:mstange]]

Comment on attachment 8607322 [details] [diff] [review]
Increase the size of the tag buffer in the profiler.

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

Are we still using dynamic tags for JS frame URLs? I thought we "symbolicated" JS frame when we save the profile? Or did that all just happen in my head?

Comment 10

[Shu-yu Guo [:shu]]

(In reply to Markus Stange [:mstange] from comment #9)
> Comment on attachment 8607322 [details] [diff] [review]
> Increase the size of the tag buffer in the profiler.
> 
> Review of attachment 8607322 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Are we still using dynamic tags for JS frame URLs? I thought we
> "symbolicated" JS frame when we save the profile? Or did that all just
> happen in my head?

For JIT frames, since we save the instruction pointers for those and "symbolicate" them at streaming time. Interpreter frames (which is what's hitting this limit) is still using the pseudo stack, unfortunately.

Comment 11

[Markus Stange [:mstange]]

Oh, I see. That is unfortunate.

Comment 12

[Pulsebot]

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

Comment 13

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

https://hg.mozilla.org/mozilla-central/rev/b1c0b8cb1e6b

Comment 14

[Jordan Santell [:jsantell] [@jsantell]]

Profiler patch landed, but unsure if anything else needs to be done on the client, reopening

Comment 15

[Jordan Santell [:jsantell] [@jsantell]]

This looks good -- parsing giant URLs. Closing.

Comment 16

[Firefox Bug Husbandry Bot]

Removing leave-open keyword from resolved bugs, per :sylvestre.