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

This is only part of a thread. view whole thread
  111139
July 22, 2020 21:34 tysonandre775@hotmail.com (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. https://wiki.php.net/rfc/stack-frame-class#performance 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 https://xdebug.org/docs/all_settings

   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.

Regards,
- Tyson