davidmarkclements / 0x

🔥 single-command flamegraph profiling 🔥
MIT License
3.23k stars 106 forks source link

Workaround for v8 tick processor bug #193

Closed mafintosh closed 6 years ago

mafintosh commented 6 years ago

Should fix https://github.com/davidmarkclements/0x/issues/192 until we can fix it in V8

AlanSl commented 6 years ago

I've tested this on Windows 10 and it appears to fix https://github.com/davidmarkclements/0x/issues/192 so LGTM functionality-wise

AlanSl commented 6 years ago

One observed issue which is maybe something for a separate PR - the names are coming out fine, but 0x is missclassifying some (all?) app and deps frames, treating them as core. For example from 0x/examples/rest-api on Windows:

image

I'm going to do some more tests to see if it always misclassifies or if it's to do with particulars here such as the app code being in a subfolder and the dependency being installed outside of the project folder

AlanSl commented 6 years ago

Does seem to always misidentify app and deps frames as core on Windows. But I think it'd be fine to merge and release this then look at why the classification is failing despite the fixed names a separate issue.

Here's an example sample using this branch of 0x on https://github.com/nearform/node-clinic-flame-demo :

https://upload.clinicjs.org/public/9bcb4eff79f4be15eefd0fd4e3e864e5ab21e7f50020d4a076da7e69524b43b6/10436.clinic-flame.html

mafintosh commented 6 years ago

@AlanSl The misidentify is another issue right? I'm guessing that has nothing to do with the escaping?

AlanSl commented 6 years ago

Probably. Let's release this and I'll post it as a separate issue.

LGTM