phacility / xhprof

XHProf is a function-level hierarchical profiler for PHP and has a simple HTML based user interface.
http://pecl.php.net/package/xhprof
Apache License 2.0
2.6k stars 925 forks source link

Implement PowerPC support #67

Closed clbr closed 8 years ago

clbr commented 9 years ago

See: https://bugs.php.net/bug.php?id=69744

All tests pass and the example works as expected.

Signed-off-by: Lauri Kasanen cand@gmx.com

edelsohn commented 9 years ago

Any progress with the pull request?

epriestley commented 8 years ago

It's important that we be able to test code we bring into the upstream to make sure it really works and test modifications to it. For example, someone else might report an issue with this and we may have no idea how to move forward if we can't even build the code.

From a discussion on IRC, there's no apparent way for me (or future maintainers) to test this code on a PowerPC system without investing thousands of dollars, or days of work, or both. These are very high barriers and it is difficult for us to find the time or resources to pursue this, particularly given the limited value to users this high barrier implies.

The best options I'm currently aware of are:

Without reliable access to PowerPC hosts, this patch isn't something we can test or maintain and isn't a good candidate for bringing upstream.

So this is blocked by finding a way to build and test this patch which is reasonable for any contributor to follow, reliable, and not substantially more complex, expensive or time consuming than launching VMs in AWS or buying hardware from an electronics retailer.

edelsohn commented 8 years ago

I'm not sure why you believe those are the only options for access to Power Systems or who you asked on IRC. First, the VM at Unicamp is not limited to one month. There also are systems at the Oregon State University Power development cloud

http://osuosl.org/services/powerdev

IBM China Research Lab SuperVessel

https://ptopenlab.com/cloudlabconsole/

Power Development cloud at Brno University

https://fit-rhlab.rhcloud.com/powerlinux-openpower-development-hosting/

epriestley commented 8 years ago

...who you asked on IRC.

@clbr, the user who submitted this pull request.

I'm not sure why you believe those are the only options for access to Power Systems

These were the suggestions @clbr had. You can review the conversation here:

https://secure.phabricator.com/chatlog/channel/6/?at=212937

First, the VM at Unicamp is not limited to one month.

I was just reading this section:

Terms of Use: Each machine will be available for 1 month. If more time is needed, please email openpower@ic.unicamp.br requesting that the deadline be extended.

epriestley commented 8 years ago

If there is no reasonable commercial/consumer access to modern PowerPC systems, this patch is a nonstarter for us as the upstream. We simply don't have the free time to hunt down research systems from universities and deal with managing those relationships. I'm not comfortable with a hypothetical build process that begins "email so-and-so at such-and-such university and ask for some hardware".

We have greater access to financial resources than developer time, but upstreaming this patch isn't worth $3K+ to us, nor is a build process that begins "purchase a $3,000 rack unit" acceptable.

We need to be able to build this code reliably and have a reasonable build process that is available to other future contributors. It sounds like access to Power systems is essentially at IBM's mercy, and that these systems do not exist at the consumer level? This is not compatible with our capabilities as an upstream. It is also not well aligned philosophically with the open nature of this project or our organization.

edelsohn commented 8 years ago

I don't know who @clbr is.

If you don't need a persistent system for Continuous Integration Testing, I don't see the problem with 1 month default access for intermittent testing. OSUOSL provides persistent shell accounts in addition to VMs. I mentioned free access for FOSS developers, there also are VMs available for pay at SiteOx and OVH. Requesting an account does not involve "days of work". There are many options available. And this particular request is connected to a bounty on Bountysource.com, which can defray some of the inconvenience that you assert.

I don't understand the hostile tone and the repeated inaccurate information about developer access to systems used as justification.

edelsohn commented 8 years ago

All other Open Source projects with which I work are proud of their code and want it to work on as many systems and configurations as possible.

Many other Open Source projects are using the various Power Systems clouds for testing. They don't seem to have a problem.

The Power Systems clouds at OSUOSL, FIT Brno and Unicamp are operated by their respective universities, not IBM. And one doesn't start off the request process by sending an email to such-and-such university.

I don't know why you feel the need to start using language like "at IBM's mercy".

An open project would accept properly submitted patches to fix bugs and support more targets.

epriestley commented 8 years ago

I don't know who @clbr is.

@clbr is the author of this pull request. He wrote the code you are interested in bringing upstream.

I don't see the problem with 1 month default access for intermittent testing.

I understand that this is probably reasonable for most projects, but this isn't acceptable to me.

There are many options available.

There don't seem to be any commercial/consumer options available with a comparable availability, complexity and expense to other architectures (e.g., EC2 in AWS).

And this particular request is connected to a bounty on Bountysource.com, which can defray some of the inconvenience that you assert.

Our hourly rate is much higher than the total payout. The bounty would not defray even the time I've spent discussing this issue on this ticket, and @clbr is presumably expecting to receive this award.

I am also quite concerned that this is the first I'm hearing about this.

All other Open Source projects with which I work are proud of their code and want it to work on as many systems and configurations as possible.

Unfortunately, this isn't an attitude we share. We have very limited engineering bandwidth and are extremely concerned about the lifetime maintenance cost of patches. Because this patch is far more difficult to test than most patches, it has a far higher lifetime maintenance cost.

You can find some discussion of our attitudes here:

https://secure.phabricator.com/book/phabcontrib/article/contributing_code/#rejecting-patches

The Bountysource bounty is especially concerning, as it structures this as a payout for @clbr and an ongoing cost for us. It also suggests that there may be zero actual users who desire this feature, since you are presumably motivated by employment at IBM and @clbr is motivated by the bounty (it sounds like he does not even have a Power system himself, and used Unicamp to test his change).

There are several forks of XHProf which may be more permissive and might be more accommodating to supporting the Power architecture. You might consider working with them instead.

clbr commented 8 years ago

The Bountysource bounty is especially concerning, as it structures this as a payout for @clbr and an ongoing cost for us. It also suggests that there may be zero actual users who desire this feature, since you are presumably motivated by employment at IBM and @clbr is motivated by the bounty (it sounds like he does not even have a Power system himself, and used Unicamp to test his change).

FWIW I do have a PowerMac G5, but I did test this change on Unicamp's cloud, as ppc64le was the target architecture.

ohhmm commented 8 years ago

I use schroot with qemu-user on Gentoo