Make WordPress Core

Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#23251 closed defect (bug) (fixed)

WordPress tries to unnecessarily set the memory limit

Reported by: peterjaap Owned by: nacin
Milestone: 3.7 Priority: normal
Severity: normal Version: 2.5
Component: General Keywords: has-patch 3.7-early
Focuses: Cc:

Description (last modified by SergeyBiryukov)

Fixed bug where WordPress tries to unnecessarily set the memory limit when a value like '2G' is used instead of '2048M'.

See bad code & good code over at Github, which is what you should be using for stuff like this; https://github.com/WordPress/WordPress/pull/26

Discovered by @willem_o and @PeterJaap

Attachments (1)

23251.patch (860 bytes) - added by faishal 9 years ago.

Download all attachments as: .zip

Change History (18)

#1 @markoheijnen
9 years ago

Please add a patch instead of a link to a pull request. Also related to this ticket is: #14889

#2 @peterjaap
9 years ago

Why was that submitted 2 YEARS AGO and still hasn't been fixed, while the patch is already there?

#3 @peterjaap
9 years ago

And please migrate to Github instead of using trac.

#4 @helen
9 years ago

#14889 is closed as fixed.

#5 @markoheijnen
9 years ago

I say related. If you read the comments you will noticed why it isn't done. Also saying stuff like migrate to Github without knowing anything about the current progress isn't really smart to do.

#6 @SergeyBiryukov
9 years ago

  • Version changed from trunk to 2.5

#7 @peterjaap
9 years ago

I've read the comments, can't see why the initial issue still isn't solved. It seems the commenters go off-topic with talk about benchmarking and the -1 option while all they should do is fix the bug and go from there. And it's marked as fixed, but its far from fixed.

#8 @nacin
9 years ago

  • Milestone changed from Awaiting Review to 3.6

Maybe I missed it then (and now, as I just re-read the ticket), but it seems that #14889 was primarily hung up on WP_MEMORY_LIMIT possibly being specified in G, rather than PHP's memory_limit core ini directive. You can shout in all caps but it isn't very respectful.

I'm fine with a patch along these lines. It would be good to determine exactly what values of memory_limit we should listen to. For example, it accounts for case insensitivity, which seems like a good idea. We have wp_convert_hr_to_bytes() in wp-includes/media.php — should we be moving that to wp-includes/load.php, improve it directly, and adapt it here?

While we don't accept pull requests via Github, you can simply add .diff or .patch to many GitHub URLs to get valid patches. For example: https://github.com/WordPress/WordPress/pull/26.diff. We also have no plans to migrate to GitHub, but that doesn't mean you can't use it to contribute — http://scribu.net/wordpress/contributing-to-wordpress-using-github.html.

#9 @SergeyBiryukov
9 years ago

  • Description modified (diff)
  • Summary changed from Fixed bug where Wordpress tries to unnecessarily set the memory limit to WordPress tries to unnecessarily set the memory limit

#10 @SergeyBiryukov
9 years ago

  • Keywords needs-patch added; has-patch removed

9 years ago

#11 @faishal
9 years ago

  • Keywords has-patch added; needs-patch removed

#12 @nacin
8 years ago

  • Keywords 3.7-early added
  • Milestone changed from 3.6 to Future Release

Going to move this to 3.7. Thanks for the contributions.

#13 @wonderboymusic
8 years ago

  • Milestone changed from Future Release to 3.7

these are all marked 3.7-early

#14 @nacin
8 years ago

Per http://www.php.net/manual/en/faq.using.php#faq.using.shorthandbytes, only K, M, and G are allowed. Trying to account for T is kind of insane.

#15 @nacin
8 years ago

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

In 25684:

Don't override a PHP memory_limit specified using G shorthand.

props peterjaap.
fixes #23251.

#16 @nacin
8 years ago

In 25685:

Support G shorthand for WP_MEMORY_LIMIT. see #23251.

#17 @ocean90
8 years ago

stripos() is new in PHP 5, means you get a fatal error on PHP 4 now.

Note: See TracTickets for help on using tickets.