This will abstract out PHP into an array of steps, which can then be called.

Design is flexible, and contrib modules can add more stuff.

Likely candidates for this include index.php stuff, menu_execute_active_handler(), and theme().

One of the advantages of this is that we can now do fun stuff like hook_steps_alter(), allowing modules to make changes to core Drupal logic where needed.

An example use case would be if I wanted not to print theme('page', $return), but instead, output it in some other format. Or perhaps I want to change the site maintenance response from site maintenance to access denied. Or perhaps I want to not bother with blocks for anonymous users. Or override menu_execute_active_handler with my new system, which accepts a different format of input than GET. The possibilities are endless, and this API can be used nearly anywhere!

Edit: Patch attached. Code removed for increased readability.

CommentFileSizeAuthor
#1 steps_0003.patch1.85 KBcwgordon7
#1 includes/steps.inc13.62 KBcwgordon7

Comments

cwgordon7’s picture

Status: Active » Needs review
StatusFileSize
new13.62 KB
new1.85 KB

Ok. It is way past my bedtime, and I am so not dealing with adding files to CVS when CVS hates me and is laughing at me right now. So I have attached the new file; it is meant to be named steps.inc and placed in the includes/ directory.

Setting to code needs review.

Anonymous’s picture

I'll take a look at this tomorrow. I will be running a performance test against this patch, as I am curious as to what sort of extra processing is going to be involved.

My initial concerns were discussed with cwgordon7 on irc.

steven jones’s picture

Yeah, the performance hit could be huge as I'm guessing that you'd want this called on every page. Needs a benchmark.

Anonymous’s picture

Initial results, stock configuration, following instructions/guidelines here: http://drupal.org/node/79237

Server Software: Apache/2.2.8
Server Hostname: localhost
Server Port: 80

Document Path: /drupal6/index.php
Document Length: 32624 bytes

Concurrency Level: 1
Time taken for tests: 246.560847 seconds
Complete requests: 500
Failed requests: 0
Write errors: 0
Total transferred: 16558000 bytes
HTML transferred: 16312000 bytes
Requests per second: 2.03 [#/sec] (mean)
Time per request: 493.122 [ms] (mean)
Time per request: 493.122 [ms] (mean, across all concurrent requests)
Transfer rate: 65.58 [Kbytes/sec] received

Connection Times (ms)
min mean[+/-sd] median max
Connect: 0 0 0.1 0 3
Processing: 434 492 23.7 489 659
Waiting: 424 481 20.9 479 650
Total: 434 492 23.7 489 659

Percentage of the requests served within a certain time (ms)
50% 489
66% 493
75% 496
80% 498
90% 508
95% 538
98% 571
99% 603
100% 659 (longest request)

And after applying the patch:

Server Software: Apache/2.2.8
Server Hostname: localhost
Server Port: 80

Document Path: /drupal6/index.php
Document Length: 32624 bytes

Concurrency Level: 1
Time taken for tests: 253.789797 seconds
Complete requests: 500
Failed requests: 0
Write errors: 0
Total transferred: 16558000 bytes
HTML transferred: 16312000 bytes
Requests per second: 1.97 [#/sec] (mean)
Time per request: 507.580 [ms] (mean)
Time per request: 507.580 [ms] (mean, across all concurrent requests)
Transfer rate: 63.71 [Kbytes/sec] received

Connection Times (ms)
min mean[+/-sd] median max
Connect: 0 0 0.0 0 0
Processing: 443 507 27.4 503 883
Waiting: 432 496 26.6 492 873
Total: 443 507 27.4 503 883

Percentage of the requests served within a certain time (ms)
50% 503
66% 507
75% 510
80% 512
90% 519
95% 534
98% 594
99% 631
100% 883 (longest request)

493ms +/- 23.7ms std. deviation before patch
507.58ms +/- 27.4ms std. deviation after patch

No noticeable performance hit or improvement.

cwgordon7’s picture

So roughly a 15 millisecond loss, give/take ~25 milliseconds. Question is now, is this worth it?

Anonymous’s picture

No, @cwgordon7. Basically the test was inconclusive, there is no measurable loss or gain in performance. Which is a good thing.

Crell’s picture

I am honestly not in favor of this methodology, not for performance reasons as much as complexity. There are ways to make PHP's pipeline dynamic that do not involve writing PHP with lots and lots of brackets around it, which is basically what this is doing. I'd much rather establish a clearer pipeline with key "bend" points than trying to abstract the PHP language itself. That's going to lead to alls sorts of array spaghetti.

dmitrig01’s picture

Why is

define('STEPS_FUNCTION', 'steps_handle_function');
define('STEPS_VAR_SET', 'steps_handle_var_set');
define('STEPS_CONSTANT', 'steps_handle_constant');
define('STEPS_IF', 'steps_handle_if');
define('STEPS_SWITCH', 'steps_handle_switch');

in steps.inc, while the hook_step_types is defined in system module?

There needs to be more calling of functions. It seems you made a couple of tweaks to steps_execute in order to specifically accomodate the page. This is not generalized enough.

Next criticsm:
in steps.inc, steps_execute:

    // Allow functions to alter it. If the drupal_alter() function doesn't exist, bootstrap!
    if (!function_exists('drupal_alter')) {
      require_once './includes/bootstrap.inc';
      drupal_bootstrap(DRUPAL_BOOTSTRAP_FULL);
    }

I see several problems with this code. First and foremost, this is just STUPID. Why not include the bootstrap in index.php? You are obfuscating code. We might as well use runkit for something like this :P.

Can you stick in a few more examples? Like, the menu system - access callback, wildcard replacements, run page callback, theme('node') for ndoes, etc.

This is ugly - 'arguments' => array('$return'),

Anonymous’s picture

@Crell I feel that it is overly complex as well, and don't see myself /ever/ wanting to use it.

cwgordon7’s picture

Status: Needs review » Needs work

cnw then.

I have some ideas about how to fix this up and make it into a nicer API. I need time to do this though.

dmitrig01 also suggested the idea of a drupal_call() function that would allow hook_function_alter()'s to take place.

E.g.

<?php
drupal_call('theme', 'page', $return);

function drupal_call() {
  $args = func_get_args();
  $func = array_shift($args);
  $array[] = array('function' => $func, 'args' => $args);
  drupal_alter('function', $array);
  drupal_alter($func, $array);
  foreach ($array as $data)
    call_user_func_array($data['function'], $data['args']);
  }
}
?>

This will allow for added awesomeness. This can either be in addition to steps or separate.

pwolanin’s picture

#10 seems like an interesting idea - though I don't know how this would fit in with registry.
Basically - you could imagine this as a rewrite of module_invoke() - where module_invoke() allows you to _alter?

Note, however, chx has pointed out to me that with PHP5, we should just avoid using call_user_func_array(0 by having a lon list of args that default to NULL.

cwgordon7’s picture

Oh yes, awesome idea! Module_invoke() should definitely allow for alterations too! We can even do funky stuff like hook_hook_{hook}_alter! It'll be awesome!

sun’s picture

Status: Needs work » Closed (won't fix)

I think this is obsolete due to the new page rendering process using drupal_render() and hook_page_alter().