Drupal recommends using REQUEST_TIME over time() whenever possible.
This is done for performance and consistency reasons.

Where the time() values are dependent on the page request and not some ongoing process during the execution of the php, then you should use REQUEST_TIME.

I believe a good example of this is:
plugins/ultimate_cron/scheduler/crontab.class.php:164
$behind = time() - $next_schedule;
Would be better switched with:
$behind = REQUEST_TIME - $next_schedule;

Comments

gielfeldt’s picture

Yeah. I actually started out using request time consistently. Then I realized that this caused problems during execution of cronjobs when checking the schedule. I therefore switched to time() everywhere for (misplaced) reasons of consistency :-).

Are you currently having performance issues because of this? In my test-setup I have 400+ cronjobs, and time() is only called 800+ times in 1900 microseconds or so. However, your suggestion may be a very good place to change this, since it will propagate a performance loss all the way to the cron parsing, due to loss of static cache (base on time).

I will do as you suggest, and look for other places too. Please let me know about performance issues, especially if you have some tips :-). I've spent a lot of time optimizing this, but I feel I'm somewhat left at micro optimizations now.

The biggest hogger seems to be all the url() calls, mostly done by the build_row (get plugin settings) and the theming layer (ctools drop-down buttons).

I may after all have some ideas on optimizing the build_row. But I'm not sure what to do with the theming, since I'm using the ctools export ui page.

gielfeldt’s picture

Committed and pushed some time() to REQUEST_TIME changes.

thekevinday’s picture

Should this be marked as closed (fixed)?

gielfeldt’s picture

I think so. I've changed time() to REQUEST_TIME in places where I think it makes sense. Any objections to the remaining use of time()?

thekevinday’s picture

no objections here.

gielfeldt’s picture

Status: Active » Closed (fixed)