Modify

Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#797 closed enhancement (fixed)

PhpLintTask performance improvement

Reported by: Elan Ruusamäe <glen@…> Owned by: mrook
Priority: minor Milestone: 2.4.8
Component: phing-core Version: 2.4.7.1
Keywords: Cc:

Description

PhpLintTask calls out php -l, but php engine does not need any extension to check file for syntax errors, therefore a lot of time could be won if php-cli is told not to load php.ini and thus load and setup extensions:

so I propose to invoke as:

  php -n -l $FILE

my timing win is 10s -> 1s with just 69 files:

$ time phing phplint | grep -F '[phplint]' | wc -l
69
real    0m10.578s
user    0m7.846s
sys     0m2.530s
$ time phing phplint | grep -F '[phplint]' | wc -l
69
real    0m1.808s
user    0m0.927s
sys     0m0.683s

Attachments (1)

bug-797.patch.txt (492 bytes) - added by Elan Ruusamäe <glen@…> 3 years ago.
proposed patch

Download all attachments as: .zip

Change History (7)

Changed 3 years ago by Elan Ruusamäe <glen@…>

proposed patch

comment:1 Changed 3 years ago by mrook

  • Milestone changed from TBD to 2.4.8
  • Priority changed from tbd to minor
  • Status changed from new to assigned

Thanks!

comment:2 Changed 3 years ago by mrook

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [1310]) Fixes #797 - do not load php.ini file, increases php lint performance

comment:3 follow-up: Changed 3 years ago by mrook

(In [1312]) Refs #797 - set error_reporting to include E_DEPRECATED when deprecatedAsError is true

comment:4 in reply to: ↑ 3 ; follow-up: Changed 3 years ago by Elan Ruusamäe <glen@…>

Replying to mrook:

(In [1312]) Refs #797 - set error_reporting to include E_DEPRECATED when deprecatedAsError is true

i'm sure you as maintainer know what you're doing, but what about other error_level options that _may_ have been configured in global php.ini? perhaps just try to collect them and pass whole bitmask not just deprecated?

comment:5 in reply to: ↑ 4 ; follow-up: Changed 3 years ago by mrook

Replying to Elan Ruusamäe <glen@…>:

Replying to mrook:

(In [1312]) Refs #797 - set error_reporting to include E_DEPRECATED when deprecatedAsError is true

i'm sure you as maintainer know what you're doing, but what about other error_level options that _may_ have been configured in global php.ini? perhaps just try to collect them and pass whole bitmask not just deprecated?

The objective of your patch was to improve the performance by not loading the php.ini. This change is needed to facilitate detection of E_DEPRECATED errors.

comment:6 in reply to: ↑ 5 Changed 3 years ago by Elan Ruusamäe <glen@…>

Replying to mrook:

The objective of your patch was to improve the performance by not loading the php.ini. This change is needed to facilitate detection of E_DEPRECATED errors.

I was kind of thinking inheritance of settings from already running Phing process (not each time php is about to invoked). Indeed those could had been already modified by some other options or Phing internal states. So if only deprecation is needed, all is fine :)

Add Comment

Modify Ticket

Action
as closed The owner will remain mrook.
The resolution will be deleted. Next status will be 'reopened'.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.