emscripten-core / emscripten

Emscripten: An LLVM-to-WebAssembly Compiler
Other
25.37k stars 3.25k forks source link

unexpected optimizer effects - need API to save FS nodes that is closure-safe #2800

Closed wothke closed 4 years ago

wothke commented 9 years ago

Encouraged by the results from other projects I just used some additional optimizations for my existing page here: http://www.wothke.ch/webuade/

I had used -O3 to create the library used in in this page and while it runs nicely it as a rather big footprint with 2.5mb. Using additional options: "-Os -O3 --closure 1 --llvm-lto 1" brought the size indeed down to 1.7mb (which is great). However I had some funny effects (for all of which I meanwhile found some workaround) and I am wondering if these all are intended and if maybe I just did not search hard enough for some existing documentation:

1) originally I had been using calls to: FS.createPath(), FS.createDataFile() and Pointer_stringify (etc). These APIs were all discarded by the optimizer. Luckily the functionality was still there I just had to call it using: Module.FS_createPath(), Module.FS_createDataFile() and Module.Pointer_stringify().

2) somewhat "inconsistently" I still have to use "Pointer_stringify()" within the code that I am including with --js-library.

3) within my JavaScript code I am caching the files that I previously created using Module.FS_createDataFile() and at some point I need to access the size of the respective files. Since I did not find any API that seemed intended for this purpose, I just accessed the "contents.length" of the respective "DataFile". Unfortunately "contents" turns into "u" once the optimizer is through with it... (is there some "stable" API that I overlooked?)

4) within the JavaScript code that I am including using --js-library: mergeInto(LibraryManager.library, { uade_notify_song_update: function(i, min, max, curr) { var infoText= Pointer_stringify(i);
... I had been using some utility functions that I had defined in my HTML code... again the optimizer obsuscated the respective names (i.e. it invented names for functions that were not actually part of the compiled code) such that the still existing functions could no longer be located. I therefore tried to include the respective util functions directly within the library. Whatever I tried I did not succeed to define the utils such that they would still be around a runtime.. I tried to add namespaces (as in the examples) but then the C code would no longer find my functions (etc)... finally I gave up and defined the utils in a separate file which I am now including using --pre-js (which seems to work). I think some extra documentation would be useful here..

kripken commented 9 years ago

This is all due to closure compiler, it eliminates normal names. Then you must use methods on Module, if accessed outside of the code that closure saw (if closure sees you call something, it can minify the call as well).

I don't understand 2 or 4. Got a testcase?

For 3, we might need to add an API. How did you cache the files? Are you caching internal FS objects?

wothke commented 9 years ago

regarding 2: within the file that I am including using --js-library I still using "Pointer_stringify" - and it works- whereas "everywhere else" this would not work and I have to use "Module.Pointer_stringify".. from a "user's" perspective this is just not very intuitive..

regarding 3: yes, I am caching the FS objects that I am constructing, e.g. from the binary data that I get via drag&drop:

        var filename="songs/"+file.name;
        var filenameFull= this.basePath+"/"+filename;
        var reader = new FileReader();
        reader.onload = function() {        
            var d= new Uint8Array(reader.result);   
            var f= FS.createDataFile("/", filenameFull, d, true, true);
            this.binaryFileMap[filenameFull]= f;
            this.setupSongData(filename);           
        }.bind(this);
        reader.readAsArrayBuffer(file);

regarding 4: example.. first I had tried to just add a "startsWith" function within my library.. and just use it within my library's "uade_notify_song_update" function (see below).. when that failed I tried to use an additional namespace around the functions (e.g. "$UADE: { ..}") .. but in any case I did not manage to get it syntactically correct so that it would have worked..

mergeInto(LibraryManager.library, {
    startsWith: function(a, str) {
        return (a.match("^"+str)==str)
    },
    uade_notify_song_update: function(i, min, max, curr) {
        var infoText= Pointer_stringify(i); 
        ...
        if (startsWith(line, "File name:")) {
         ...
         },
}
kripken commented 9 years ago

The issue is that a js library is optimized together with the code, so Module. is not needed. But, you can also use Module. to be safe. We can't solve that any better, this is just part of the complexity of using closure compiler (if it's not absolutely necessary, best to not use it).

Regarding caching FS library objects, that is dangerous, as they are minified, and if the build loading them is different, it might be minified differently. We might need to add a new API to generate a non-minified version of an FS object.

wothke commented 9 years ago

Thanks, 2) is no problem and I am happily accepting the inconvenience for the 30% size reduction that I just saw on some of my other projects.

Regarding the FS stuff it would indeed be nice to have an API so that I stop using instable internals. (Since I have already loaded the binary file data before I start using the FS stuff I could also directly cache that data as a workaround... )

Do you have any hints regarding 4?

kripken commented 9 years ago

Maybe it's the lack of formatting, but that's hard to read (use 4 backticks to escape it). Maybe create a gist of a full testcase?

wothke commented 9 years ago

thx for the tip :-) I fixed the formatting in the above entry: My C code calls the 'uade_notify_song_update' function which is defined in the respective lib (works fine). I tried to define a utility function 'startsWith' - which is only used here - directly within the same lib (so that the stuff is neatly in one place).. the problem is that whatever I tried here, at runtime the function 'startsWith' (or whatever the closure complier thought it should be called) was not found..

kripken commented 9 years ago

Look at SDL (library_sdl.js) for an example. You need something like

  $Namespace: {
    startsWith: function() { .. }
  },
  method__deps: ['$Namespace'],
  method: function() { Namespace.startsWith() }
wothke commented 9 years ago

Thx! From my perspective we can close this issue.. or do you want to keep it open as a reminder for the FS question?

kripken commented 9 years ago

Yeah, let's keep it open for that. Renamed.

wothke commented 9 years ago

Thanks again.. keep up the good work :-)

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because there has been no activity in the past 2 years. It will be closed automatically if no further activity occurs in the next 7 days. Feel free to re-open at any time if this issue is still relevant.