Skip to content

new \Exception change internal debug_backtrace() to only one row saving generator to receive other rows #18333

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
gzhegow1991 opened this issue Apr 16, 2025 · 7 comments

Comments

@gzhegow1991
Copy link

gzhegow1991 commented Apr 16, 2025

Description

As thousands of times sad before, the \Exception is born-to-die class. It should not appear to top-level of the application, its like self-protection detonator - should be to improve speed of fixing.

But as i can see with PHP 8.4.5 tests following code

$mt = microtime(true);
$errors = [];
for ($i = 0; $i < 10000; $i++) {
	try {
		// $errors[] = new \stdClass();
		throw new LogicException($i);
	} catch (\Throwable $e) {
	}
}
var_dump(microtime(true) - $mt);

still much slower than object creation, like 15 times slower.

I guess its about collecting backtrace on new \Exception call, but actually we DONT need that backtrace.

We dont need backtrace UNTIL we ASK for backtrace.

If your debug screen, tool or symfony dd() ask for render backtrace only then we need rows of backtrace, its arguments and so on. Also if exception becomes to log-file we're getting backtrace to simplify error search.

Old good PHP code that never throws any exceptions and uses return false we can, or even we must call unsafe because application can do actions, that we dont expect if we follow that procedure style (small scrips - ok, big enterprise - no).

So we follow the way where any function could throw the exception, but using those functions in production code without try/catch those exceptions is very un-friendly for users. Its ok for developers. Not for users.

So i see there two possible cases:

  • you get unexpected exception, like typeError or something - that you must put to log and fix as fast as you can
  • you write own function over existing to prevent propagation of the exception because maybe string is empty = you cannot create email object from empty string. Maybe, correct way is to evade that case, but actually i prefer to create own types with own checks and evade to write code like:
$value = $this->to_string()->check_non_empty_string()->try_to_create_email()->check_email_is_fake();

i prefer create own function and call it like (because its short and readable):

$emailNonFake = $this->type_email_non_fake($value);

So exceptions in this case may appear billion times a day. Because functions to check email throw logic exceptions if regex pattern fails or this stuff. But type_ function over that parser will catch all the exceptions, resolve it, making value NULL or return boolean false.

Otherwise i have to write one function WITH exceptions, other function WITHOUT exceptions, just because first one is slower than second one in case of errors. Or maybe even three functions (three - with \TypeErrors to control level of the warning or something). In any case - its spending time for developer needs, not for task needs.

So much of exceptions is disappear at runtime and never become log file or debug tool.

At first, it feels like store generator with backtrace is a bad idea, but exception, that is become to log or never become to log - will be destroyed in microseconds. TTL of exception classes = time to stop application, or time to resolve catch clause. And we need trace only if we print it. When we catch exception immediate - we dont even need to know where this error appear. Of course - we can dump exception object and then generator must return whole path. But if we DONT ASK - we DONT NEED.

Could you improve performance after reading this?

@DanielEScherzer
Copy link
Member

Some stats from https://3v4l.org/rXTja comparing 100000 iterations of throwing exceptions, just creating them, creating stdClass, and creating Attribute (a built in class given a parameter) for 8.4.6

Throwing exceptions: 0.028692007064819
Creating exceptions: 0.014251947402954
Creating stdClass  : 0.0025491714477539
Creating Attribute : 0.0034079551696777

So exceptions do have worse performance when created. Given that

ZEND_METHOD(Exception, __construct)
doesn't do much beyond argument parsing, the slowness probably comes from the fact that the objection creation in
static zend_object *zend_default_exception_new(zend_class_entry *class_type) /* {{{ */
needs to fetch the backtrace

But if we DONT ASK - we DONT NEED.

True, but we can't predict the future - the backtrace needs to be generated when an exception is created, because it won't be available to generate later, and it might be required later. If we don't generate one when the exception is created, we would never have one

@gzhegow1991
Copy link
Author

gzhegow1991 commented Apr 16, 2025

Maybe is possible to do one step of reading debug backtrace like "clone path in memory as is" (but seems it requires reading tree too) or maybe create another way to backtrace like "one step of propagation - one line in trace" (without arguments)...

I see it like generators - you can calculate all permutations/combinations of english alphabet but if you try to create array with them - i dont know how many gigabytes it will require. But you can do one step saving the behavior and current state... Maybe its inapplicable here, but clone already processed path for futher reading without touching it (just duplicate for a while) - something like that?

Or create something like "internal trace" - less informative, with ability to switch to debug mode with full traces. Or just catch the place where error is happened, with again, production mode/debug mode. Anything to evade writing few functions - thats the problem.

Dont use exceptions as errors like Fowler say ten years ago seems a solution, but again, i tried to RFC with that mailing list about faster error bag (i even had created package with) - everybody uses exception and become true-hero about he knows only correct way

Disclaimer.

Javascript Promise is not just promise. Its handy Pipeline as first. Reducing count of nesting about try/catching. You just write one line to convert \Throwable to null result, instead of writing separate function that does try catch over existing one, just because reading try/catch hell is a... hell.

@DanielEScherzer
Copy link
Member

Any type of trying to clone memory is going to much slower than just recording the trace. You could try disabling https://www.php.net/manual/en/ini.core.php#ini.zend.exception-ignore-args

@gzhegow1991
Copy link
Author

gzhegow1991 commented Apr 16, 2025

So i tried two ways:

  1. create own error bag, that works like this:
$bag->errors_start($b);

for ( $i = 0; $i < 3; $i++ ) {
    $bag->error([ 'This is the error message' ]);
}

$errors = $bag->errors_end($b);

var_dump($errors);

Source:

class MyClass {
    /**
     * @return object{ stack: array }
     */
    public function errors() : object
    {
        static $current;

        $current = $current
            ?? new class {
                /**
                 * @var object[]
                 */
                public $stack = [];
            };

        return $current;
    }

    /**
     * @return object{ list: array }
     */
    public function errors_current() : ?object
    {
        $stack = $this->errors();

        $errors = (0 !== count($stack->stack))
            ? end($stack->stack)
            : null;

        return $errors;
    }

    /**
     * @return object{ list: array }
     */
    public function errors_new() : object
    {
        $errors = new class {
            /**
             * @var array
             */
            public $list = [];
        };

        return $errors;
    }

    /**
     * @return object{ list: array }
     */
    public function errors_start(?object &$errors = null) : object
    {
        $stack = $this->errors();

        $errors = $this->errors_new();

        $stack->stack[] = $errors;

        return $errors;
    }

    public function errors_end(?object $until) : array
    {
        $stack = $this->errors();

        $errors = $this->errors_new();

        while ( count($stack->stack) ) {
            $current = array_pop($stack->stack);

            foreach ( $current->list as $error ) {
                $errors->list[] = $error;
            }

            if ($current === $until) {
                break;
            }
        }

        return $errors->list;
    }

    public function error($error, $result = null) // : mixed
    {
        $current = $this->errors_current();

        if (null !== $current) {
            $current->list[] = $error;
        }

        return $result;
    }
}

But the general problem with this method is that "you HAVE TO create error bag manually at the moment you decide."

If the package is not created, you will not combine errors, so somewhere the number of errors will be zero, and the logic will continue, not be interrupted. It works with api debugging, and I even created a "gzhegow/error-bag" package that allows you to combine an existing error package with a parent error package or even duplicate errors by tags for batch requests... However, this package NEVER breaks the code like THROW does.

Thus, using this method requires thousands of "if" statements anywhere to check if an error has occurred, and a batch of errors to collect them.

  1. The usual way, which I already suggested 1.5 years ago on the php-internals mailing list (and was rejected because I'm not Elon Musk and people start chatting on wibe instead of designing).

Add the raise operator, which works like a throw, but collects only the place where the error occurred. I mean debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS, $limit = 1) or even better [ __FILE__, __LINE__ ]. This statement is not intended to save exceptions to the log using tracing, but it can still stop the code by reducing the number of "if" statements to zero. And, maybe, INI flag that causes any raise to work as a throw (debug_mode).

Therefore, the developer should use throw if he knows that the error is serious and is part of an unpleasant case. Or use raise if it's a simple logical exception (type error, return type error). Or maybe just change the behavior of the \Error throwables classes - by default they store only one line, but it would be possible to enable an ini setting to store the entire trace, but as far as I remember, these are internal PHP components (changing them would have caused a lot more mandatory things to fail).

For me better to raise operator that can be applied for developer needs instead of global replacement of \Error classes. So we can (raise \Error or \Exception) OR (throw \Error or \Exception). And both of them can be caught by try/catch block and processed as usual, just raise operator work faster, but gives less info.

Actually, i know that new operator is a bottleneck, but that neck may be moved to raise/throw purpose.

@jeffdafoe
Copy link

If I remember right, this from the app you inherited for maintenance, where it keeps losing the state of its batch processing due to poor error handling in the original implementation? And you want a core engine change to accommodate your specific case?

@gzhegow1991
Copy link
Author

gzhegow1991 commented Apr 16, 2025

If I remember right, this from the app you inherited for maintenance, where it keeps losing the state of its batch processing due to poor error handling in the original implementation? And you want a core engine change to accommodate your specific case?

Provocative questions in which "you're not wrong" in order to reproach me for trying to "solve my problems by making everyone work except myself" are not part of productivity, but are part of protecting your own reputation.

Your comment is hindering the discussion. In simple - you're asking people to collaborate in hating something, to RIP the suggestion.

Please, keep my point (not my case, but your possibilities of the feature request) in your mind. Engineering/design is a hard job, and 60% of this job is in ability to forget social status and relationships, and focus to "dont do worse, but do better". I dont imagine how faster operator can do worse. Little confusion maybe, but... if you read docs is not a problem. But benefit is code milliseconds - 15times faster code with 15 times less count of if statements.

throw gives second part of this benefit (and ability to save full log files, simplify debugging etc)
raise gives the first (but removes additional info to achieve maximum speed for non-exception cases).

Next step would be implementing pipeline with catch statement and redesign callable storing from arrays (methods, closures, functions, internal functions) to value objects, giving ability to call it like:

$var = pipe($value, $context = (object) [])
->do( method(new MyObject(), $method1) )
->do( [ arg1, arg2 ], method(static::class, $method2), [ arg4, arg5 ] ) // > additional args, why not?
->do( fn_internal('strval') ) // > requires strict count of arguments (!)
->do( fn('my_func') )
->catch(&$e, $result = false);

$logger->log($e);

vs

try {
  $object = new MyObject();

  $result = $value;
  $result = $object->method1($result);
  $result = static::method2($result);
  $result = strval($result); // > hello, can be 'Array' if array (casting|debug purposes collision)
  // $result = intval($result); // > hello, can be 1 (RESOURCE_ID) if resource (casting|debug purposes collision)
  // $result = boolval($result); // > hello, can be FALSE if string '0' (casting|userinput purposes collision)
  $result = my_func($result);

} catch (\Throwable $e) {
  $logger->log($e);

  $result = false;
}

Most angry point here is nesting code section that forces attention about its not safe code (readability)

Anyway, am happy you remembered points that we discussed.

@kallesommernielsen
Copy link

I guess its about collecting backtrace on new \Exception call, but actually we DONT need that backtrace.

We dont need backtrace UNTIL we ASK for backtrace.

Like @DanielEScherzer mentioned, we need to keep capture the frame when it happens otherwise it is not gonna be available after that.

If you wish to change this behavior, then please write an RFC and take this discussion to the internals@ mailing list, as we do not use issues as a discussion medium.

Please see the following guide if you seek to make a proper RFC:
https://wiki.php.net/rfc/howto

@kallesommernielsen kallesommernielsen closed this as not planned Won't fix, can't repro, duplicate, stale Apr 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants