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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php

Reply via email to