Re: [PHP-DEV] Re: [RFC][DISCUSSION] debug_backtrace alternative as an array of StackFrame objects
Hi Nikita, wt., 21 lip 2020 o 10:41 Nikita Popov napisał(a): > Why does StackFrame define both public typed properties *and* getter > methods? One or the other would make sense to me, but not both. > TBH I didn't know what should be the preference, but since a StackFrame class is final providing methods doesn't bring any value in the API. I've updated the RFC and have removed all the methods and their references. Will update the PR with appropriate changes and resolve pending comments in next days. Cheers, Michał
Re: [PHP-DEV] Re: [RFC][DISCUSSION] debug_backtrace alternative as an array of StackFrame objects
On Tue, Jul 21, 2020 at 7:14 AM Michał Marcin Brzuchalski <
michal.brzuchal...@gmail.com> wrote:
> wt., 21 lip 2020 o 07:07 tyson andre
> napisał(a):
>
> > > Can you give an example of an internal stack frame?
> >
> > A stack frame that was created by a PHP internal function instead of by
> > userland code, i.e. one without a file in the frame.
> > Related to
> > https://github.com/php/php-src/pull/5820#pullrequestreview-452080212
> >
> >
> > ```
> > php > array_map(fn() => $GLOBALS['x'] = StackFrame::getTrace()[0], [2]);
> > php > var_export($x);
> > StackFrame::__set_state(array(
> >'file' => NULL,
> >'line' => NULL,
> >'function' => '{closure}',
> >'class' => NULL,
> >'type' => NULL,
> >'object' => NULL,
> >'object_class' => NULL,
> >'closure' => NULL,
> >'args' =>
> > array (
> > 0 => 2,
> > ),
> > ))
> > ```
> >
>
> The RFC has been adjusted regarding the nullability of file and line
> properties and related getter methods.
>
> Is there anything else stopping it from getting up to the vote?
>
> Cheers,
> Michał
>
Why does StackFrame define both public typed properties *and* getter
methods? One or the other would make sense to me, but not both.
Nikita
Re: [PHP-DEV] Re: [RFC][DISCUSSION] debug_backtrace alternative as an array of StackFrame objects
wt., 21 lip 2020 o 07:07 tyson andre napisał(a):
> > Can you give an example of an internal stack frame?
>
> A stack frame that was created by a PHP internal function instead of by
> userland code, i.e. one without a file in the frame.
> Related to
> https://github.com/php/php-src/pull/5820#pullrequestreview-452080212
>
>
> ```
> php > array_map(fn() => $GLOBALS['x'] = StackFrame::getTrace()[0], [2]);
> php > var_export($x);
> StackFrame::__set_state(array(
>'file' => NULL,
>'line' => NULL,
>'function' => '{closure}',
>'class' => NULL,
>'type' => NULL,
>'object' => NULL,
>'object_class' => NULL,
>'closure' => NULL,
>'args' =>
> array (
> 0 => 2,
> ),
> ))
> ```
>
The RFC has been adjusted regarding the nullability of file and line
properties and related getter methods.
Is there anything else stopping it from getting up to the vote?
Cheers,
Michał
Re: [PHP-DEV] Re: [RFC][DISCUSSION] debug_backtrace alternative as an array of StackFrame objects
> Can you give an example of an internal stack frame?
A stack frame that was created by a PHP internal function instead of by
userland code, i.e. one without a file in the frame.
Related to https://github.com/php/php-src/pull/5820#pullrequestreview-452080212
```
php > array_map(fn() => $GLOBALS['x'] = StackFrame::getTrace()[0], [2]);
php > var_export($x);
StackFrame::__set_state(array(
'file' => NULL,
'line' => NULL,
'function' => '{closure}',
'class' => NULL,
'type' => NULL,
'object' => NULL,
'object_class' => NULL,
'closure' => NULL,
'args' =>
array (
0 => 2,
),
))
```
Regards,
- Tyson
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC][DISCUSSION] debug_backtrace alternative as an array of StackFrame objects
Hi Tyson, wt., 21 lip 2020 o 06:54 tyson andre napisał(a): > Hi Michał Marcin Brzuchalski, > > At the time of writing, the documentation in > https://wiki.php.net/rfc/stack-frame-class#stackframe_class is also > inconsistent with the implementation. > `public readonly string $file;` is not actually read only in the > implementation in the GitHub PR. It can be overwritten or deleted with > `$frame->file = 'newfile.php';` > or `unset($frame->file);`. > I'd guess this is expressing the desired way that StackFrame instances > would be used by PHP code, > but that seems confusing because some properties of other internal classes > are actually effectively read only due to providing magic getter APIs. > (Also see my previous comment about ArrayAccess inconsistencies) > Dropping the readonly from the RFC is one possible way to clarify that. > The readonly was dropped in the RFC to meet the implementation similar to PhpToken where a developer is free to modify all properties. Cheers, Michał
Re: [PHP-DEV] Re: [RFC][DISCUSSION] debug_backtrace alternative as an array of StackFrame objects
Hi Tyson,
wt., 21 lip 2020 o 04:27 tyson andre 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ł
Re: [PHP-DEV] Re: [RFC][DISCUSSION] debug_backtrace alternative as an array of StackFrame objects
Hi Michał Marcin Brzuchalski,
At the time of writing, the documentation in
https://wiki.php.net/rfc/stack-frame-class#stackframe_class is also
inconsistent with the implementation.
`public readonly string $file;` is not actually read only in the implementation
in the GitHub PR. It can be overwritten or deleted with `$frame->file =
'newfile.php';`
or `unset($frame->file);`.
I'd guess this is expressing the desired way that StackFrame instances would be
used by PHP code,
but that seems confusing because some properties of other internal classes are
actually effectively read only due to providing magic getter APIs.
(Also see my previous comment about ArrayAccess inconsistencies)
Dropping the readonly from the RFC is one possible way to clarify that.
```
final class StackFrame implements ArrayAccess
{
public readonly string $file;
public readonly int $line;
```
The first time I've compiled or looked at the implementation and RFC document
in detail was this evening, out of a curiosity of how it worked
and to look into the performance impact to expect,
after seeing the vote announcement this morning.
> Sorry for the late comment - I'd been busy/occupied with various other things
> and this RFC fell off my radar
Regards,
- Tyson
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
Re: [PHP-DEV] Re: [RFC][DISCUSSION] debug_backtrace alternative as an array of StackFrame objects
Hi Michał Marcin Brzuchalski,
> Heads up, I plan to open the vote tomorrow morning.
Sorry for the late comment - I'd been busy/occupied with various other things
and this RFC fell off my radar,
and I'd assumed that anything I'd end up noticing when looking at the RFC
would have already been pointed out by other reviewers.
(In general, I've only reviewed or locally compiled the code of a small
fraction of recent RFCs mentioned on the mailing list.)
I had some comments on the implementation in
https://github.com/php/php-src/pull/5820#pullrequestreview-452045885
- 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.
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.
- 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())
- The typed property default was incompatible with the typed property
declaration (`class StackFrame { string $file = null; /*...*/}` for internal
stack frames)
With the feature freeze nearby, more of a heads up would have been useful.
Aside: The guidelines in https://wiki.php.net/rfc/howto have been followed,
but I think it'd be useful for those guidelines to say "consider *at least* one
days heads up mail on the mailing list" instead of "consider one days" to avoid
cases where review comments are made on short notice.
Normally, any issues found the day before voting starts would have more time to
reschedule the vote to be addressed.
- I don't know how amendments to the howto get proposed or decided on
Regards,
- Tyson
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: https://www.php.net/unsub.php
[PHP-DEV] Re: [RFC][DISCUSSION] debug_backtrace alternative as an array of StackFrame objects
Hi Internals, Heads up, I plan to open the vote tomorrow morning. Cheers, Michał Marcin Brzuchalski wt., 7 lip 2020 o 22:46 Michał Marcin Brzuchalski < michal.brzuchal...@gmail.com> napisał(a): > Hi internals, > > following recently added alternative to `token_get_all()` in form of a > dedicated class `PhpToken` > I've made a similar proposal for collecting stack traces by introducing > `StackFrame::getTrace()` > and `StackFrame` which replaces array with keys and values known from > `debug_backtrace()`. > > To compete with easier adoption `StackFrame` implements `ArrayAccess` > giving identical API > to `debug_backtrace()`. > > A test script with deep recursion up to 1M frames uses 50% less memory > than `debug_backtrace()`. > > The RFC is described here https://wiki.php.net/rfc/stack-frame-class > It includes implementation of PoC and tests. > > Cheers, > Michał Marcin Brzuchalski >
