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

[OPTIMISATION] Better conditions transcriptions from AST inside if/while/!/bool() #2274

Open
Tracked by #2250
denis-migdal opened this issue Oct 12, 2023 · 8 comments

Comments

@denis-migdal
Copy link
Contributor

denis-migdal commented Oct 12, 2023

Hi,

The logical operators are, rightfully, converted as follow, as they need to return the value (and not just True/False) :

1 and 2
3 or 4
(! $B.$bool(locals.$test = 1) ? locals.$test : 2)
($B.$bool(locals.$test = 3) ? locals.$test : 4)

However, in direct children of nodes :

  • !
  • if
  • while
  • bool()

We don't care about the value.
For example if 1 and 2: is converted as :

if($B.set_lineno(frame, 1) && $B.$bool((! $B.$bool(locals.$test = 1) ? locals.$test : 2))){  .... }

When it could be converted to :

if($B.set_lineno(frame, 1) && ( $B.$bool(1) &&  $B.$bool(2)  )   ){  .... }

Resulting on smaller, cleaner, and faster code.

Hence I suggest the following behavior :

  1. let N the current node. If N isn't if or while or ! or bool(), stops here.
  2. let n the direct child of N.
    1. if n is a comparison operator (e.g. > or not in or in), write : ($B.rich_comp('__gt__', 1, 3)) instead of $B.$bool($B.rich_comp('__gt__', 1, 3)) (indeed it already returns a boolean.)
    2. if n is ! or and or or or bool(), write e.g. : ( [LEFT_VALUE] && [RIGHT_VALUE] ) instead of $B.$bool((! $B.$bool(locals.$test = [LEFT_VALUE]) ? locals.$test : [RIGHT_VALUE]))
      1. if [LEFT_VALUE] (or [RIGHT_VALUE]) isn't a ! or and or or or bool(), write it as : $B.bool([THE_VALUE]) (we need to convert it).
      2. else, put N = [LEFT_VALUE] and go step 2. to determine how to write [LEFT_VALUE].

Note : I think adding === true in if and while can very slightly increase execution speed as JS/the browser would knows it doesn't have to try casting the types :

if($B.set_lineno(frame, 1) && ( $B.$bool(1) &&  $B.$bool(2)  ) === true  ){  .... }

Cordially,

@denis-migdal denis-migdal changed the title [OPTIMISATION] Better conditions writting for "if"/"while"/"!" when converting AST => JS [OPTIMISATION] Better conditions transcriptions from AST inside if/while/!/bool() Oct 12, 2023
@denis-migdal
Copy link
Contributor Author

Also, there are a lot of temporary variables that aren't used.

E.g.

a = 2
b = a

Is translated as :

  var v1380 = 2
  locals_exec.a = v1380
  var v1381 = locals_exec.a
  locals_exec.b = v1381

Could be rewritten :

let v1380 = locals_exec.a = 2;
locals_exec.b = v1380; // avoid doing locals_exec.a which can be a little costly

or

locals_exec.a = 2;
locals_exec.b = locals_exec.a;

@PierreQuentel
Copy link
Contributor

Removing $B.$bool()) is not possible, because Python allows a rich comparison method to return something else than a boolean. If we removed $B.$bool(), the code below

class A:

  def __eq__(self, other):
    return []

if A() == 'xy':
    print('ok')

would not give the same result as CPython.

@PierreQuentel
Copy link
Contributor

I implemented the second suggestion in commit b1383ca

@denis-migdal
Copy link
Contributor Author

Removing $B.$bool()) is not possible, because Python allows a rich comparison method to return something else than a boolean. If we removed $B.$bool(), the code below

Hum, I think we can still do it by removing the 2.i step.

Pretty sure :

if 1 and 2:

Instead of :

if( $B.$bool( (! $B.$bool(locals.$test = 1) ? locals.$test : 2) )   ){  .... }

can be rewritten as :

if( ( $B.$bool(1) &&  $B.$bool(2)  )  ){  .... }

Because the if doesn't care about the real return value of a and b, it just care whether the expression evaluate to True or False. So in this context, the traduction of a and b, in JS, can be simplified.

@denis-migdal
Copy link
Contributor Author

denis-migdal commented Nov 30, 2023

I also wonder if using a function would be faster or slower for the general case (would requires to test that) :

function and(a,b) {
     if( $B.bool(a) )
          a;
     return b;
}

EDIT : it seems 1.57x faster, but the test is very very limited.
https://jsperf.app/nanebi/2

EDIT: I'm an idiot, I forgot that the second operand must not be evaluated if "a" is evaluated to False.

PierreQuentel added a commit that referenced this issue Nov 30, 2023
@PierreQuentel
Copy link
Contributor

Implemented in the commit above. Do you think we can close this issue ?

@denis-migdal
Copy link
Contributor Author

Hi,

Sorry for the late answer.

I am currently in the middle of an house moving so I currently doesn't have a lot of time.
I'll check on that when I'll have more time ;)

@PierreQuentel
Copy link
Contributor

Pas de problème Denis, bon courage pour le déménagement !

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

No branches or pull requests

2 participants