search

pyutils / line_profiler

Line-by-line profiling for Python
Other
2.56k stars 118 forks source link

get_stats: Avoid dict conversion of c_code_map for each element of code_hash_map #236

Closed klauer closed 10 months ago

klauer commented 10 months ago

Issue

For each call of get_stats, the implicit conversion of the C++ unordered_mapping c_code_map to a Python dict is currently being performed len(code_hash_map) -times. In scenarios where a large amount of code is being profiled, this can result in a significant performance penalty.

The fix in this PR

Moving the implicit conversion out of the loop and only performing it once is what this PR proposes.

In numbers

In our use case, line-profiler 4.x has been unusable due to this performance hit (which I've only just quantified and narrowed down to this conversion today):

With len(code_hash_map) at around 1000, line_profiler main branch shows for me:

   ncalls  tottime  percall  cumtime  percall filename:lineno(function)
        2    2.043    1.021    2.044    1.022 _line_profiler.pyx:358(get_stats)

And with this PR:

        2    0.010    0.005    0.010    0.005 _line_profiler.pyx:358(get_stats)
Erotemic commented 10 months ago

Great catch. Looks like a simple and easy fix.

@Theelx any comments? Otherwise I'll make a patch release ASAP.

codecov[bot] commented 10 months ago

Codecov Report

Merging #236 (73b8385) into main (17387b3) will not change coverage. The diff coverage is n/a.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/pyutils/line_profiler/pull/236/graphs/tree.svg?width=650&height=150&src=pr&token=xIK8nFU3K5&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyutils)](https://app.codecov.io/gh/pyutils/line_profiler/pull/236?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyutils) ```diff @@ Coverage Diff @@ ## main #236 +/- ## ======================================= Coverage 53.36% 53.36% ======================================= Files 11 11 Lines 832 832 Branches 168 168 ======================================= Hits 444 444 Misses 329 329 Partials 59 59 ``` ------ [Continue to review full report in Codecov by Sentry](https://app.codecov.io/gh/pyutils/line_profiler/pull/236?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyutils). > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyutils) > `Δ = absolute (impact)`, `ø = not affected`, `? = missing data` > Powered by [Codecov](https://app.codecov.io/gh/pyutils/line_profiler/pull/236?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyutils). Last update [17387b3...73b8385](https://app.codecov.io/gh/pyutils/line_profiler/pull/236?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyutils). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=pyutils).
Theelx commented 10 months ago

Looks good to me! Thanks for catching this @klauer!

klauer commented 10 months ago

Thanks for the great tool and the quick response on this, @Theelx and @Erotemic!