Per the talk by Rasmus at Drupalcon, we could avoid many, many, system calls by using $_SERVER['REQUEST_TIME']; rather than calling time().

To make for a better DX, we could either create a define or a wrapper function. e.g.:

define ('R_TIME', $_SERVER['REQUEST_TIME']);

or 

function r_time() {
  return $_SERVER['REQUEST_TIME'];
}
CommentFileSizeAuthor
#5 REQUEST_TIME-305645-4.patch60.33 KBpwolanin
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

kbahey’s picture

kbahey CreditAttribution: kbahey commented
Status: Active » Closed (duplicate)

This has already been implemented in core D7 in #302763.

However, I like the idea of a function or define, just because $_SERVER['blah ... is too verbose.

pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Title: Preformance: replace time() with $_SERVER['REQUEST_TIME']; » replace $_SERVER['REQUEST_TIME'] with a define
Status: Closed (duplicate) » Active

re-opening for discussion of a define.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

r_time is not developer-friendly. I would need to consult documentation to know what that meant.

Since we don't abbreviate things, the shortest we can really go is REQUEST_TIME. TIME isn't specific enough, I don't think.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Title: replace $_SERVER['REQUEST_TIME'] with a define » DX: replace $_SERVER['REQUEST_TIME'] with a define
pwolanin’s picture

pwolanin CreditAttribution: pwolanin commented
Status: Active » Needs review
FileSize
REQUEST_TIME-305645-4.patch60.33 KB

TIME would be nicest (and I don't really think anyone would be confused), but even REQUEST_TIME is far easier on the eyes and fingers than what we have now.

Ran all tests with this patch- all pass (5599 passes, 0 fails, 0 exceptions).

RobLoach’s picture

RobLoach
he/him
CreditAttribution: RobLoach commented
Status: Needs review » Reviewed & tested by the community

Applied patch, visited around the site, and then re-ran all tests with 100% passes.

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Agreed 100%. Committed to CVS HEAD. Thanks pwolanin.

dropcube’s picture

dropcube CreditAttribution: dropcube commented

I have updated the documentation at Converting 6.x modules to 7.x

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.