On Tue, Jun 2, 2020 at 10:57 PM Max Semenik <maxsem.w...@gmail.com> wrote:
> On Sun, May 31, 2020 at 4:47 PM Nikita Popov <nikita....@gmail.com> wrote: > > > I'm concerned about the performance implications of this change. > Backtrace > > gathering is quite expensive and doing this for every diagnostic will > have > > a large performance impact if there are many of them. This is okay if > your > > code is intended to be diagnostics clean, but unfortunately this is not a > > given. If you generate 10k deprecation warnings during a request, you're > > definitely going to feel those backtraces. > > > > INI setting should take care of this. If your code is crap, don't flip it > on. > There's a couple of problems with this line of argumentation: First, even if the code is crap, wouldn't it still want to have stack traces for fatal errors? Why should crap code be denied stack traces? Isn't that the code that needs them most? If the code weren't crap it wouldn't be causing fatal errors anyway ;) Second, PHP has plenty of functions (especially I/O functions) where ignoring notices and warnings is a requirement. You are not interested in back traces for errors your ignore. Keep in mind that error_get_last() data is populated independently of whether errors are suppressed or not. Similarly, if your code promoted all warnings to exceptions using a custom error handler, then you'll perform a duplicate backtrace calculation, first when the warning is originally through, and then again if you construct the exception. (This is less bad than other cases, because it's "just" twice as slow.) Finally, even if we completely disregard the question of performance, back traces are very noisy. You get one fatal error per request, but you might easily get 1k warnings if something happens to trigger in a loop. With back traces, those 1k warnings will be 100k lines. I'm not sure if limiting it to just fatals is the best choice, but I do think that this needs a bit more control. And limited to just backtraces for fatals, I think this functionality should be enabled by default. This might also want to integrate with the zend.exception_ignore_args option (or, because that one is exception-specific, a renamed option that does the same thing). We will want to disable argument gathering in a default production configuration, because the arguments may contain sensitive data that must not appear inside log files. Regards, Nikita
