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

Using generate_args0 : -20% exec time??? #2316

Conversation

denis-migdal
Copy link
Contributor

@denis-migdal denis-migdal commented Nov 9, 2023

Not ready for merge.

Link to the old benchmark

Using the following benchmark :

from time import perf_counter as timer

def ftest1():
  pass

def ftest2(x):
  pass

def ftest3(x, y):
  pass
  
def ftest4(x, y, z):
  pass

def ftest5(*args):
  pass

def ftest6(x=0):
  pass
  
results = []

def add_result(*args):
    results.append(args)
    
N = 1000000

t0 = timer()
for i in range(N):
  ftest1()
  ftest2(i)
  ftest5(i,i)
  ftest6()
  ftest3(i, i)
  ftest6(i)
  ftest3(y=1, x=2)
  ftest4(i, i, i)

add_result('total', timer() - t0)

print(results)

Usage of generateArgs0 is toggled in js_to_ast.js line 2107 : const USE_PERSO_ARGS0 = true;.

It seems execution time is reduced by 20% (from 3.15sec to 2.5sec).

Tasks remaining :

  • Modify the JS generation to use new parsing for all functions in order to validate against the unit tests
    • Need to store the parsing function in the function infos (to not make tests during function execution).
    • Will require to modify the default attributes setter (need to find where this is defined)
    • Optimize function creation by having a "cache" of parsing functions.
    • Pass unit tests
  • Optimize my output
    • test unique combinations output ?
    • do not use useless variables
    • precompute some info ?
    • $defaults also built upon function create ? (implies useless double call to getArgs0).
    • Better handling of {$kw:[{},{}]}.
    • better handling of dicts ?
    • split kw args loop (first one vs the next ???) => would remove some conditions ?

@denis-migdal
Copy link
Contributor Author

#2283

@denis-migdal
Copy link
Contributor Author

Aaaaaaaand I retest and fail the firsts unit tests WTF

@PierreQuentel
Copy link
Contributor

Salut Denis,

Impressive work !

I have ran the same tests as a few days ago, the results are not really different from the current development version. Do you get different results ?

Firefox
func with no arg -2.540
func 1 positional arg 14.163
func 2 positional args 8.686
func 3 positional args -6.418
func with *args -1.626
"def f(x=0)" called with 1 argument -10.144
"def f(x=0)" called without argument 5.420
"def f(x, y)" called with f(y=1, x=2) 9.582

Chrome
func with no arg -0.310
func 1 positional arg -1.841
func 2 positional args -3.302
func 3 positional args -2.651
func with *args -3.159
"def f(x=0)" called with 1 argument -12.951
"def f(x=0)" called without argument -3.359
"def f(x, y)" called with f(y=1, x=2) -3.423

@denis-migdal
Copy link
Contributor Author

denis-migdal commented Nov 11, 2023

I have ran the same tests as a few days ago, the results are not really different from the current development version. Do you get different results ?

I still haven't removed some garbage and optimized it.
The fact I have to do fct.$infos.args_parser() instead of fct.args_parser() is a little costly.

Still, your results with Firefox are a little strange. I'll have to check on that.
Normally it is roughly the same code as before, with some lines/conditions removed so it shouldn't be slower in any cases.
Maybe I let some debug code that is costly, or something else is causing issue.

When testing each function separately the difference might be difficult to see as the browser will likely optimize some conditions when it sees that they are often "true" (which in real life isn't always the case).

I'll have to check the difference in speed between Function(...args, code) and eval("function X"), maybe Function is doing something I do not expect.
EDIT: I see no difference https://jsperf.app/zixize

@denis-migdal

This comment was marked as outdated.

@denis-migdal

This comment was marked as outdated.

@denis-migdal
Copy link
Contributor Author

It's strange.

When using fct.$args_parser it works, but when using fct.args_parser it doesn't.
Will push new commit.

@denis-migdal
Copy link
Contributor Author

denis-migdal commented Nov 11, 2023

I have ran the same tests as a few days ago, the results are not really different from the current development version. Do you get different results ?

I do get different results on Firefox (toggle USE_PERSO_ARGS0_EVERYWHERE line 2163 of src/ast_to_js.js) :

func with no arg 6.466
func 1 positional arg -3.354
func 2 positional args -6.838
func 3 positional args -10.428
func with *args -5.830
"def f(x=0)" called with 1 argument -10.914
"def f(x=0)" called without argument -5.168
"def f(x, y)" called with f(y=1, x=2) -8.852

Notice that "func with no args" is +6.4% slower with my version.... when this is the same code between the 2 versions...
Still have some variables cleaning to do.

Raw data :

NEW [('func with no arg', 0.24700000000000166), ('func 1 positional arg', 0.3169999999999984), ('func 2 positional args', 0.32699999999999996), ('func 3 positional args', 0.33500000000000085), ('func with *args', 0.41999999999999993), ('"def f(x=0)" called with 1 argument', 0.3020000000000014), ('"def f(x=0)" called without argument', 0.3669999999999991), ('"def f(x, y)" called with f(y=1, x=2)', 0.3810000000000002)]

OLD [('func with no arg', 0.23199999999999932), ('func 1 positional arg', 0.3279999999999994), ('func 2 positional args', 0.3509999999999991), ('func 3 positional args', 0.37400000000000055), ('func with *args', 0.44599999999999973), ('"def f(x=0)" called with 1 argument', 0.3390000000000004), ('"def f(x=0)" called without argument', 0.38700000000000045), ('"def f(x, y)" called with f(y=1, x=2)', 0.41799999999999926)]

@denis-migdal
Copy link
Contributor Author

denis-migdal commented Nov 11, 2023

Should now be a little more optimized for :

  • (true, DEFAULTS.NONE, false, DEFAULTS.NONE, false, false, DEFAULTS.NONE, false)
    • replace > ARGS_POS_COUNT and < ARGS_POS_COUNT by != ARGS_POS_COUNT.
  • (true, DEFAULTS.SOME, false, DEFAULTS.NONE, false, false, DEFAULTS.NONE, false)
  • (true, DEFAULTS.ALL, false, DEFAULTS.NONE, false, false, DEFAULTS.NONE, false)
  • (false, DEFAULTS.NONE, true, DEFAULTS.NONE, false, false, DEFAULTS.NONE, false)
  • (false, DEFAULTS.NONE, true, DEFAULTS.SOME, false, false, DEFAULTS.NONE, false)
  • (false, DEFAULTS.NONE, true, DEFAULTS.ALL, false, false, DEFAULTS.NONE, false)
  • (false, DEFAULTS.NONE, false, DEFAULTS.NONE, true, false, DEFAULTS.NONE, false)
  • (false, DEFAULTS.NONE, false, DEFAULTS.NONE, false, true, DEFAULTS.NONE, false)
  • (false, DEFAULTS.NONE, false, DEFAULTS.NONE, false, true, DEFAULTS.SOME, false)
  • (false, DEFAULTS.NONE, false, DEFAULTS.NONE, false, true, DEFAULTS.ALL, false)
  • (false, DEFAULTS.NONE, false, DEFAULTS.NONE, false, false, DEFAULTS.NONE, true)
    • Remove found and ioffset

Not gonna lie, it is a little hard to optimize the output considering the number of possible combinations ( 192? ).

@denis-migdal
Copy link
Contributor Author

Results with old benchmark :

func with no arg 2.586
func 1 positional arg 1.961
func 2 positional args -7.670
func 3 positional args -7.910
func with *args -3.211
"def f(x=0)" called with 1 argument -9.006
"def f(x=0)" called without argument -1.096
"def f(x, y)" called with f(y=1, x=2) -4.124

Raw data :

NEW [('func with no arg', 0.23799999999999955), ('func 1 positional arg', 0.3119999999999976), ('func 2 positional args', 0.3130000000000024), ('func 3 positional args', 0.3259999999999934), ('func with *args', 0.42199999999999704), ('"def f(x=0)" called with 1 argument', 0.29299999999999216), ('"def f(x=0)" called without argument', 0.3610000000000042), ('"def f(x, y)" called with f(y=1, x=2)', 0.3719999999999999)]

OLD [('func with no arg', 0.23199999999999932), ('func 1 positional arg', 0.30600000000000094), ('func 2 positional args', 0.33899999999999864), ('func 3 positional args', 0.354000000000001), ('func with *args', 0.43599999999999994), ('"def f(x=0)" called with 1 argument', 0.32200000000000095), ('"def f(x=0)" called without argument', 0.3650000000000002), ('"def f(x, y)" called with f(y=1, x=2)', 0.3879999999999999)]

Results with new benchmark :

OLD : 3.2070000000000007
NEW : 2.5

-21.875% on execution time.

Discussion :

  • Maybe I need to test on more data to have more stable results for the first benchmark ?

  • Argument parsing is now up to 41.19% (*args) of total exec time, and down to 22% (1 pos arg) of total exec time.
    The cost of *args is probably mainly due to the slice(), but I think we need the slice. Which means that other things are now responsible of 59% to 78% of total exec time. Gotta make a new test someday.

  • Better handling of python dictionaries (only having to look at $strings or $jsobj) would allow quicker parsing of named arguments.

  • If last argument were guaranteed to be either null or named argument, it'll allow some opti.

@denis-migdal
Copy link
Contributor Author

denis-migdal commented Nov 11, 2023

With 10x more data :

func with no arg 5.683
func 1 positional arg -11.239
func 2 positional args -9.799
func 3 positional args -9.014
func with *args -10.112
"def f(x=0)" called with 1 argument -10.022
"def f(x=0)" called without argument -5.629
"def f(x, y)" called with f(y=1, x=2) -9.742

New version seems overall faster.
Funny that func with no arg is slower when this is the same code executed. I assume this is due to RNG and some biais.

Raw data:

NEW [('func with no arg', 2.398999999999999), ('func 1 positional arg', 2.851000000000001), ('func 2 positional args', 3.0009999999999977), ('func 3 positional args', 3.1290000000000013), ('func with *args', 4.0), ('"def f(x=0)" called with 1 argument', 2.8369999999999997), ('"def f(x=0)" called without argument', 3.5539999999999985), ('"def f(x, y)" called with f(y=1, x=2)', 3.493000000000002)]

OLD [('func with no arg', 2.269999999999996), ('func 1 positional arg', 3.2120000000000033), ('func 2 positional args', 3.326999999999998), ('func 3 positional args', 3.438999999999993), ('func with *args', 4.450000000000003), ('"def f(x=0)" called with 1 argument', 3.153000000000006), ('"def f(x=0)" called without argument', 3.765999999999991), ('"def f(x, y)" called with f(y=1, x=2)', 3.8700000000000045)]

@denis-migdal
Copy link
Contributor Author

denis-migdal commented Nov 12, 2023

Hum... I may have an idea to speed up processing of defaults values when kwargs are passed :
https://jsperf.app/turube/2

@PierreQuentel PierreQuentel merged commit 811b4fe into brython-dev:master Nov 18, 2023
1 check passed
@PierreQuentel
Copy link
Contributor

I have merged the PR. Thanks for the hard work Denis !

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.

2 participants