Opened 14 years ago
Closed 11 years ago
#14424 closed enhancement (fixed)
Eliminate dynamic function definitions, ie create_function
Reported by: | ScottMac | Owned by: | westi |
---|---|---|---|
Milestone: | 3.9 | Priority: | normal |
Severity: | normal | Version: | 3.0 |
Component: | General | Keywords: | ongoing-project has-patch |
Focuses: | performance | Cc: |
Description
create_function() calls are essentially a form of eval() where the body of the functions is defined at runtime rather than compile time. This patch removes all but one of the calls within the translation code.
There are a few reasons for doing this, one is apc can't cache the results of the function definition and secondly HipHop doesn't support complex create_function definitions.
Most of the references were array_filter / array_map and replaced with a simple foreach.
Others were sorting and a callback function was added.
Attachments (10)
Change History (46)
#7
follow-up:
↓ 10
@
14 years ago
[16034] missed the ticket.
I think we have that exact helper function in deprecated.php, _funky_javascript_callback().
#13
follow-up:
↓ 17
@
14 years ago
huichen was the Google Summer of Code student for HipHip, and one of his tasks was to make WordPress run on it.
Last commit came from here: https://github.com/huichen/wordpress/commit/a1cf1b0b3c496f4f8ee0295200208fada877eae8
#18
@
13 years ago
- Keywords has-patch added; needs-patch removed
In r16313 _wp_tag_cloud_name_sort_cb and _wp_tag_cloud_count_sort_cb are introduced but not used. Patch will fix it.
#23
@
11 years ago
- Keywords has-patch removed
PHP is considering deprecating create_function
in 5.6 (~2015). This isn't accepted or anywhere close yet, but as a matter of best practice, we should avoid using it in core.
With the exception of the usage in POMO and wp_generate_tag_cloud
, these seem pretty trivial to convert to normal functions. Alternatively, we can wait until it's accepted and nearing release, by which point I hazard a guess we can consider 5.3+ with closures anyway.
#24
@
11 years ago
I am all for doing whatever it takes to removing all uses of create_function(), even outright refactoring.
#25
follow-up:
↓ 27
@
11 years ago
- Keywords has-patch added
Patch converts the remaining ones, outside of POMO and the AtomParser class.
I noticed that importers were sorted twice, once with strcmp()
and once with strnatcasecmp()
. Is it okay to go with the latter in both cases?
#26
@
11 years ago
- Component changed from Optimization to General
- Focuses performance added
- Milestone changed from Future Release to 3.9
#27
in reply to:
↑ 25
@
11 years ago
Replying to obenland:
I noticed that importers were sorted twice, once with
strcmp()
and once withstrnatcasecmp()
. Is it okay to go with the latter in both cases?
Should be fine.
#28
@
11 years ago
So, I don't know whether that $_GET['paged']
superglobal modification was just for the limit, or if other things depend on it. I really don't like it, and would like to remove it.
#32
in reply to:
↑ 30
@
11 years ago
Replying to nacin:
In 27374:
It seems _uasort_by_first_member callback was directly used in wp-admin/import.php (line 77). This commit leads then to the following PHP warning:
uasort() expects parameter 2 to be a valid callback, function '_uasort_by_first_member' not found or invalid function name
The callback should be updated in that file too, or we may use a new function to merge data from get_importers() and wp_get_popular_importers()?
Needs a bit more massaging. The first foreach loop, for instance, does not initialize the array.