Moodle
  1. Moodle
  2. MDL-23754 Performance improvements META
  3. MDL-25637

count/sizeof calls are slow for big arrays. Reduce them whenever possible.

    Details

    • Type: Sub-task Sub-task
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: None
    • Component/s: Performance
    • Difficulty:
      Moderate
    • Affected Branches:
      MOODLE_20_STABLE
    • Rank:
      6370

      Description

      there are places where we can find:
      for($i = 1; $i < sizeof($data); $i++) {...

      these should really be
      $datasize = sizeof($data);
      for($i = 1; $i < $datasize; $i++) { ...

      this doesn't apply to places where the loop condition expression on both sides of the expression are dynamic.

        Activity

        Hide
        Aparup Banerjee added a comment -

        grepping for '\ for(.*(' shows a list to look into

        Show
        Aparup Banerjee added a comment - grepping for '\ for(.*(' shows a list to look into
        Hide
        Aparup Banerjee added a comment -

        switching to stable backlog so a sit-rep as this was in progress..

        list of possible places to fix..
        ./backup/restorelib.php:380: for($i = 0; $i < sizeof($tagsarr); $i++) {
        ./backup/lib.php:132: for($i=0; $i<count($dir_files); $i++) {
        ./backup/lib.php:140: for($i=0; $i<count($dir_subdirs); $i++) {
        ./mod/glossary/import.php:190: for($i = 0; $i < sizeof($xmlentries); $i++) {
        ./mod/glossary/import.php:244: for($k = 0; $k < sizeof($xmlaliases); $k++) {
        ./mod/glossary/import.php:260: for($k = 0; $k < sizeof($xmlcats); $k++) {
        ./mod/wiki/parser/markups/nwiki.php:145: for($i = 0; $i < count($headercells); $i++) {
        ./mod/wiki/diff/difflib.php:303: for(;$pos < strlen($data) && substr($data,$pos,1)===' ';$pos++) ;
        ./mod/wiki/diff/difflib.php:530: for($index1=count($diff);$index1>=1;$index1--) {
        ./mod/lesson/importppt.php:113: for($i = 0; $i < count($matches[1]); $i++) { // go through all of our div matches
        ./mod/feedback/item/numeric/lib.php:169: // for($i = 1; $i < sizeof($data); $i++) {
        ./mod/feedback/item/multichoice/lib.php:122: for($i = 1; $i <= sizeof($answers); $i++) {
        ./mod/feedback/item/multichoice/lib.php:139: for($i = 1; $i <= sizeof($answers); $i++) {
        ./mod/feedback/item/multichoice/lib.php:170: for($i = 0; $i < sizeof($vallist); $i++) {
        ./mod/feedback/item/multichoice/lib.php:171: for($k = 0; $k < sizeof($presentation); $k++) {
        ./mod/feedback/item/multichoice/lib.php:235: for($i = 0; $i < sizeof($data); $i++) {
        ./mod/feedback/item/multichoice/lib.php:570: for($i = 1; $i < sizeof($arrvals); $i++) {
        ./mod/feedback/item/multichoicerated/lib.php:124: for($i = 1; $i <= sizeof($lines); $i++) {
        ./mod/feedback/item/multichoicerated/lib.php:218: for($i = 0; $i < sizeof($data); $i++) {
        ./mod/feedback/item/textarea/lib.php:145: for($i = 1; $i < sizeof($data); $i++) {
        ./mod/feedback/item/textfield/lib.php:142: for($i = 1; $i < sizeof($data); $i++) {
        ./mod/feedback/item/info/lib.php:133: for($i = 0; $i < sizeof($data); $i++) {
        ./mod/feedback/item/info/lib.php:151: for($i = 1; $i < sizeof($data); $i++) {
        ./auth/shibboleth/logout.php:203: for( $i = 0; $i < count( $a ); $i = $i+2 ) {
        ./question/type/calculatedsimple/edit_calculatedsimple_form.php:171: for($key = 1 ; $key <= sizeof($olddef) ; $key++) {
        ./question/format/blackboard_six/format.php:76: for($i=0; $i<count($dir_files); $i++) {
        ./question/format/blackboard_six/format.php:84: for($i=0; $i<count($dir_subdirs); $i++)

        list of whats been done

        1. modified: backup/lib.php
        2. modified: backup/restorelib.php
        3. modified: mod/feedback/item/multichoice/lib.php
        4. modified: mod/glossary/import.php
        5. modified: mod/lesson/importppt.php
        6. modified: mod/wiki/diff/difflib.php
        7. modified: mod/wiki/parser/markups/nwiki.php...

        can be pulled from this location (to resume work):
        https://github.com/nebgor/moodle/commit/defd1a7daedb4ec8703256b3083a47a6e1a06627

        Show
        Aparup Banerjee added a comment - switching to stable backlog so a sit-rep as this was in progress.. list of possible places to fix.. ./backup/restorelib.php:380: for($i = 0; $i < sizeof($tagsarr); $i++) { ./backup/lib.php:132: for($i=0; $i<count($dir_files); $i++) { ./backup/lib.php:140: for($i=0; $i<count($dir_subdirs); $i++) { ./mod/glossary/import.php:190: for($i = 0; $i < sizeof($xmlentries); $i++) { ./mod/glossary/import.php:244: for($k = 0; $k < sizeof($xmlaliases); $k++) { ./mod/glossary/import.php:260: for($k = 0; $k < sizeof($xmlcats); $k++) { ./mod/wiki/parser/markups/nwiki.php:145: for($i = 0; $i < count($headercells); $i++) { ./mod/wiki/diff/difflib.php:303: for(;$pos < strlen($data) && substr($data,$pos,1)===' ';$pos++) ; ./mod/wiki/diff/difflib.php:530: for($index1=count($diff);$index1>=1;$index1--) { ./mod/lesson/importppt.php:113: for($i = 0; $i < count($matches [1] ); $i++) { // go through all of our div matches ./mod/feedback/item/numeric/lib.php:169: // for($i = 1; $i < sizeof($data); $i++) { ./mod/feedback/item/multichoice/lib.php:122: for($i = 1; $i <= sizeof($answers); $i++) { ./mod/feedback/item/multichoice/lib.php:139: for($i = 1; $i <= sizeof($answers); $i++) { ./mod/feedback/item/multichoice/lib.php:170: for($i = 0; $i < sizeof($vallist); $i++) { ./mod/feedback/item/multichoice/lib.php:171: for($k = 0; $k < sizeof($presentation); $k++) { ./mod/feedback/item/multichoice/lib.php:235: for($i = 0; $i < sizeof($data); $i++) { ./mod/feedback/item/multichoice/lib.php:570: for($i = 1; $i < sizeof($arrvals); $i++) { ./mod/feedback/item/multichoicerated/lib.php:124: for($i = 1; $i <= sizeof($lines); $i++) { ./mod/feedback/item/multichoicerated/lib.php:218: for($i = 0; $i < sizeof($data); $i++) { ./mod/feedback/item/textarea/lib.php:145: for($i = 1; $i < sizeof($data); $i++) { ./mod/feedback/item/textfield/lib.php:142: for($i = 1; $i < sizeof($data); $i++) { ./mod/feedback/item/info/lib.php:133: for($i = 0; $i < sizeof($data); $i++) { ./mod/feedback/item/info/lib.php:151: for($i = 1; $i < sizeof($data); $i++) { ./auth/shibboleth/logout.php:203: for( $i = 0; $i < count( $a ); $i = $i+2 ) { ./question/type/calculatedsimple/edit_calculatedsimple_form.php:171: for($key = 1 ; $key <= sizeof($olddef) ; $key++) { ./question/format/blackboard_six/format.php:76: for($i=0; $i<count($dir_files); $i++) { ./question/format/blackboard_six/format.php:84: for($i=0; $i<count($dir_subdirs); $i++) list of whats been done modified: backup/lib.php modified: backup/restorelib.php modified: mod/feedback/item/multichoice/lib.php modified: mod/glossary/import.php modified: mod/lesson/importppt.php modified: mod/wiki/diff/difflib.php modified: mod/wiki/parser/markups/nwiki.php... can be pulled from this location (to resume work): https://github.com/nebgor/moodle/commit/defd1a7daedb4ec8703256b3083a47a6e1a06627
        Hide
        Helen Foster added a comment -

        Apu, guessing that you intended the fix version to be stable backlog, not the affects version

        Show
        Helen Foster added a comment - Apu, guessing that you intended the fix version to be stable backlog, not the affects version
        Hide
        Eloy Lafuente (stronk7) added a comment -

        LOL, I also saw this and was creating on filter + report here to check that "backlog XXX" was never used in affect version field and I was getting crazy why the report wasn't revealing this issue.

        Hehe, you fixed it, perfect! (only one anecdote).

        Show
        Eloy Lafuente (stronk7) added a comment - LOL, I also saw this and was creating on filter + report here to check that "backlog XXX" was never used in affect version field and I was getting crazy why the report wasn't revealing this issue. Hehe, you fixed it, perfect! (only one anecdote).
        Hide
        Aparup Banerjee added a comment -

        oops, yea one wrong bulk operation on a friday evening and a whole mess. fixed all my issues. Thanks for heads up!

        Show
        Aparup Banerjee added a comment - oops, yea one wrong bulk operation on a friday evening and a whole mess. fixed all my issues. Thanks for heads up!
        Hide
        Eloy Lafuente (stronk7) added a comment -

        I think this is a good candidate for next performance-focused sprint. raising priority and labeling.

        Show
        Eloy Lafuente (stronk7) added a comment - I think this is a good candidate for next performance-focused sprint. raising priority and labeling.
        Hide
        Aparup Banerjee added a comment -

        PULL-155 created

        Show
        Aparup Banerjee added a comment - PULL-155 created
        Hide
        Petr Škoda added a comment -

        Next time let's do changes like this in dev branch instead of stable. For stable we should only pick those that really improve performance in a measurable way on real sites.

        Closing, thanks.

        Show
        Petr Škoda added a comment - Next time let's do changes like this in dev branch instead of stable. For stable we should only pick those that really improve performance in a measurable way on real sites. Closing, thanks.

          People

          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: