Closed TRowbotham closed 3 years ago
After thinking about this more and looking at some of my recent profiles, its pretty clear the whole integer overflow thing is a very big problem with the change to nanoseconds; at least for the summary line, I haven't spent the time check the costs of individual functions.
A recent profile generated by Xdebug 3 from one of my projects (profiling only took 1.26 min) ends up producing a summary line of:
$ grep -I 'summary:' /tmp/cachegrind.out.1618_1622494851.330687
summary: 9047770910 62830704
9,047,770,910 is clearly outside the range of a 32-bit number and it ends up being truncated when packed:
$ psysh
Psy Shell v0.10.8 (PHP 7.4.3 — cli) by Justin Hileman
>>> $num = 9047770910;
=> 9047770910
>>> $p = pack('V*', $num);
=> "\x1E\x07J\e"
>>> unpack('V*', $p)
=> [
1 => 457836318,
]
A more proper fix would be to check for the events
header in the pre-processors, then convert the numbers to something smaller, such as microseconds, before packing. This would probably require a bump in the file format version, so as to invalidate any webgrind generated files prior to normalizing the stored costs number. Though, it may not be sufficient should any of the individual costs exceed the size of a signed 32-bit int before normalization.
The only other options I can think of here is to switch to using a big int library and packing the numbers as strings, or possibly dropping support for running webgrind on 32-bit versions of PHP and changing the packing format to "P". Both of which would require a bump in the file format version.
I'll leave it up to you if you want to take this patch as an interim fix.
Thanks for your contribution. Decided to go with a simplified version of this; the full solution would need to be done in the preprocesser, so avoiding complexity in the reader (which would later be removed).
This should fix #132, though I don't have an Xdebug 2 profile to compare to. Based on the findings in #132 Xdebug 2 reported costs in microseconds, but Xdebug 3 now reports those costs in nanoseconds. This looks at the
events
header, which explicitly declares the time unit used for the costs, if defined, and dynamically adjusts the divisor based on the declared time unit.This seemed like the easiest fix. Since Xdebug 3 outputs much larger numbers due to the change to nanoseconds, the preprocessor is probably at greater risk of integer overflow, but that is a pre-existing problem.