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.

Sign up for GitHub

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

Jump to bottom

Measure if _warnings.c is still worth having #69126

Closed
brettcannon opened this issue Aug 25, 2015 · 4 comments
Closed

Measure if _warnings.c is still worth having #69126

brettcannon opened this issue Aug 25, 2015 · 4 comments
Assignees
@brettcannon
Labels
performance Performance or resource usage

Comments

@brettcannon
Copy link
Member

BPO 24938
Nosy @brettcannon
Files
  • issue24938.diff: Hack to create _frozen_warnings
    		•

    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:

    assignee = 'https://github.com/brettcannon'
closed_at = <Date 2015-09-06.18:42:28.459>
created_at = <Date 2015-08-25.18:49:27.517>
labels = ['performance']
title = 'Measure if _warnings.c is still worth having'
updated_at = <Date 2015-09-06.18:42:28.457>
user = 'https://github.com/brettcannon'

    bugs.python.org fields:

    activity = <Date 2015-09-06.18:42:28.457>
actor = 'brett.cannon'
assignee = 'brett.cannon'
closed = True
closed_date = <Date 2015-09-06.18:42:28.459>
closer = 'brett.cannon'
components = []
creation = <Date 2015-08-25.18:49:27.517>
creator = 'brett.cannon'
dependencies = []
files = ['40269']
hgrepos = []
issue_num = 24938
keywords = ['patch']
message_count = 4.0
messages = ['249150', '249152', '249213', '250004']
nosy_count = 1.0
nosy_names = ['brett.cannon']
pr_nums = []
priority = 'low'
resolution = 'works for me'
stage = 'needs patch'
status = 'closed'
superseder = None
type = 'performance'
url = 'https://bugs.python.org/issue24938'
versions = ['Python 3.6']
    @brettcannon
    Copy link
    Member Author

    _warnings.c was initially created to help with startup performance. It turned out to be a tricky bit of code to get right to continue to support the Python version of the module.

    But now that we live in a world where we have startup benchmarks instead of hunches and we freeze code like importlib, maybe it's time to re-evaluate whether warnings.py is such a bad thing to have as part of the startup process? I would be curious to know what the performance impact is if we made _warnings the frozen version of warnings.py instead of the C code and measured the startup performance.

    @brettcannon brettcannon added the performance Performance or resource usage label Aug 25, 2015
    @brettcannon brettcannon self-assigned this Aug 25, 2015
    @brettcannon
    Copy link
    Member Author

    I should also mention the other motivating factor was providing C access to the warnings system, but once again importlib blazed that trail already by providing a C API which simply uses the Python code.

    @brettcannon
    Copy link
    Member Author

    Here are the benchmark results of freezing warnings.py as _frozen_warnings (very quick hack which doesn't use _frozen_warnings is attached). Basically -S slows down by about 3% and normal startup has no measurable impact. Not sure if that's worth it to simplify the warnings code or not.

    INFO:root:Automatically selected timer: perf_counter
    INFO:root:Skipping benchmark bzr_startup; not compatible with Python 3.6
    INFO:root:Skipping benchmark hg_startup; not compatible with Python 3.6
    Running normal_startup...
    INFO:root:Running ../cpython/default/python.exe -c 1000 times
    INFO:root:Running ../cpython/pristine/python.exe -c 1000 times
    Running startup_nosite...
    INFO:root:Running ../cpython/default/python.exe -S -c 2000 times
    INFO:root:Running ../cpython/pristine/python.exe -S -c 2000 times

    Report on Darwin Bretts-MacBook-Pro.local 14.5.0 Darwin Kernel Version 14.5.0: Wed Jul 29 02:26:53 PDT 2015; root:xnu-2782.40.9~1/RELEASE_X86_64 x86_64 i386
    Total CPU cores: 4

    ### startup_nosite ###
    Min: 0.311247 -> 0.317280: 1.02x slower
    Avg: 0.316682 -> 0.325945: 1.03x slower
    Significant (t=-17.89)
    Stddev: 0.00326 -> 0.00403: 1.2360x larger

    The following not significant results are hidden, use -v to show them:
    normal_startup.

    @brettcannon
    Copy link
    Member Author

    After thinking about it and with _warnings.c not exactly changing very much I think I'm going to call YAGNI on ripping out _warnings.c.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @brettcannon brettcannon

    Labels
    performance Performance or resource usage
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    1 participant
    @brettcannon