Re: [PHP-DEV] [RFC][VOTE] debug_backtrace alternative as an array ofStackFrame objects

This is only part of a thread. view whole thread
July 22, 2020 14:46 (Bob Weinand)
> Am 22.07.2020 um 09:10 schrieb Côme Chilliet>: > > Le Tue, 21 Jul 2020 13:52:39 +0200, > Michał Marcin Brzuchalski> a écrit : >> Voting opened 2020-07-21 and closes 2020-08-04. >> >> Link to the RFC > > Hello, > > Can people voting no explain their vote? > I did not see that much discussion against this prior to vote. > > (I’m not convinced this should pass either, just wondering what the > pro/cons are here) > > Côme
Hey, I guess, the main objection, at least for me, is just introducing a new API, just because it looks slightly nicer, but the actual benefit being very marginal. Bob
July 22, 2020 21:34 (tyson andre)
Hi internals,

To answer the question of why I've voted no on this RFC,
it's largely based on a uncertainty on what performance impact I'd expect
and bc breaks.

1. There are memory leaks in the implementation PR, **so this can't be accurately benchmarked at the moment.**
   Improving performance seems to be a major reason for this proposal, so if it doesn't improve performance in typical use cases,
   it'd be counterproductive, and end users may make the switch but get worse or only slightly better performance
   (depending on how the traces are processed)
2. was an impressive benchmark result, but not representative.
   "On a test script with 1M recursions to produce huge result results were as above:" would be more realistic
   with actually processing the data, and accounting for the time needed for freeing arrays/objects.
   1 million recursions is not a typical stack depth - For example, xdebug.max_nesting_level = 256 in

   I could not reproduce the results because the test script used wasn't provided in the RFC.
   (e.g. unsetting the object_class property on all frames then calling json_encode, to get the same stack trace as debug_backtrace)

   My experience is that typical applications don't keep around the stack frame array after logging/processing them,
   so the processing time would be a larger consideration for me than memory usage.
3. The change to the default for getTrace() for the secondary vote (which I assume requires the default 50% majority)
    would only make sense if there is a significant performance benefit for a majority of applications,
    and I didn't realize it was part of the secondary vote.

    E.g. `function normalizeFrame(array $frame): array {...}` or `$frame['args'] = json_encode($frame['args'])` 
    would start throwing TypeErrors if it was called - this may affect legacy scripts/applications/libraries/crons.

    Another API alternative without the B.C. break I thought of while writing this response might be `StackFrame::traceFromObject(Throwable|ReflectionGenerator $t): StackFrame[]`
4. Adding multiple ways to do the same thing makes it somewhat harder to learn/remember a language.
   While adding another way to do something has been accepted/rejected in other RFCs,
   that depends on the pros and cons of individual RFCs and preferences of voters
   (e.g. how often that functionality is/would be in code people read/write, overall performance, etc.).
5. I forgot about this when looking at this earlier, but my experience is that a minority of PHP applications spend significant amounts of time in exception handling or backtrace generation.

   Those applications may still be able to benefit from StackTrace as a PECL if they frequently generate trace arrays and/or keep around stack traces for a long time.

If opcache gets significantly better at optimizing php code that works with internal typed properties,
or if there was a better evaluation of the performance improvement a larger variety of uses of debug_backtrace() would see,
and if my intuition about typical applications was inaccurate,
I might reconsider this.

- Tyson