Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft: Add profiling to leapp and snactor #676

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

drehak
Copy link
Contributor

@drehak drehak commented Nov 3, 2020

Picking up where #482 left off. For science, profit and fun.

LEAPP_CPROFILE=1 turns on the profiler and shows stats after a successful execution. Maybe an option would be better instead...

pirat89 and others added 2 commits November 2, 2020 19:22
Currently on some machines the run of the leapp / snactor is too
slow. Especially framework itself should be fast but sometimes the
time differences are in order of tens seconds. From this point,
it seems useful to be able to start profilling based on the env
variable when it is needed.
@drehak drehak added the wip label Nov 3, 2020
@centos-ci
Copy link

Can one of the admins verify this patch?

The same profiling code also works in leapp with minimal changes. It
could be useful, although it shows more than 90% of the time spent in
posix.waitpid.

Also import StringIO from six instead.
@leapp-bot
Copy link
Collaborator

Thank you for contributing to the Leapp project!

Please note that every PR needs to comply with the
Leapp Guidelines and must pass all tests in order to be mergable.

If you want to re-run tests or request review, you can use following commands as a comment:

  • leapp-ci build to run unit tests, copr build and e2e tests in OAMG CI
  • e2e tests to run unit tests, copr build and end-to-end tests in Murphy CI (OAMG members only) [OLD PIPELINE]
  • review please to notify leapp developers of review request

Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please consider rerunning the CI by commenting leapp-ci build (might require several comments). If the problem persists, contact leapp-infra.

A value of 1 is still required to turn profiling on.
@zhukovgreen
Copy link
Contributor

Comment on lines +19 to +20
os.environ['LEAPP_CPROFILE'] = os.environ.get('LEAPP_CPROFILE', '0')
profile_enabled = os.environ['LEAPP_CPROFILE'] == '1'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
os.environ['LEAPP_CPROFILE'] = os.environ.get('LEAPP_CPROFILE', '0')
profile_enabled = os.environ['LEAPP_CPROFILE'] == '1'
profile_enabled = os.environ.get('LEAPP_CPROFILE', '0') == '1'

Comment on lines +80 to +81
os.environ['LEAPP_CPROFILE'] = os.environ.get('LEAPP_CPROFILE', '0')
profile_enabled = os.environ['LEAPP_CPROFILE'] == '1'
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
os.environ['LEAPP_CPROFILE'] = os.environ.get('LEAPP_CPROFILE', '0')
profile_enabled = os.environ['LEAPP_CPROFILE'] == '1'
profile_enabled = os.environ.get('LEAPP_CPROFILE', '0') == '1'

Copy link
Contributor

@zhukovgreen zhukovgreen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How do you see the usage of this tool? How actually the profiling process will look like? Could you provide a snippet, what the developer will see?

@drehak drehak changed the title Add profiling to leapp and snactor Draft: Add profiling to leapp and snactor Apr 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants