Make WordPress Core

Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#25161 closed task (blessed) (fixed)

Define SCRIPT_DEBUG in trunk/src and remove it when building

Reported by: azaozz's profile azaozz Owned by: nacin's profile nacin
Milestone: 3.7 Priority: normal
Severity: normal Version: 3.7
Component: General Keywords:
Focuses: Cc:

Description

Running file_exists() on every load (incl. on the front-end) to determine whether SCRIPT_DEBUG should be set is expensive.

SCRIPT_DEBUG can be defined in trunk/src/ and removed when copying the files with Grunt.

Attachments (2)

25161.patch (2.0 KB) - added by azaozz 11 years ago.
25161.2.patch (1.5 KB) - added by azaozz 11 years ago.

Download all attachments as: .zip

Change History (13)

@azaozz
11 years ago

#1 @nacin
11 years ago

I think there is a need for a solution here (the file_exists() check was just temporary) but I am not in love with 25161.patch. Other ideas welcome — I don't have one.

#2 follow-up: @TobiasBg
11 years ago

Is that file_exists() really that much more expensive than e.g. require_once() that we are doing so many times per page load, anyway?

#3 in reply to: ↑ 2 @nacin
11 years ago

Replying to TobiasBg:

Is that file_exists() really that much more expensive than e.g. require_once() that we are doing so many times per page load, anyway?

require/include, when done unconditionally, can be sped up with an opcode cache and the sysopen call is avoided. That's not the case for conditional includes or any other kind of stat calls like file_exists().

#4 @azaozz
11 years ago

I am not in love with 25161.patch

Yeah, not particularly happy with it too. Filtering PHP code with a regex is not ideal. If we end up doing something like this, at least the filter should be really simple.

Setting something in /src that is removed by Grunt in /build can work. One problem is that the processContentExclude option on the copy task works only on directories, and we don't want to process hundreds or thousands of files. We can work around this by setting two 'copy' tasks, the first will copy all files except the file(s) that need processing and the second will copy and process the remaining file(s).

Thinking the proper place to do this is in version.php. How about we add -src to the end of $wp_version? This will make the filtering regex much simpler.

Another thing that can be handled there is cache-busting for js and css in /build. Bumpbot was pretty good about it, now we need an alternative.

Last edited 11 years ago by azaozz (previous) (diff)

#5 @adamsilverstein
11 years ago

  • Cc adamsilverstein@… added

#6 @nacin
11 years ago

  • Severity changed from normal to blocker

Thinking the proper place to do this is in version.php. How about we add -src to the end of $wp_version? This will make the filtering regex much simpler.

I think this is a solid alternative. The script needs to be modified anyway to handle $wp_version changes, so I will do this.

#7 @nacin
11 years ago

In 25692:

Append -src to the $wp_version in the develop repo (only) to allow for us to differentiate. see #25161.

#8 @nacin
11 years ago

  • Severity changed from blocker to normal
  • Type changed from defect (bug) to task (blessed)

Currently the syncing script strips -src. Ideally grunt does this, in part so the builddirectory does not reflect -src. Using the processContent approach above slows down the build process by a few seconds, even when bailing when filepath !== 'src/wp-includes/version.php'. So I think we instead need to use something like https://npmjs.org/package/grunt-replace, https://npmjs.org/package/grunt-string-replace, or https://npmjs.org/package/grunt-text-replace. Thoughts?

@azaozz
11 years ago

#9 @azaozz
11 years ago

In 25161.2.patch testing with splitting the "copy" task in two: one that copies all except version.php, and another that copies and processes only version.php. That way the delay of using processContent() is very small. The Grunt replace plugins do something similar.

#10 @nacin
11 years ago

  • Owner set to nacin
  • Resolution set to fixed
  • Status changed from new to closed

In 25693:

New grunt copy:version task that removes -src from $wp_version on build.

props azaozz.
fixes #25161.

#11 @SergeyBiryukov
11 years ago

  • Version changed from trunk to 3.7
Note: See TracTickets for help on using tickets.