-
Notifications
You must be signed in to change notification settings - Fork 3.3k
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
Use JavaScript for code elimitation instead of templates #5417
Comments
I like this idea. It would take slightly more work, but not much: we don't run closure by default, because it's fairly slow, but what we could do is add code to the js optimizer (in the "shell cleanup" pass, probably) to match that pattern (i.e., if an if on a condition, where the condition is a global const). So basically our js cleanup code would be guaranteed to eliminate that code. This would make -O0 builds larger (since they don't run closure or the js optimizer), but I think that's a reasonable tradeoff. Thoughts? |
To be honest, I'm not sure it is worth investing an effort to write any code elimination in-house. It will be potentially error-prone and besides that in most projects developers will either use Closure Compiler option on build time or will add some post-processing to the file afterwards. I'd just give those tools a chance to do what they do well (code minification) and focus Emscripten on code compilation part. In other words, use cases where code elimination is desired and no other tool is run on top of the JavaScript build are fairly limited. |
How would you handle library_async.js's Edit: I guess it would be possible to move the conditionals inside the functions, but what about the
Edit: after thinking more, I'm pretty sure it would be smart enough, as fairly arbitrary code is allowed. |
@curiousdannii, I do not entirely understand your concerns. If there are multiple parts of object literal then yes, we'll need to split it into pieces if we want to use plain JavaScript, nothing special about it. Mentioned minifiers operate with AST (as far as I remember) and ternary operator for them is no more difficult than regular if/else. You can play with Closure Compiler here: https://closure-compiler.appspot.com/ (do not forget to wrap variables into self-calling function so that they will not be in global scope. |
@nazar-pc This has nothing to do with minification. The .js library files are processed by emcc, not output directly, and so you need to be careful that the conditional preprocessing still works correctly. I think it probably will work fine. But I think it will make it very messy. Perhaps the best option would be to allow for both: use JS constant if statements for most things, but allow preprocessor directives when it would otherwise be too messy. |
I think we can start with small things first and see how it goes. If it gets too messy with JS constants I have nothing against using more appropriate tools. The whole point of proposal is to make things easier to work with, not harder. |
@curiousdannii, I think you're right, this idea could work well for complete lines of code (it's just adding a condition around that code) but not for object literals and perhaps other things (like a fragment of a line of code). So I don't think it's practical to expect to be able to avoid proprocessing entirely. |
…y(), emscripten_enter_soft_fullscreen() and emscripten_exit_soft_fullscreen() which allow a more intelligent switch between fullscreen modes with different preset display modes. The original emscripten_request_fullscreen() is still available, and should be used in scenarios where it's desired that the html5.h layer should perform no changes whatsoever to the DOM tree.
This does not look like a good direction. It is not ideal to rely on dynamic code analysis to optimize something that is already optimizable statically. This would necessitate using Closure to get to good lean sized result, and there are developers who don't want to use Closure due to not having to deal with Java (as it stands right now), or not having to deal with defining minified-nonminifed boundaries for inferfacing with external code, or not wanting to add an extra costly compilation step, or because they do want to support asm.js, or due to wanting to use some other minifier, which might not be as sophisticated to do this particular optimization. I don't agree that It should also be noted that this applies to a fairly internally developed set of runtime, users can go whichever way they choose in their own library_xx.js files. |
First of all, static optimizations require an effort to do them and as we can see, modern Closure Compiler shows that not everything is actually statically optimized even on the very high level. I do not see why should we spend any time on manually optimizing something that can be completely automated and gain way better results than it is possible to achieve manually with reasonable time spent on it. Java use is important, hopefully in nearest months it will be possible to switch to JavaScript version of Closure Compiler, so this concern will be always valid. However, emsdk includes it out of the box, so it is not that big of a deal. Also I'd go as far as saying that Closure Compiler should be enabled by default and non-optimized builds should only be used by Emscripten developers for debugging. This will require to eliminate all of the
No, it comes down to tooling. Regular JavaScript can be analyzed by lots of existing tools, preventing multitude of typos and other kinds of errors. While currently powerful IDE like WebStorm struggles to provide basic syntax highlighting when using |
I'm sympathetic to the goal of making the code more tooling-friendly. However one problem with relying on optimization (as opposed to making it explicit) is that it's much less clear exactly what is happening and where (i.e. which optimizations are being applied) and easy to lose the optimizations without noticing (which in this case would result in bloat in the output). If we replace the existing ifdef-based way of excluding unused library code, we should have some testing to ensure we don't regress. Also if we do this now, we'd basically make Java a hard dependency for emscripten users and I'm not sure I want to do that. |
I guess one other argument against requiring closure is that it reduces debugability for unoptimized builds (i.e. you either don't run closure, in which case you get all the syscalls you don't need, or you run it anyway and obfuscate everything). Can it be run in such a way that it only does the dead-code elimination that we want? |
Think about it as about C/C++ code compilation: how often do you try to beat compiler with manual optimizations? My guess would be not that often, especially considering that compiler often does a better job at optimizations.
Now I think 3 is particularly interesting option to consider. |
The UglifyJS devs are... less than ideal. They refuse to make a changelog. I don't think it would be a safe dependency for a project like this. Babel-minify is good, but could be considerably slower. I haven't tested though. It also changes asm.js code so you couldn't just apply to the whole output. Maybe Closure is the best option. I just don't use it because I don't like ADVANCED_OPTIMIZATIONS. It makes writing custom library files, or using pre/post-js, trickier. Maybe SIMPLE_OPTIMIZATIONS could be the default instead (with an option to switch to ADVANCED_OPTIMIZATIONS)? How much bigger would that make the files?
Are you saying that you want DCE but not other minification? Maybe babel-plugin-minify-dead-code-elimination would be an option in that case. |
No way to tell without testing.
Not necessarily want, but I think it is an option to consider just so that we don't force everyone to use Closure Compiler in ADVANCED mode and don't need to manually optimize everything either. We'll need to explore available tools if it sounds like a good idea. |
This issue has been automatically marked as stale because there has been no activity in the past year. 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. |
The mix of plain JavaScript with templates breaks analysis and autocompletion in IDE and is generally hard to read and maintain.
Assuming Google Closure Compiler is always used for production builds (don't see the reason why it shouldn't) or any other relatively advanced tool like UglifyJS is used, we can put all of the code elimination work on those tools.
Let's turn this:
Into this:
Where
FETCH_DEBUG
andUSE_PTHREADS
definition can be prepended to existing JavaScript code.When any of these variables are defined upfront and never change in runtime, Google Closure Compiler should be able to remove unnecessary branches entirely and open branches that should be kept.
Google Closure Compiler already does a very good job of deleting unused code, inlining short functions and so on, why not delegate the rest of work to it?
I think this might greatly improve readability and maintainability of the JavaScript part of this project.
The text was updated successfully, but these errors were encountered: