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

JSON format profile printer (additions) #326

Merged
merged 3 commits into from Oct 27, 2012

Conversation

iconara
Copy link
Contributor

@iconara iconara commented Oct 1, 2012

This is some additions to pull request #270, as per the discussion there.

I've added a layer of indirection between Ruby#printProfileData and the profile printers to make it possible to write a profile to a file. I also added ProfilePrinter#printHeader and #printFooter to make it possible to write all thread profiles into the same file when the format is HTML or JSON, without the output being all weird. I've also updated all profile printers to support this.

All changes should be backwards compatible, and I've marked the old version of Ruby#printProfileData that took a PrintStream as deprecated.

I've added an optional argument to --profile.xxx that is the file to write to. Please check that part extra carefully so that I haven't done anything stupid. There wasn't any other argument that took optional parameters on the form --xyz optarg, only arguments like -Ioptarg or that always expected a value. The way I did it was by checking if there is one more argument, and making sure that argument doesn't start with a dash. It will fail in this case: ruby --profile.json hello_world.rb, because hello_world.rb will be interpreted as the argument to --profile.json, not the script to run. I don't know if this is acceptable. Maybe it would be better to do it like --profile.json=output.json, but that's not how the other arguments that take values work (e.g. --compat RUBY_18).

I'm not entirely satisfied with the way profiles are printed, and I'd like to do some more changes to structure it differently, but I think that another pull request that can aim for a later release.

Support for writing profile reports to files, notably support for writing profiles from multiple threads to the same JSON or HTML file without syntax errors.

There's a new abstraction that handles profile printing: ProfileOutput. Together with new methods on ProfilePrinter, printHeader, printFooter and a new parameter to printProfile (first), it can orchestrate the the profile printers so that they print the right things at the right time.
@enebo
Copy link
Member

enebo commented Oct 6, 2012

Theo, we will merge this right after final for 1.7.1. rc2 (which very very very likely will be 1.7.0) will be out on Monday and although this all looks solid to me is just a tad too close to this release.

@iconara
Copy link
Contributor Author

iconara commented Oct 6, 2012

Thanks! Getting it into 1.7.0 would have been awesome, but I completely understand that that is not possible, no problem at all. I'm just happy that you like it.

headius added a commit that referenced this pull request Oct 27, 2012
@headius headius merged commit 7ba1531 into jruby:master Oct 27, 2012
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.

None yet

3 participants