Hi Tyson, wt., 21 lip 2020 o 04:27 tyson andre <tysonandre...@hotmail.com> napisał(a):
> ... > I had some comments on the implementation in > https://github.com/php/php-src/pull/5820#pullrequestreview-452045885 > Most of these comments can be worked upon the next days in the meantime. > - How much does this change elapsed time in typical use cases? This had > completely skipped my mind when I saw this RFC, but I'd generally assume > that stack traces aren't kept along for longer than needed to process them, > and that most stack traces would be at most hundreds of frames in typical > applications. So I'd consider CPU time elapsed more important than RAM. > The current implementation works similarly as original one which instead of creating an array creates an object of StackFrame. In general use of resources depends on programming style, amount of rethrows etc. Some use exceptions inappropriately and keep the objects living for a significant time etc. > I'd had similar issues with the tradeoffs in PhpToken (and forgot). > Those issues were that the applications using the array would use less > memory and be faster if I used the returned array multiple times, but less > so if I only processed the array once and immediately freed it. > PHP also seems faster at fetching dimension from arrays than properties > from objects. > > Still, though, this might help with a bit with developer productivity > (e.g. IDEs being better at inferring types of $frame->getFile() in some > cases, or providing help text for the method), or in being able to check > types when passing individual frames > - I found a bug: The ArrayAccess magic methods are implemented, but not > the real methods. > This can be fixed, PR provides an initial implementation which can be improved in the next days. > - I found some memory leaks, possibly related to tracking argument arrays > in the object being stored > > - Casting getFile() to the empty string is unexpected for me > ($frame['file'] and $frame->file are null in the implementation). > - Forbidding modification/deletion on the ArrayAccess magic API (e.g. > `$frame['trace'] = ...`) > seems inconsistent when modification and deletion are allowed through > the real properties, > (and throwing there may affect code written to process the array > returned by Throwable->getTrace()) > This can be adjusted to meet 99% compatibility with current array frames, the only limitation I would see here would be the inability to create properties dynamically. > - The typed property default was incompatible with the typed property > declaration (`class StackFrame { string $file = null; /*...*/}` for > internal stack frames) > Can you give an example of an internal stack frame? Cheers, Michał
