Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[5.4] Use do while instead of goto #15973

Closed
wants to merge 1 commit into from
Closed

Conversation

bepsvpt
Copy link
Contributor

@bepsvpt bepsvpt commented Oct 18, 2016

No description provided.

@GrahamCampbell GrahamCampbell changed the title Use do while instead of goto [5.4] Use do while instead of goto Oct 18, 2016
@GrahamCampbell
Copy link
Member

Goto was used because it keeps the call stack down. Your implementation will crash after too many retrys.

@bepsvpt
Copy link
Contributor Author

bepsvpt commented Oct 18, 2016

I'm using the following code to get the stack trace, it seems there is no different between do while and goto. Is there something wrong with the following code?

do while: https://ideone.com/MYBV9w
goto: https://ideone.com/EoGhdF

@taylorotwell
Copy link
Member

Here is the reasoning for using goto given in the library that inspired the function: igorw/retry#3

@bepsvpt
Copy link
Contributor Author

bepsvpt commented Oct 18, 2016

Here is the opcodes generated using php70, I remove $sleep parameter and optimize goto version to make comparison easier.

do_while.php

<?php

function retry($times, callable $callback)
{
    do {
        try {
            return $callback();
        } catch (Exception $e) {
        }
    } while (--$times);

    throw $e;
}

do_while.php opcodes

filename:       do_while.php
function name:  retry
number of ops:  19
compiled vars:  !0 = $times, !1 = $callback, !2 = $e
line     #* E I O op                           fetch          ext  return  operands
-------------------------------------------------------------------------------------
   3     0  E >   EXT_NOP
         1        RECV                                             !0
         2        RECV                                             !1
   5     3        NOP
   6     4    >   NOP
   7     5        EXT_STMT
         6        INIT_DYNAMIC_CALL                                        !1
         7        EXT_FCALL_BEGIN
         8        DO_FCALL                                      0  $3
         9        EXT_FCALL_END
        10      > RETURN                                                   $3
        11*       JMP                                                      ->13
   8    12  E > > CATCH                                                    'Exception', !2
  10    13    >   PRE_DEC                                          $4      !0
        14      > JMPNZ                                                    $4, ->4
  12    15    >   EXT_STMT
        16      > THROW                                         0          !2
  13    17*       EXT_STMT
        18*     > RETURN                                                   null

goto.php

<?php

function retry($times, callable $callback)
{
    beginning:
    try {
        return $callback();
    } catch (Exception $e) {
        if (! --$times) {
            throw $e;
        }

        goto beginning;
    }
}

goto.php opcodes

filename:       goto.php
function name:  retry
number of ops:  22
compiled vars:  !0 = $times, !1 = $callback, !2 = $e
line     #* E I O op                           fetch          ext  return  operands
-------------------------------------------------------------------------------------
   3     0  E >   EXT_NOP
         1        RECV                                             !0
         2        RECV                                             !1
   6     3    >   NOP
   7     4        EXT_STMT
         5        INIT_DYNAMIC_CALL                                        !1
         6        EXT_FCALL_BEGIN
         7        DO_FCALL                                      0  $3
         8        EXT_FCALL_END
         9      > RETURN                                                   $3
        10*       JMP                                                      ->20
   8    11  E > > CATCH                                                    'Exception', !2
   9    12    >   EXT_STMT
        13        PRE_DEC                                          $4      !0
        14        BOOL_NOT                                         ~5      $4
        15      > JMPZ                                                     ~5, ->18
  10    16    >   EXT_STMT
        17      > THROW                                         0          !2
  13    18    >   EXT_STMT
        19      > JMP                                                      ->3
  15    20*       EXT_STMT
        21*     > RETURN                                                   null

@taylorotwell, it seems that goto version takes additional BOOL_NOT(#14) and JMP(#19) opcodes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants