Skip to content

Conversation

yselkowitz
Copy link
Collaborator

@natoscott
Copy link
Contributor

That look's great, thanks so much @yselkowitz

@hhorak
Copy link
Contributor

hhorak commented Sep 5, 2025

Thanks, let's wait with merging before @kurik or @wcohen have chance to look at.

What I'm not sure myself is explicitly listing dependencies - chan, HdrHistogram_c, js-d3-flame-graph. Those should be pulled automatically and in case PCP+grafana moves to another package to provide that functionality, those components here might be pulled unnecessarily. Is there a reason to list those dependencies explicitly, @yselkowitz?

@yselkowitz
Copy link
Collaborator Author

yselkowitz commented Sep 5, 2025

If you look at the commits individually, it will make more sense. They were already there, and I just copied the existing config for c10s so that the main (eln) config could be updated, while dropping an obsolete duplicate config in the process.

That being said, I agree that mere dependencies should NOT be listed for the reason you give. chan and HdrHistogram_c are still pcp deps, but can and probably should be delisted.

However js-d3-flame-graph is not a dependency of anything, and I'm having trouble finding if it ever was. @natoscott you did the Fedora package review for its addition, what purpose does it serve?

@natoscott
Copy link
Contributor

@yselkowitz I believe it is/was used by perf(1). Similar/same code exists (embedded?) in grafana-pcp but I don't know all the details there - Sam Feifer may be able to provide more insight there if needed.

@yselkowitz
Copy link
Collaborator Author

OK, grafana-pcp bundles it, but that doesn't require a standalone package. I don't see anything with perf though.

Am I missing something, or is the standalone js-d3-flame-graph superfluous and can just be dropped?

@natoscott
Copy link
Contributor

@yselkowitz
Copy link
Collaborator Author

Thanks for clarifying, I delisted chan and HdrHistogram_c as discussed above, they'll stay in ELN as dependencies as long as pcp requires them.

The performance config was an outdated duplicate.
chan and HdrHistogram_c are runtime dependencies of pcp subpackages, but
should not be explicitly listed, so that they'll be dropped if that ever
changes.

js-d3-flame-graph is a documented use case and therefore remains:

https://docs.redhat.com/en/documentation/red_hat_enterprise_linux/10/html/monitoring_and_managing_system_status_and_performance/getting-started-with-flamegraphs
@yselkowitz yselkowitz merged commit fa871ef into minimization:main Sep 11, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants