Uploaded image for project: '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
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: None
    • Component/s: Performance
    • Difficulty:
      Moderate
    • Affected Branches:
      MOODLE_20_STABLE

      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.

        Gliffy Diagrams

          Attachments

            Activity

            nebgor Aparup Banerjee created issue -
            nebgor Aparup Banerjee made changes -
            Field Original Value New Value
            Assignee Petr Škoda (skodak) [ skodak ] Aparup Banerjee [ nebgor ]
            Hide
            nebgor Aparup Banerjee added a comment -

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

            Show
            nebgor Aparup Banerjee added a comment - grepping for '\ for(.*(' shows a list to look into
            Hide
            nebgor 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
            nebgor 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
            nebgor Aparup Banerjee made changes -
            Affects Version/s STABLE backlog [ 10463 ]
            Affects Version/s 2.0 [ 10122 ]
            Hide
            tsala Helen Foster added a comment -

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

            Show
            tsala Helen Foster added a comment - Apu, guessing that you intended the fix version to be stable backlog, not the affects version
            tsala Helen Foster made changes -
            Labels triaged
            Fix Version/s STABLE backlog [ 10463 ]
            Affects Version/s 2.0 [ 10122 ]
            Affects Version/s STABLE backlog [ 10463 ]
            Hide
            stronk7 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
            stronk7 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
            nebgor 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
            nebgor 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!
            stronk7 Eloy Lafuente (stronk7) made changes -
            Summary for loops with function calls inside the loop conditions should be done only once before the loop if possible. count/sizeof calls are slow for big arrays. Reduce them whenever possible.
            Difficulty Moderate
            dougiamas Martin Dougiamas made changes -
            Workflow jira [ 40828 ] MDL Workflow [ 47325 ]
            stronk7 Eloy Lafuente (stronk7) made changes -
            Labels triaged performance triaged
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

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

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - I think this is a good candidate for next performance-focused sprint. raising priority and labeling.
            stronk7 Eloy Lafuente (stronk7) made changes -
            Priority Major [ 3 ] Critical [ 2 ]
            dougiamas Martin Dougiamas made changes -
            Fix Version/s STABLE Sprint 4 [ 10500 ]
            Fix Version/s STABLE backlog [ 10463 ]
            jtomkinson Jordan Tomkinson made changes -
            Comment [ testing comment mail out ]
            jtomkinson Jordan Tomkinson made changes -
            Comment [ testing comment mailout ]
            nebgor Aparup Banerjee made changes -
            Status Open [ 1 ] In Progress [ 3 ]
            Hide
            nebgor Aparup Banerjee added a comment -

            PULL-155 created

            Show
            nebgor Aparup Banerjee added a comment - PULL-155 created
            nebgor Aparup Banerjee made changes -
            Status In Progress [ 3 ] Ready for review [ 10010 ]
            Resolution Fixed [ 1 ]
            Hide
            skodak Petr Skoda 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
            skodak Petr Skoda 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.
            skodak Petr Skoda made changes -
            Status Ready for review [ 10010 ] Closed [ 6 ]
            Fix Version/s 2.0.2 [ 10421 ]
            dougiamas Martin Dougiamas made changes -
            Fix Version/s 2.0.2 [ 10421 ]
            dougiamas Martin Dougiamas made changes -
            Workflow MDL Workflow [ 47325 ] MDL Full Workflow [ 95780 ]
            stronk7 Eloy Lafuente (stronk7) made changes -
            Fix Version/s STABLE Sprint 4 [ 10500 ]

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: