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

Xdebug 3.0 upgrade appears to exponentially increase the total time (cost). #132

Open
alexscott64 opened this issue Dec 24, 2020 · 8 comments
Labels

Comments

@alexscott64
Copy link

Unfortunately I don't know how or why this is happening.. I'm assuming it has something to do with the way xdebug 3 is now doing profiling output. But, the total inclusive / self cost (in milliseconds) appears to be hyper inflated.

For instance, I have a call that in total, takes 16.66 seconds to run. Loading it into webgrind, it appears that the call takes 1095246 milliseconds or 1095.246 seconds. The percentages seem correct, and in fact the order seems fine too, even when viewing from milliseconds, it just looks like its massively scaled up.

I wouldn't even know where to begin to look into this, but if someone has an idea as to why this might be occurring, or what I could do to help I'd be glad to. Also note I tested the same thing on Xdebug 2 and did not have this issue.

@alpha0010
Copy link
Collaborator

alpha0010 commented Dec 29, 2020

Sounds like there is some sort of number parsing issue, which would be

If you are able to get the cachegrind.out.* files for the same script, run under Xdebug 2 and Xdebug 3, comparing the results may help (the files are plaintext; human readable-ish).

(Sorry, I do not have Xdebug 3 in use currently.)

@alexscott64
Copy link
Author

alexscott64 commented Dec 29, 2020

I've just output the two cachegrind files on the same script. One running Xdebug 3, and one running Xdebug 2. The most noticeable thing is that the cachegrind file for Xdebug 3.0 has the following header:

events: Time_(10ns) Memory_(bytes)

I looked into the library for xdebug and it looks like they updated the cachegrind output files to use nanoseconds instead of milliseconds.

xdebug/xdebug@23316fe#diff-8347dd65d04dfcc2f130b7acdc8124680c52fa32f2bc2d387443a0a9748ad7b7

My question is how do you want to handle the different version of Xdebug, should I just put the following in the webgrind config?

static $xdebug_new_features = version_compare('3.0', phpversion('xdebug'), '>=');

And then use that in the php parser? For the C++ parser, is it enough to just grab the version of xdebug from the second line:

creator: xdebug 3.0.1 (PHP 7.4.13)

By comparing what comes after "xdebug"?

@alpha0010
Copy link
Collaborator

Good find. I think reading the events header would be a more future proof implementation. Header lines are already extracted (php, C++), so maybe check there if the events header says Time_(10ns). If so, set a scale factor to multiply the costs by.

@alpha0010 alpha0010 added the bug label Dec 31, 2020
@hemberger
Copy link
Contributor

Thanks for tracking this down. I thought I was going crazy when I saw my costs in webgrind with Xdebug 3.0. After a lot of time spent grasping at straws, I finally downloaded KCachegrind and also noticed that the costs there were exactly 100x smaller.

I took a quick look at where the headers are parsed, but as a complete webgrind novice, it wasn't clear to me how this issue could/should be handled. Is there anyone with more expertise who would be willing to look into this? 🥺

@ilibilibom
Copy link

+1
Any plan to fix this issue ?

@ozupey
Copy link

ozupey commented Sep 12, 2021

@alpha0010 before I spend time writing a pull request that won't be accepted (like #141), can you please confirm this project is still being maintained and that pull requests will be merged?

@alpha0010
Copy link
Collaborator

@ozupey Thanks for your interest. I am still here, just limited time for open source projects that I do not use on the job. This means that for PRs like 141 (where the author stated it only works in some cases), reviewing and testing is low on my priorities.

I will prioritize reviewing PRs that fully address bugs, so if you have the time, a fix would be appreciated.

@Woet
Copy link
Contributor

Woet commented Sep 16, 2021

I've just submitted a pull request that addresses this issue: #145.

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

Successfully merging a pull request may close this issue.

6 participants