Ticket #2469 (closed enhancement: fixed)

Opened 4 years ago

Last modified 3 years ago

Use cProfile for twistd profiling on 2.5

Reported by: itamarst Owned by: therve
Priority: highest Milestone:
Component: core Keywords:
Cc: jack, twonds, zooko, dreid Branch: branches/cprofile-2469
Author: therve Launchpad Bug:

Description


Attachments

cprofile.diff Download (0.6 KB) - added by jack 4 years ago.
patch to add cprofile support (falls back to profile for older pythons)

Change History

Changed 4 years ago by jack

  • cc jack added

Changed 4 years ago by jack

patch to add cprofile support (falls back to profile for older pythons)

Changed 4 years ago by jack

  • cc twonds added

Changed 3 years ago by therve

  • owner changed from glyph to therve

Changed 3 years ago by zooko

  • cc zooko added

I just looked at the attached diff and it looks right. What's the next step to push this into trunk?

In a related issue:

 http://docs.python.org/lib/module-hotshot.html

-- this says "For common usages it is recommended to use cProfile instead. hotshot is not maintained and might be removed from the standard library in the future."

So perhaps the logic of using hotshot by default and falling back to profile/cProfile should be reversed to use profile/cProfile by default.

Changed 3 years ago by therve

The problem is that currently twistd is very hard to test (because of a structure problem), and no feature will be added before we manage to test it.

The ticket with the highest priority on this subject is #2571, but I'm unable to progress on it. Any help here is welcome :).

Changed 3 years ago by therve

  • branch set to branches/cprofile-2469
  • author set to therve

(In [22523]) Branching to 'cprofile-2469'

Changed 3 years ago by therve

(In [22524]) Add cprofile runner, with tests.

Refs #2469

Changed 3 years ago by therve

  • priority changed from low to highest
  • owner therve deleted
  • keywords review added

Changed 3 years ago by exarkun

  • owner set to therve
  • keywords review removed

twisted.test.test_twistd.AppProfilingTestCase.test_cProfile and twisted.test.test_twistd.AppProfilingTestCase.test_cProfileSaveStats fail on the dumb debian pstats-less configuration. Guess they should check pstats availability and skip if it is not around.

Changed 3 years ago by therve

  • keywords review added
  • owner therve deleted

Thanks for the check, it looks better.

Changed 3 years ago by dreid

  • owner set to therve

Great work as always therve, however my gut reaction is that the AppProfiler.profilers dictionary should be keyed on the module name. i.e. profile, hotshot, and cProfile with an uppercase P.

In fact we call self._reportImportError("cProfile", e) when "cprofile" is specified on the command line, so I definitely think this needs to be addressed.

Changed 3 years ago by dreid

  • cc dreid added
  • keywords review removed

Changed 3 years ago by therve

(In [22621]) Rename option flag to cProfile.

Refs #2469

Changed 3 years ago by therve

  • keywords review added
  • owner therve deleted

Changed 3 years ago by dreid

  • keywords review removed
  • owner set to therve

Thanks therve, this looks great, merge away.

Changed 3 years ago by therve

  • status changed from new to closed
  • resolution set to fixed

(In [22627]) Merge cprofile-2469

Author: therve Reviewers: dreid, exarkun Fixes #2469

Add cProfile to the list of available profilers for twistd.

Note: See TracTickets for help on using tickets.