Request for Comments: Error formatting optimizations

• Version: 1.0
• Date: 2010-08-21
• Author: Kalle Sommer Nielsen kalle@php.net
• Status: Draft (Inactive)
• First Published at: http://wiki.php.net/rfc/error-optimizations

This RFC proposes a way to optimize our errors, by performing a type of just-in-time formatting to reduce the additional costs by formatting errors thats never shown.

Every error in PHP is formatting in a sprintf()-alike syntax using one of our many error handling functions, every time an error, warning or notice occurs, we hit the error handlers, format them even if we never wish to display the errors. This RFC proposes a way that could be used to reduce that.

This RFC's implementation, proposes that we extends the EG() globals (executor) inside the Engine, to contain four new fields:

        ...
        zend_bool error_stack_enabled: 1;
        zend_bool error_stack_logging: 0;
        zend_error_arguments error_stack[];
        int error_stack_size: 0;
        ...

The new structure, “zend_error_arguments” looks like the following:

struct {
	/* Message format */
	const char *format;
 
        /* Error type */
        const short type;
 
	/* File, and line number, if any */
	const char *filename;
	const int lineno;
 
	/* Arguments */
	va_args arguments;
} _zend_error_arguments;
 
typedef struct _zend_error_arguments zend_error_arguments;

The error_stack_logging executor global needs to match that of INI_BOOL(“log_errors”) and is only enabled if error_stack_enabled (INI_BOOL(“display_errors”) is off) is 1, which should be handled in main/ at PHP, outside the Engine which just sets a default. When we hit an error, it will stack the error using the error arguments structure and stash it into the global. If display_errors is off and log_errors is on, then dispatch it directly to the logging buffer, but only stack the last occured one, meaning that if two errors occurs in a row, with log_errors set to on and display errors set to off, the second call will override the last argument, using the error_stack_size global:

zend_error_arguments arguments;
 
/* put the data into the arguments structure */
 
if (EG(error_stack_enabled) && EG(error_stack_logging)) {
        EG(error_stack)[EG(error_stack_size)] = arguments;
 
        if (EG(error_stack_logging)) {
                  /* dispatch to error logging hook */
        } else {
                  /* no logging, increase the stack size */
 
                  ++EG(error_stack_size);
        }
} else {
        /* BC code */
}

error_get_last() should internally check if EG(error_stack_enabled) is on, like:

if (EG(error_stack_enabled) && EG(error_stack_size)) {
        char *error = (char *) emalloc(sizeof(zend_error_arguments));
        int error_length;
 
        /* dispatch to formatting function, and copy it into, the "error" variable */
        /* NOTE: This is just a pseudo function name for formatting */
        zend_format_error_arguments(&error, &error_length, EG(error_stack)[EG(error_stack_size)]);
 
        RETURN_STRINGL(error, error_length, 0, 1);
} else {
        /* BC code */
}

This one is a tricky one, as we do not have any hooks for altering variables at reading, nor do we have JIT assignments. I think the best solution here is to simply remove $php_errormsg and require userland to use error_get_last() if they *REALLY* want the last errors, without depending on the track_errors ini options to be on.

If its not removed, then we cannot gain any optimization with track_errors = On at all, so if thats the case, it has to be taken into account when initializing the error_stack executor globals.

Obvious the memory usage here can grow quite rapidly, but people who would use this feature already takes great care of their code to not assume the opposite.

Since the error stack is hooked into the executor globals, which requires tsrm_ls to be available in thread safety mode, to avoid a call to TSRMLS_FETCH() everytime we hit the error callbacks or functions (zend_error, zend_error_cb, ...) then we should alter all references to include TSRMLS_DC/TSRMLS_CC for additional performance with ZTS.