perf: avoid repeatedly re-parsing frames - #27
Conversation
This should reduce the effort on parsing frames by a factor of 4 when operating on a `--full` trace. In that case `ms` will be a list of 4 metrics but we only need to parse the frames once because they are the same in all cases. Also add a couple of comments which weren't obvious to me on first reading.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #27 +/- ##
==========================================
+ Coverage 74.07% 74.08% +0.01%
==========================================
Files 16 16
Lines 1508 1509 +1
Branches 218 219 +1
==========================================
+ Hits 1117 1118 +1
Misses 328 328
Partials 63 63 ☔ View full report in Codecov by Sentry. |
|
@sparrowt Well spotted! Thanks a lot for your contribution 🙏 . I was thinking that perhaps something like a namedtuple or a dataclass would provide for a better API to collect profiles, instead of a plain list. But that's for another time 🙂 . |
P403n1x87
changed the title
|
True yes I think that could simplify things at some point!
|
|
Thanks. I think some sources need to be reformatted with black. Otherwise all seems good! 🙌 |
|
Ah I missed that check (I thought github was supposed to put all the failing ones at the top of the list...!) sorted now |
|
@sparrowt just checking whether there's an extra commit that still needs to be pushed to resolve the lining check 🙂 |
|
Gah, sorted now. I thought I'd made the necessary changes after running black locally but I'd clearly missed one! |
|
Awesome 🙌 Thanks a lot for your contributions! 🙂 |
|
Pleasure! In my limited local testing this was a 30% speedup which I'll take any day |
2 participants
Hi there! Thanks for producing such an excellent tool. This was a small optimisation I spotted while looking at
stats.pyto learn more about the Austin trace/profile file format.This should reduce the effort on parsing frames by a factor of 4 when operating on a
--fulltrace, because in that casemswill be a list of 4 metrics but we only need to parse the frames once because they are the same in all cases.Also add a couple of comments in
Metric.parsewhich weren't obvious to me on first reading, I hope that's ok.