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

[OPTIMIZATION] Lazy Python => JS transcription ? #2318

Open
denis-migdal opened this issue Nov 11, 2023 · 9 comments
Open

[OPTIMIZATION] Lazy Python => JS transcription ? #2318

denis-migdal opened this issue Nov 11, 2023 · 9 comments

Comments

@denis-migdal
Copy link
Contributor

Hi,

Though of some ideas, some maybe ridiculous, but I think they are interesting to talk about.

@PierreQuentel what do you think ?

A. Use localStorage to store the name of the known modules ?

localStorage is synchronous and faster than indexDB.

Using it like :

let known_modules = localStorage.known_modules.split(',');

... would maybe enable quicker checks if a module is present in indexDB ?

Maybe storing also e.g. a hash of the script (e.g. myscript:hash,myscript2:hash2) to know if we have to fetch it in indexDB or to transpile it / or somehow some kind of cache policy ?

Note: maybe we should look at the cost of indexDB vs transcription cost for small Python script - maybe using indexDB is not always worthing it ?

B. JIT transcription

Currently, Brython is doing AOT transcription during the page loading, storing the generated JS into indexDB.

But there is no needs to parse a function/class until its first use/call, even more when it isn't used/called by the script.
Maybe something like :

foo4455 = { $run: function (...args) {
     let fct = transpile(foo4455, "function body code"); // little overcost as we'll need to use `eval()` once the JS is generated.
     foo4455.$run = fct;
     return fct(...args); // first call
}};
foo4455.$infos = {}
//etc.

// to call it in generated JS :
foo4455.$run(); // a little overcost - but could be required to optimize the conditions in `$B.call0()` ? 

The goal would be to reduce transcription/loading time knowing that lot of functions are often not used/called.
This would allow to start first execution sooner, with a small cost on runtime speed, which should be smaller than the total gain in transcription time.

The advantage is that this is adaptive.
If we can detect that one function will be used, we can directly generate the JS code inside run().

C Tree shaking / detection of dead code.

When generating Brython bundle/module, being able to have a static analysis, or run the script once, to detect the functions / parts of codes that aren't called, so that we can choose to remove then (by hand or automatically).

Cordially,

@shark-in-a-hat
Copy link

My $0.02 on idea 'A'- My use case maybe a bit unorthodox, basically I'm making stand-alone programs (games) that are distributed as .html files and also meant to be run locally (locally as in file:/// not http://localhost:8000/ ). Since Chrome (and therefore most of the internet) treat file:/// as one domain for sharing localstorage, the space there is at a premium, there's 2-5MB(?) to share for all games and programs. For example Twine (a popular html game engine) saves living in local storage are known to take up 0.5-1.5MB just for one game.

One script dumping some data into localstorage is probably not a problem, but if that data is not cleared when the page is closed - local storage will fill up pretty quick. Not sure if one can even clear the data when the page is unloaded -I've always been told that one can't reliably catch the 'unload' event (especially on mobile) and 'beforeunload' is it's own can of worms.

For me, this would be a game breaking change (unless it's opt-in, like the indexedDB thing).

@denis-migdal
Copy link
Contributor Author

denis-migdal commented Nov 11, 2023

One script dumping some data into localstorage is probably not a problem, but if that data is not cleared when the page is closed - local storage will fill up pretty quick.

I'm not talking about putting the script into localStorage, but putting the module name + eventually a hash.

If we assume each module name would take in average ~40 characters, then in 1kB, we can store ~25 modules names.
If you have more than 25 modules files that are downloaded, transpiled, then loaded, you might have other issues.

@shark-in-a-hat
Copy link

Oh, my bad.
For some reason, I assumed the transpiled script will end up in localstorage. Still - an opt-in-out option would be nice, since indexedDB is not available on file:/// anyway.

@denis-migdal
Copy link
Contributor Author

For some reason, I assumed the transpiled script will end up in localstorage. Still - an opt-in-out option would be nice, since indexedDB is not available on file:/// anyway.

Using indexDB is the current behavior of Brython.
Maybe it is simply not used when not available ?

@PierreQuentel
Copy link
Contributor

Hi,

Sorry for the late answer, some thoughts here:

A - the indexedDB API is not used to detect if a module is stored in the database, because it is asynchronous. The detection is made from $B.VFS (this is in py_import.js / VFSFinder.find_spec). No point in using localStorage here.

B - it's hard to tell if the balance between a plus in translation time against a minus in execution time is worth the extra complexity. If someone want to experiment it's ok for me.

C - that would be great ! I am starting to play with ESLint, it seems to be able to detect at least a part of such "dead code", for instance names that are declared but never used.

@denis-migdal
Copy link
Contributor Author

A - the indexedDB API is not used to detect if a module is stored in the database, because it is asynchronous. The detection is made from $B.VFS (this is in py_import.js / VFSFinder.find_spec). No point in using localStorage here.

I don't get it.

You do need to store data in a persistent, but dynamic, way in order to detect if a module is stored in the dataset.
How do you achieve that ?

B - it's hard to tell if the balance between a plus in translation time against a minus in execution time is worth the extra complexity. If someone want to experiment it's ok for me.

I guess the first step would be to make some benchmark on translation time.

C - that would be great ! I am starting to play with ESLint, it seems to be able to detect at least a part of such "dead code", for instance names that are declared but never used.

For Python code, I guess using settrace could help identify the lines that are never called, but that'd be a little more harder for JS code. Well this isn't the priority, but the idea is here.

@denis-migdal
Copy link
Contributor Author

For the parser cost :

  • Parsing: 1sec = ~3*10,000 lines.
  • Execution: 1sec = 3*4,500 lines (I removed the print, and added JS code so that it is executable, + the cost of the eval).

Parsing execution time is a little too slow. A big chunk of its cost seems to come from $B.tokenizer.
Of course, when we have a loop and functions called many time, execution will start to become slower and slower. And when having functions that aren't called, then it's the parsing that becomes slower and slower.

I think we'll have some work to do on the parsing.

Maybe we could also provide some good practices for performances :

  • import what you need as late as possible (then the parsing and execution of the imported module will be done later, only when needed, and once the code already started execution).
    • maybe we could somehow provide/implement some kind of lazy import function that'd make the module be fetched/parsed/executed only upon first usage (but how) ? Which could give a huge start up latency decrease if the module is used e.g. inside an event handler function ?

But I guess the priority is first the features, then the execution speed time, then the parsing speed time, and then the documentation/good practices.

Screenshot 2023-11-18 at 12-36-09 Firefox 119 – Linux – 18_11_2023 11 30 56 UTC – Firefox Profiler

Raw data :
Firefox 2023-11-18 12.30 profile.json.gz

    <!DOCTYPE html>
    <html>
        <head>
            <!-- Required meta tags-->
            <meta charset="utf-8">
            <title>X</title>

            <!-- Brython -->
            <script src="https://raw.githack.com/brython-dev/brython/master/www/src/brython.js"></script>
            <!--<script src="https://raw.githack.com/brython-dev/brython/master/www/src/brython_stdlib.js"></script>-->


        </head>
        <body>
        </body>

            <script>
                
                let data = "";
                const pycode = `
def foo(i):
    return i+i

print( foo(3) )
                `;

                let d = Date.now();

                for(let i = 0; i < 1000000; ++i) {
                    data += __BRYTHON__.py2js(pycode, "exec").to_js()
                }

                let e = Date.now();
                document.body.textContent = data.length + ' in ' + (e-d);

            </script>
    </html>

@PierreQuentel
Copy link
Contributor

You do need to store data in a persistent, but dynamic, way in order to detect if a module is stored in the dataset.
How do you achieve that ?

This is explained in section "indexedDB cache for standard library modules" of the page How Brython works. Probably as hard to read as it was hard to write ;-)

@denis-migdal
Copy link
Contributor Author

denis-migdal commented Nov 20, 2023

I found out that JS parser is very very very slow, e.g. using eval is sometimes x300 slower than executing the code directly (because the parser is calling again to parse the content of eval).

Meaning that, very often, it might be better to call a function during runtime instead of generating more JS code during translation. For example :

fct.$infos = {
...
... // LOTS of lines
...
...
};
  1. It is parsed once by JS, and executed only once.
  2. It is a big chunk of code.

Then doing something like :

setFctInfos(fct, "self, i, *arg"); // set the $infos.

Should be waaaay quicker to parse by JS, and some operations are also transferred from Brython parsing time to Brython execution time. Meaning that code will start to execute sooner, and should be overall faster.

I see that Brython is calling eval() at many places. I think we should take a look at each one of them and see if we can remove them.

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

3 participants