Very simple. Cleans the element-by-element array shifting done in path_admin_overview (path.admin.inc):

  if ($multilanguage) {
    $header[3] = $header[2];
    $header[2] = array('data' => t('Language'), 'field' => 'language');
  }
...
    if ($multilanguage) {
      $row[4] = $row[3];
      $row[3] = $row[2];
      $row[2] = module_invoke('locale', 'language_name', $data->language);
    }

... with the wonderful PHP function array_splice, which can insert a new element in an array using array_splice($array, $offset, 0, $new_data):

  if ($multilanguage) {
    array_splice($header, 2, 0, array(array('data' => t('Language'), 'field' => 'language')));
  }
...
    if ($multilanguage) {
      array_splice($row, 2, 0, module_invoke('locale', 'language_name', $data->language));
    }

Patched and ran the path tests with 100% pass (102 passes).

CommentFileSizeAuthor
#8 318887-path-remove-array-splice-D7.patch3.52 KBdave reid
path-array-splice-D7.patch1.37 KBdave reid

Comments

Anonymous’s picture

Anonymous (not verified) commented

Simple patch. Do you have performance results?

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

If anything, this will result in a very small increase in performance since we'd be using the native PHP function for this purpose. Main purpose of this patch was to simplify the code. I'll try and do some testing today.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

Alright. Finally figured out how to get ab to test the admin/build/path page and not get 403 errors. Run on 2GHz Intel P4, 2000 users, 50 nodes (couldn't get devel_generate 7.x to use batch correctly) with path aliases and translation enabled. Before patch:

Requests per second:    4.73 [#/sec] (mean)
Time per request:       211.206 [ms] (mean)
Time per request:       211.206 [ms] (mean, across all concurrent requests)
Transfer rate:          103.22 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.0      0       0
Processing:   203  210  16.0    207     338
Waiting:      183  189  13.4    186     296
Total:        203  210  16.0    207     338

After patch:

Requests per second:    4.77 [#/sec] (mean)
Time per request:       209.526 [ms] (mean)
Time per request:       209.526 [ms] (mean, across all concurrent requests)
Transfer rate:          104.04 [Kbytes/sec] received

Connection Times (ms)
              min  mean[+/-sd] median   max
Connect:        0    0   0.0      0       0
Processing:   203  209  11.2    207     307
Waiting:      169  187  10.4    186     270
Total:        203  209  11.2    207     307

Net result, very small increase in performance, but an increase nonetheless.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Reviewed & tested by the community
dries’s picture

dries commented

Thanks for the little gem, and the performance testing. I've committed it to CVS HEAD.

Unfortunately, neither the new or the old code is tested: http://acquia.com/files/test-results/modules_path_path.admin.inc.html. As such, I'm marking this 'code needs work' so we can follow-up with some tests.

Thanks Dave.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Needs work
dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented

Ideas if this should be integrated as a new test in path.test or part of the existing path admin test?

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Status: Needs work » Needs review
StatusFileSize
new 318887-path-remove-array-splice-D7.patch3.52 KB

Actually here's a much better way to do this without doing any array manipulation.

aaronbauman’s picture

aaronbauman
he/him
commented
Status: Needs review » Needs work

Looks like #8 has been committed, so this should go back to needs work per comment #5.

dave reid’s picture

dave reid
he/him
Primary language English
Location Nebraska USA
commented
Assigned: dave reid » Unassigned
Issue tags: +Needs tests

Adding tag

  • Dries committed 3ce6808 on 8.3.x 
    - Patch #318887 by Dave Reid: clean-up some array shifting code.

  • Dries committed 3ce6808 on 8.3.x 
    - Patch #318887 by Dave Reid: clean-up some array shifting code.

  • Dries committed 3ce6808 on 8.4.x 
    - Patch #318887 by Dave Reid: clean-up some array shifting code.

  • Dries committed 3ce6808 on 8.4.x 
    - Patch #318887 by Dave Reid: clean-up some array shifting code.

  • Dries committed 3ce6808 on 9.1.x 
    - Patch #318887 by Dave Reid: clean-up some array shifting code.

Status: Needs work » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.