Reuse frames in profile

[stayallive]

Currently we just insert every frame in the profile, however we can lower the payload size by not doing that and reusing identical frames (which in most applications are the first few frames of every stack) and also reuse identical stacks. This results in a significant smaller payload. This saves on data we need to transmit saving on ms spent transmatting to Sentry and saving Sentry on storage.

From some googling for benchmarks I found that md5(serialize($frame)) would be a good tradeoff between correctness (collisions) and speed.

As a very non-scientific test I deployed this to a production app and looked at similar length (time) and same amount of sample profiles of the same transaction and saw a reduction from 145 KB to 44 KB when downloading the profile JSON from the Sentry UI which I thougt was pretty neat 😄

[Jean85]

Is there a measurable CPU overhead with this approach?

But I would imagine that the saving in data sending would cover that...

[stayallive]

There is an overhead (obviously) when doing 100k runs of one of the tests the wall time on my system goes from 0.45s to 0.95s. For a single runs it is not measurable (with my tools).

I did not try what the impact is on serializing it as JSON and compressing to send of to Sentry, but I assume that it is faster with less data but no idea if true and or a significant number.

I also tried this instead as a hash:

$frameHash = "f{$frame['filename']};a{$frame['abs_path']};m{$frame['module']};fn{$frame['function']};l{$frame['lineno']}";

// and just:

$frameHash = serialize($frame);

This is faster (0.65s and 0.69s respectively) but not sure if it's worth it since this first one is easy to mess up and I'm also not sure how efficient having very large array keys is.

I'm open to better suggestions if you have them (both for better de-dup algo and a way to test this), but from what I can gather this seems the most efficient and correct way to ensure unique values (if the values are arrays) in PHP.

[armcknight]

@stayallive Thanks for submitting this PR. Looks like the right track. For reference this is how we implemented it in the Cocoa SDK: https://github.com/getsentry/sentry-cocoa/blob/a176fc448cf5f3ea7b188449b8170b14ce6805cf/Sources/Sentry/include/SentryProfilerState.h#L37-L74 (implementation here).

We also similarly do a stack dedupe, as if a leaf function takes more than 10ms the stack will be identical across samples; would you be willing to add that as well?

[stayallive]

That makes a lot of sense, thanks for linking to another implementation, didn’t even look for one… will take a look at the stack dedupe too, makes sense!

[phacops]

Looks good to me. Big plus if you de-duplicate stacks as well.

[stayallive]

Thanks @phacops and @armcknight for the suggestions, implemented stack re-using too (same method as for frames).

[Zylphrex]

Not sure if this is applicable for PHP but in python, since we're able to use a triple composed of

• abs_path
• lineno
• function
  as the map key. Normally abs_path and lineno suffice already but python allows you to patch over functions so we added function to dedupe that case.

Because of this, one thing we were able to do in python is to defer deriving the extra fields until later. Here, it looks like the work $file can be deferred until after checking if the frame doesn't exist.

[Jean85]

Wouldn't abs_path and lineno be not unique with recursive calls?

[stayallive]

Profiles are from the callee perspective not the caller right? If so I think abs_path and lineno would indeed be enough since in PHP we don't have monkey patching... so that would remove the need for hashing and we can also indeed skip some more work if we already have the frame.

[Zylphrex]

Wouldn't abs_path and lineno be not unique with recursive calls?

Why wouldn't it be unique? It's the same function, just called recursively. So we can use the same frame index to represent it.

[cleptric]

🪨