python / cpython

The Python programming language
https://www.python.org
Other
62.68k stars 30.06k forks source link

Merge profile/cProfile in 3.n+1 #47168

Closed brettcannon closed 10 years ago

brettcannon commented 16 years ago
BPO 2919
Nosy @warsaw, @brettcannon, @birkenfeld, @abalkin, @avassalotti, @hdiogenes, @benjaminp, @merwok, @asvetlov, @florentx, @ericsnowcurrently
Files
  • profile_module2.tgz: merged profile/cProfile module, rev'd
  • pyprofile.py: Profiler interface for lsprof.
  • lsprof.py: Python version of the _lsprof module (updated).
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields: ```python assignee = 'https://github.com/avassalotti' closed_at = created_at = labels = ['type-bug', 'library'] title = 'Merge profile/cProfile in 3.n+1' updated_at = user = 'https://github.com/brettcannon' ``` bugs.python.org fields: ```python activity = actor = 'alexandre.vassalotti' assignee = 'alexandre.vassalotti' closed = True closed_date = closer = 'alexandre.vassalotti' components = ['Library (Lib)'] creation = creator = 'brett.cannon' dependencies = [] files = ['14156', '14377', '14463'] hgrepos = [] issue_num = 2919 keywords = ['patch'] message_count = 22.0 messages = ['67073', '67896', '68043', '71312', '71392', '71393', '88553', '88613', '88614', '88624', '88762', '89171', '89778', '90205', '97612', '97647', '110190', '110216', '110217', '165958', '199897', '199974'] nosy_count = 13.0 nosy_names = ['barry', 'brett.cannon', 'georg.brandl', 'belopolsky', 'alexandre.vassalotti', 'hdiogenes', 'MrJean1', 'benjamin.peterson', 'eric.araujo', 'asvetlov', 'flox', 'BreamoreBoy', 'eric.snow'] pr_nums = [] priority = 'critical' resolution = 'wont fix' stage = 'patch review' status = 'closed' superseder = None type = 'behavior' url = 'https://bugs.python.org/issue2919' versions = ['Python 3.2'] ```

    brettcannon commented 16 years ago

    cProfile should go away in 3.0 (as a public module) and just be what profile uses when available.

    avassalotti commented 16 years ago

    I will try to fix issue. I cannot promise that I will get it done before the beta though.

    warsaw commented 16 years ago

    This didn't get done for the betas, but we're still going to release. Knocking it down to critical.

    warsaw commented 16 years ago

    Can you get this done before beta 3?

    avassalotti commented 16 years ago

    Sorry, I can't. I would love to get this done, but I am just too busy this week (and next week doesn't look any better). Furthermore, the amount of work that the profile/cProfile merge will require is quite considerable. So, I don't expect someone else could get this done before the beta.

    benjaminp commented 16 years ago

    Let's retarget 3.1 then.

    9cd365b5-2b5d-4849-906c-aa06fadd6be4 commented 15 years ago

    How is this going?

    5996bd1d-e940-4043-9218-45ba363184f6 commented 15 years ago

    Attached is *an* attempt to combine the old profile/cProfile modules into a single one called profile. Only the C and Python source and test files are included, not any documentation files. More details are in the README file, copied below.

    Hopefully, this is useful.

    /Jean Brouwers

    These are the modified files to replace the old profile module with the fast cProfile one and rename the latter. A complete list and description follow.

    ./README

    ./profileNEW/setup.py ./profileNEW/Lib/profile.py ./profileNEW/Lib/test/profilee.py ./profileNEW/Lib/test/test_profile.py ./profileNEW/Modules/_profile.c

    ./profilePy3k/setup.py ./profilePy3k/Lib/profile.py ./profilePy3k/Lib/test/profilee.py ./profilePy3k/Lib/test/test_cprofile.py ./profilePy3k/Lib/test/test_profile.py ./profilePy3k/Modules/_lsprof.c

    ./diff_-u_profilePy3k_Modules_lsprof.c_profileNEW_Modulesprofile.c.txt ./diff-ur_profilePy3k_profileNEW.txt

    ./_profile_hires.c

    Directory profileNEW contains all the source and test files in their corresponding directories. Directory profilePy3k contains the old, original files from the py3k trunk taken on May 31, 2009.

    Basically, the old _lsprof.c file has been modified and renamed to _profile.c. Both old test_profile.py and test_cprofile.py files have been combined into a single new file test_profile.py.

    The file diff_-ur_profilePy3k_profileNEW.txt contains the forward diffences between all the new and orginal files.

    File diff_-u_profilePy3k_Modules_lsprof.c_profileNEW_Modules_profile.c.txt contains just the differences between the old _lsprof.c and new _profile.c files.

    Lastly, for your consideration, file _profile_hires.c is the new _profile.c file enhanced to use a high resolution timer, see \http://bugs.python.org/issue2281\.
    This file would replace the _profile.c file in the profileNEW/Modules directory.

    The new files and tests were installed in a Python 3.0.1 build and compiled and tested on MacOS X 10.4.11 Tiger (Intel). The patches from bpo-5330 were also installed in this build, see \http://bugs.python.org/issue5330\. All tests passed after regenerating the expected results

    % python.exe Lib/test/test_profile.py -r Regenerating Lib/test/test_profile.py...

    % python.exe Lib/test/test_profile.py test_bad_counter_during_dealloc (main.ProfileTest) ... ok test_calling_conventions (main.ProfileTest) ... ok test_profile (main.ProfileTest) ... ok

    ---------------------------------------------------------------------- Ran 3 tests in 0.017s

    OK

    brettcannon commented 15 years ago

    Thanks for the code, Jean. With Python 3.1 about to go out the door this will have to be a 3.2 thing. But I plan to start looking at this module merge some time in July so I should get to looking at what you did then (unless someone beats me to it).

    5996bd1d-e940-4043-9218-45ba363184f6 commented 15 years ago

    I just added another comment about the high-resolution timer in \http://bugs.python.org/issue2281\.

    5996bd1d-e940-4043-9218-45ba363184f6 commented 15 years ago

    Fixed a couple of typos in the README and _profile_hires.c files of the attached profile_module2.tgz tar ball.

    avassalotti commented 15 years ago

    Jean Brouwers wrote:

    All tests passed after regenerating the expected results

    The tests for profile are output test. If you regenerate them, they pass for sure.

    Merging cProfile/profile will require a lot more work than just renaming the module. The designs of cProfile and profile are significantly different. So a successful merge will require settling one profiler design (cProfile is the better designed one, IMHO) and rewrite the other one to match the chosen design.

    avassalotti commented 15 years ago

    Yesterday, I ported the _lsprof module to Python as an experiment. I still have a few issues to iron out, but the core of the functionality is there.

    I am not sure yet how this experiment will fit in the profile/cProfile merge. I wrote the port mainly to get a better idea of what is needed for the merge and how the profiler works.

    avassalotti commented 15 years ago

    Here's an updated version of my lsprof.py module. I fixed a few bugs, includes the awful reference leak that was present in the previous version (i.e., the profiler code was keeping a reference to every frame executed).

    I consider the lsprof.py code functionally equivalent to _lsprof. Now, I am wondering if I should add the features from profile.py missing in _lsprof, e.g., the bias argument and the calibration code.

    avassalotti commented 14 years ago

    Any comment about the code I posted?

    brettcannon commented 14 years ago

    I'll see if I can have a look sometime soon, but my profile experience is practically non-existent so I am not the best person for a code review.

    83d2e70e-e599-4a04-b820-3814bbdb9bef commented 14 years ago

    Is there anyone who could review this, it seems a shame that someone's work has been sitting in limbo for over a year?

    abalkin commented 14 years ago

    Is this a case of pure python implementation with a C speedup? Can you upload the patch to Rietveld? I'll take a look once it is there.

    brettcannon commented 14 years ago

    Alexandre has commit privs so he could just go ahead and commit if he wants.

    I do hope to look at this code at some point in my life. I will see if I can't find time at EuroPython next week, especially if Alexander has fixes to it and still feels the need for a review.

    brettcannon commented 12 years ago

    So I didn't find time at EuroPython. =) Is this dead, Alexandre, or do you want to see this happen for Python 3.4?

    birkenfeld commented 10 years ago

    Last desperate ping for 3.4?

    avassalotti commented 10 years ago

    Let's close this. There is no point keeping this languishing any further.

    The profile and cProfile modules are different enough that we can't merge them without breaking compatibility. They should treated as two separate profilers.

    Maybe in the future we should include a drop-in module written in Python for cProfile to be used for another Python implementation. But for now, I don't see the benefit, since the profile module is good enough and alternate implementations should be able to re-do the _lsprof accelerator in their target language without too much trouble if desired.