Profiling panel improvements #1669
Conversation
docs/panels.rst
Outdated
| The panel will include all function calls made by your project if your using | ||
| the setting ``settings.BASE_DIR`` to point to your project's root directory. | ||
| If a function is in a file within that directory and does not include | ||
| ``"/site-packages/"`` in the path, it will be included. |
There was a problem hiding this comment.
I had some strange occurrences of broken Python installations when packages ended up in dist-packages as well. I couldn't really explain the difference -- all I know is that site-packages signify that everything is working well, dist-packages signify that many things will end up broken.
There was a problem hiding this comment.
We could check for that path here too.
There was a problem hiding this comment.
I think it would be prudent to do so, both dist-packages and site-packages are very improbable to exist outside virtualenvs.
|
So a little context here @matthiask, as I was writing up my tutorial I wrote this view that iterated over 50k posts to calculate how many were created in the last 30, 90 and 180 days. I had a function that was called for each iteration, but this Additionally, do you think we should have a killswitch preventing the |
|
@matthiask is it worth adding another setting to prevent this inclusion? |
tim-schilling
force-pushed
the
profiler-improvements
branch
from
7cd1ab0
to
2c100cb
Compare
By checking that the code lives in the settings.BASE_DIR directory, we know that the code was likely written by the user and thus more important to a developer when debugging code.
Co-authored-by: Matthias Kestenholz <mk@feinheit.ch>
Co-authored-by: Matthias Kestenholz <mk@feinheit.ch>
Co-authored-by: Matthias Kestenholz <mk@feinheit.ch>
This can be used to disable the attempt to include all project code. This is useful if dependencies are installed within the project.
tim-schilling
force-pushed
the
profiler-improvements
branch
from
b75a3bc
to
9e897c5
Compare
|
@matthiask can you review the latest changes? If it's still good, we can squash merge it. |
There was a problem hiding this comment.
Thank you!
Additionally, do you think we should have a killswitch preventing the or func.is_project_code() bypass? I'm thinking maybe we get something wrong with site-packages or dist-packages and there is too much to the profiling stats so it doesn't return.
I'm a bit confused by the setting. The default was to skip some frames and now you want to add the option to include the projects' code no matter what? PROFILER_CAPTURE_PROJECT_CODE = False sounds to me as if the profiler wouldn't capture the projects' code at all. Naming is always borderline bikeshedding, but maybe we could avoid finding a good name for the setting by not making this configurable at all for now? Or maybe it's just me. I definitely don't have any requested changes and the code looks sound to me.
Correct. The system currently guesses at what frames it should include. My pitch is that we should include all of the project's code in the profiler. The killswitch I was mentioning ended up being
That's a fair comment. I feel strongly enough about having it be configurable (I can explain if you want me to) that I'd push for us to come up with a better name or merge it as is. |
|
|
Let's move ahead. I want to try this out with a current project :) |
|
Sorry, it was a long weekend for me. I think we can do another version release after #1673 is merged. Probably a minor version bump? |
|
Nice! And yes, let's make a release after that. |
PROFILER_THRESHOLD_RATIOto give usersbetter control over how many function calls are included. A higher value
will include more data, but increase render time.
code is more important to developers than dependency code.