[RFC][VOTE] debug_backtrace alternative as an array of StackFrame objects

  111091
July 21, 2020 11:52 michal.brzuchalski@gmail.com (=?UTF-8?Q?Micha=C5=82_Marcin_Brzuchalski?=)
Hi Internals,

I've opened the voting for adding object-based debug_backtrace()
alternative.
Secondary vote is about replacing object-based trace for
Throwable::getTrace().

Voting opened 2020-07-21 and closes 2020-08-04.

Link to the RFC https://wiki.php.net/rfc/stack-frame-class#vote

Cheers,
Michał Marcin Brzuchalski
  111098
July 22, 2020 07:10 come.chilliet@fusiondirectory.org (=?UTF-8?B?Q8O0bWU=?= Chilliet)
Le Tue, 21 Jul 2020 13:52:39 +0200,
Michał Marcin Brzuchalski brzuchalski@gmail.com> a écrit :
> Voting opened 2020-07-21 and closes 2020-08-04. > > Link to the RFC https://wiki.php.net/rfc/stack-frame-class#vote
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
  111123
July 22, 2020 14:46 bobwei9@hotmail.com (Bob Weinand)
> Am 22.07.2020 um 09:10 schrieb Côme Chilliet chilliet@fusiondirectory.org>: > > Le Tue, 21 Jul 2020 13:52:39 +0200, > Michał Marcin Brzuchalski brzuchalski@gmail.com> a écrit : >> Voting opened 2020-07-21 and closes 2020-08-04. >> >> Link to the RFC https://wiki.php.net/rfc/stack-frame-class#vote > > 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
  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
  111309
August 4, 2020 08:05 michal.brzuchalski@gmail.com (=?UTF-8?Q?Micha=C5=82_Marcin_Brzuchalski?=)
Hi Internals,

Voting has ended. The RFC was declined with 14 votes against and 14 votes
in favour.
The second vote shows we don't wanna change exceptions trace into objects
at all.

If I'm not wrong this means that debug_backtrace using objects as
frames instead of arrays has some potential and maybe it's worth to put it
as a PECL extension. Please correct me if I'm wrong.

Thanks for participating.

Cheers,
Michał Marcin Brzuchalski

wt., 21 lip 2020 o 13:52 Michał Marcin Brzuchalski <
michal.brzuchalski@gmail.com> napisał(a):

> Hi Internals, > > I've opened the voting for adding object-based debug_backtrace() > alternative. > Secondary vote is about replacing object-based trace for > Throwable::getTrace(). > > Voting opened 2020-07-21 and closes 2020-08-04. > > Link to the RFC https://wiki.php.net/rfc/stack-frame-class#vote > > Cheers, > Michał Marcin Brzuchalski >