Add support for Safari profiles

[radex]

Closes #294

This adds import for Safari/webkit profiler. Well, for Safari 13.1 for sure, I haven't done any work to check if there's been changes to the syntax.

It seems to work OK, and is already a huge improvement over profiling in Safari (which doesn't even have a flame graph, let alone something like left heavy). Sadly, the sampler resolution is only 1kHz, which is not super useful for a lot of profiling work. I made a ticket on webkit bug tracker to ask for 10kHz/configurable sampling rate: https://bugs.webkit.org/show_bug.cgi?id=214866

Another thing that's missing is that I cut out all the idle time. We could also insert layout/paint samples into the timeline by parsing events. But I'll leave that for another time.

[coveralls]

Coverage increased (+0.3%) to 47.113% when pulling dff45c9 on Nozbe:safari into 64fe369 on jlfwong:master.

[jlfwong]

Oooooo COOL thanks for doing this!

I haven't had time to read through the code yet, but I'll try do to that this weekend.

Another thing that's missing is that I cut out all the idle time

I'd prefer if it didn't do this. It can be helpful to keep them consistent to compare information from the Safari devtools & speedscope, and also to see gaps between frame renders. The left-heavy view should already remove all the idle time for doing % comparisons.

Thanks again for the contribution, including adding tests!

[radex]

I'd prefer if it didn't do this. It can be helpful to keep them consistent to compare information from the Safari devtools & speedscope, and also to see gaps between frame renders.

I agree, but I couldn't quickly figure out how to add samples that don't touch each other using StackListProfileBuilder - I can append new samples by providing their timestamp or their weight, but I don't know how to add "empty space". If you point me in the right direction, I'll give it a try

[jlfwong]

@radex I think what you want is profile.appendSampleWithWeight([], idleDuration). Does that work?

[radex]

@jlfwong Ah! I didn't think of that :) That should work, I'll take a look at it.

[jlfwong]

Hi @radex! Do you think you'll have time to fix the idle time issue soon? I'm planning on tweeting about some of the newer speedscope features and would love to include this, but don't want to hold off the launch for too long, and also don't want to rush you if you're busy with other things.

I'm also happy to take this over if you'd like, but don't want to deprive you of the satisfaction of finishing this :)

[radex]

@jlfwong Let me take a stab at it this morning!

[radex]

And done!

The idle times are not exactly… precise... If I compare stack's timestamps with their durations, they don't line up, and there's always some phantom idle time in between. And it's way more than a rounding error - even if I ignore <1ms gaps in the timeline, there will still show up idle time when I know that actually the same function is executing. So I'm only showing idle time when there's a gap >2ms. I don't know if this is Safari's fault or mine, but this should be good enough for most purposes...

[radex]

@jlfwong BTW: Would you mind taking a look at #301 ? I'd love to implement a simple version of this feature, but I'd prefer only doing it if you'll be OK with merging it

[jlfwong]

Ah dang, this is unfortunate. From looking at the data, I also don't see a way to resolve this though. I guess it's not super surprising given that Safari doesn't provide any way of visualizing this data in a flamechart.

[jlfwong]

Do you have the text for this written up in markdown somewhere so I can copy/paste it into the wiki?

[jlfwong]

Nice, this is really clean and simple! The phantom idle time thing is a bummer (I just hit it recording a profile of importing a large profile into speedscope), but I don't see how it's resolvable with the data provided.

Before this lands, I just need the content for the wiki page, and it looks like somehow the formatting drifted without failing CI. If you run npm run prettier it should clean up a few small formatting issues, then this is good to go!

Sorry this took so long to get around to the review for!

[jlfwong]

I just copied this branch to #313 so I could run prettier on it and merge it. Thank you @radex for doing the work here!

I wrote up my own wiki page here: https://github.com/jlfwong/speedscope/wiki/Importing-from-Safari. Let me know if anything looks amiss.

[radex]

@jlfwong Awesome! Sorry for procreating for a long time with getting this to the finish line, I'm glad it's merged now :)