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

Add the definition filename/line to each function #31

Closed
wants to merge 8 commits into from

Conversation

dshafik
Copy link
Contributor

@dshafik dshafik commented Oct 28, 2013

Also defines a boolean key called "internal" to easily check internal vs
userland and therefore easily handle lack of filename/line keys.

The intention is to allow reporting tools (e.g. xhgui) to show/link to
original source code (or say, source on github)

An option would be to pull in the line_start and line_end to allow
easier extraction of just the function from source files.

(Also includes the change in #30 as I ran into issues debugging without that)

@dshafik
Copy link
Contributor Author

dshafik commented Oct 28, 2013

Example code and output:

<?php
xhprof_enable();
function helloWorld() {
    echo "hello world";
}

helloWorld();
helloWorld();

$out = xhprof_disable();

var_dump($out);

And output:

array(3) {
  ["main()==>helloWorld"]=>
  array(5) {
    ["internal"]=>
    bool(false)
    ["filename"]=>
    string(42) "/Users/davey/src/xhprof/extension/test.php"
    ["line"]=>
    int(3)
    ["ct"]=>
    int(2)
    ["wt"]=>
    int(69)
  }
  ["main()==>xhprof_disable"]=>
  array(3) {
    ["internal"]=>
    bool(true)
    ["ct"]=>
    int(1)
    ["wt"]=>
    int(0)
  }
  ["main()"]=>
  array(3) {
    ["internal"]=>
    bool(true)
    ["ct"]=>
    int(1)
    ["wt"]=>
    int(135)
  }
}

@billf
Copy link
Contributor

billf commented Apr 17, 2014

what's the advantage of this over using reflection and manipulating the payload returned by xhprof_disable()? the extension should be as simple as possible and avoid anything that could be done natively in php.

@epriestley
Copy link
Member

This project has a new maintainer, Phacility. We worked with the original developers and maintainers of XHProf at Facebook until 2011/2012. We recently offered to take over maintenance because we use XHProf heavily in our software (Phabricator) while Facebook is now focused on the HHVM platform and relies on XHProf much less than it once did.

I'm trying to clear out the backlog of open requests and issues.

I agree with @billf that this is better done in PHP with reflection than in the extension itself, unless there is some strong argument against that approach. I think it would tentatively be OK to provide support for this in the APIs that manage results, but since we just took over the project I'm hesitant about API changes until we have a chance to review the state of things. If you'd like to proceed: sign the CLA (https://secure.phabricator.com/L28), implement this with Reflection as a convenience wrapper which operates on the results of xhprof_disable(), and resubmit it. We may not want to expand the API for a bit, but I think this capability is conceptually a reasonable one.

@epriestley epriestley closed this Aug 27, 2014
@LionsAd LionsAd mentioned this pull request Nov 6, 2015
@Minasu Minasu mentioned this pull request Jun 20, 2016
yazshel pushed a commit to FluentDevelopment/xhprof that referenced this pull request Dec 3, 2020
phacility#31 Fix macOS time benchmark
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.

6 participants