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

gh-100403: Collect GC statistics via -X option #100958

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

Conversation

pablogsal
Copy link
Member

@pablogsal pablogsal commented Jan 11, 2023

@pablogsal
Copy link
Member Author

Example plot this tool does for ./python -m black --check Lib (black over the whole stdlib):

lel

@nascheme
Copy link
Member

For testing with Python 3.11.x, this is a version ported on top of v3.11.1 .

The pystats.h header is not really needed so I removed the references to it. I also added a check for the PYTHON_GC_STATS env var. That keeps stderr cleaner when you are building stuff and not actually wanting stats. Rather than hard-coding /tmp/py_stats you could get that path from an env var too. An alternative idea would be to have a new -X flag that enables the stats.

I'm planning to collect stats on some applications but haven't got too far on that yet. Initial indication is that the plots look quite different depending on application.

@pablogsal
Copy link
Member Author

That keeps stderr cleaner when you are building stuff and not actually wanting stats. Rather than hard-coding /tmp/py_stats you could get that path from an env var too. An alternative idea would be to have a new -X flag that enables the stats.

For this PR I opted to mirror the workings and contracts of the other interpreter stats collection that were added in the specializing interpreter to not end with many different ways to activate and retrieve statistics.

If set, youngest generation threshold will be set to value.
Use shorter labels for generations.  Show unit as y-axis label on
bar charts.
@nascheme
Copy link
Member

I pushed some changes here: https://github.com/nascheme/cpython/commits/gc_stats in case you want to cherry-pick or merge.

@pablogsal pablogsal marked this pull request as ready for review January 21, 2023 15:59
@pablogsal
Copy link
Member Author

pablogsal commented Jan 21, 2023

@nascheme I have implemented the stats via a -X option where the filename is provided there. This should be a first baseline for the stats collection and we can add public interfaces in the future as I am keeping the containers around. Let me know if you think we should do further modifications.

@pablogsal pablogsal changed the title gh-100403: Collect GC statistics when --enable-pystats is provided gh-100403: Collect GC statistics via -X option Jan 21, 2023
Copy link

@bedevere-bot bedevere-bot left a comment

Choose a reason for hiding this comment

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

Pull request does not meet quality requirements.

@@ -80,7 +80,6 @@ typedef struct _stats {
ObjectStats object_stats;
} PyStats;


Choose a reason for hiding this comment

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

Spurious edit.

Copy link
Member

Choose a reason for hiding this comment

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

Sir Bedevere obtained new powers?

@@ -151,8 +189,15 @@ _PyGC_InitState(GCState *gcstate)
};
gcstate->generation0 = GEN_HEAD(gcstate, 0);
INIT_HEAD(gcstate->permanent_generation);

Choose a reason for hiding this comment

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

Spurious edit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants