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

          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: