kswoll / WootzJs

C# to Javascript Compiler implemented via Roslyn
MIT License
110 stars 21 forks source link

Optional Disabling of Reflection #13

Closed Danielku15 closed 10 years ago

Danielku15 commented 10 years ago

I know you decided to include reflection related code than rather having a small output file

Again, it's clearly better for the output to be smaller than larger, but if for example, we have to decide between reflection and output size, reflection wins. (https://github.com/kswoll/WootzJs/wiki/Comparisons-with-other-C%23-to-JS-Cross-compilers)

But still: If you do not need the Reflection API this is quite a heavy amount of code you do not need. My Project has a huge code base and at this stage, the size of the compilation matters quite a lot. Here a small comparison of the same code compiled using different "2JavaScript" compilers:

Compiler Project JavaScript Core Lib Total
Haxe 526kb 0kb 526kb
Saltarelle Compiler 593kb 107kb 700kb
SharpKit 625kb 0 700kb
WootzJs 2485kb 2874kb 5360kb
WootzJs with $t.$CreateType removed 1348kb 1021kb 2369kb

From a users point of view its just not acceptable to download 5MB just just for a single JavaScript library. The size difference is still quite huge in the "noreflection" version, but one further size improvement is covered in issue #12

[Edit #1] Besire removing the $t.$CreateType method also the delegate initialization can be improved a lot. Including the whole delegate name consumes a huge amount of space in the file:

AlphaTab.Environment().RenderEngines["svg"] = $delegate(this, System.Func$2$(System.Object, AlphaTab.Platform.ICanvas), function(d) {
    return AlphaTab.Platform.Svg.SvgCanvas.prototype.$ctor.$new();
});
AlphaTab.Environment().LayoutEngines["default"] = $delegate(this, System.Func$2$(AlphaTab.Rendering.ScoreRenderer, AlphaTab.Rendering.Layout.ScoreLayout), function(r) {
    return AlphaTab.Rendering.Layout.PageViewLayout.prototype.$ctor.$new(r);
});
AlphaTab.Environment().LayoutEngines["page"] = $delegate(this, System.Func$2$(AlphaTab.Rendering.ScoreRenderer, AlphaTab.Rendering.Layout.ScoreLayout), function(r) {
    return AlphaTab.Rendering.Layout.PageViewLayout.prototype.$ctor.$new(r);
});
AlphaTab.Environment().LayoutEngines["horizontal"] = $delegate(this, System.Func$2$(AlphaTab.Rendering.ScoreRenderer, AlphaTab.Rendering.Layout.ScoreLayout), function(r) {
    return AlphaTab.Rendering.Layout.HorizontalScreenLayout.prototype.$ctor.$new(r);
});
AlphaTab.Environment().StaveFactories["marker"] = $delegate(this, System.Func$2$(AlphaTab.Rendering.Layout.ScoreLayout, AlphaTab.Rendering.BarRendererFactory), function(l) {
    return AlphaTab.Rendering.EffectBarRendererFactory.prototype.$ctor.$new(AlphaTab.Rendering.Effects.MarkerEffectInfo.prototype.$ctor.$new());
});

This can be reduced to the following code if no type information is required:

AlphaTab.Environment().RenderEngines["svg"] = $delegate(this, function(d) {
    return AlphaTab.Platform.Svg.SvgCanvas.prototype.$ctor.$new();
});
AlphaTab.Environment().LayoutEngines["default"] = $delegate(this, function(r) {
    return AlphaTab.Rendering.Layout.PageViewLayout.prototype.$ctor.$new(r);
});
AlphaTab.Environment().LayoutEngines["page"] = $delegate(this, function(r) {
    return AlphaTab.Rendering.Layout.PageViewLayout.prototype.$ctor.$new(r);
});
AlphaTab.Environment().LayoutEngines["horizontal"] = $delegate(this, function(r) {
    return AlphaTab.Rendering.Layout.HorizontalScreenLayout.prototype.$ctor.$new(r);
});
AlphaTab.Environment().StaveFactories["marker"] = $delegate(this, function(l) {
    return AlphaTab.Rendering.EffectBarRendererFactory.prototype.$ctor.$new(AlphaTab.Rendering.Effects.MarkerEffectInfo.prototype.$ctor.$new());
});
kswoll commented 10 years ago

My long-term plans has been to support a solution-wide minification of all the code into maximally shortened identifiers with unnecessary whitespace removed (meaning one run of the WootzJs compiler to produce a single JS representing all the code in your application -- including dependencies like mscorlib). That would reduce the size of the library by at least a factor of 10. I've just procrastinated on that since it would only be used in production scenarios and until now, that hasn't been a priority.

That being said, we can try to optionally remove the reflection code, but I'm concerned it my have unintended consequences in surprising areas. (For example, it's via reflection that the is operator is implemented)

Can you share your thoughts on what you think the best approach would be for your product? If code readability of the released Javascript is important, then your suggestions would have merit. But I suspect we'll have to work through quite a lot of bugs to get to that point, since I've never designed WootzJs with any intention of removing reflection, and in fact rely on it for the implementation of a fair amount of the type system. In contrast, if you just want some sort of minification for production, without any concern for the readability of that Javascript, I think just committing to minification would be more straightforward and robust.

Danielku15 commented 10 years ago

I'll do JavaScript Minification anyway by running uglifyjs afterwards but that currently does not bring much benefit.

I see that you need some reflection info for the is operator. From Haxe I know they just include the base type and interfaces as for type checking purposes: https://github.com/CoderLine/alphaTab/blob/master/bin/js/lib/alphaTab/alphaTab.core.js#L9284

Such a solution would keep the operator working while removing unneeded reflection data. I think the internal type system does not need any field, property and method information. The "noreflection" version could skip generating the member information while keeping the base type information.

Maybe it would be a good solution to compile the reflection information that's not needed by the type system into a separate JavaScript file. "MyAssembly.js" (contains type information required by type system) "MyAssembly.reflection.js" contains additional "opt-in" data for the reflection API. If the reflection file is included, the data is available, otherwise you'll only receive empty data from the reflection API.

In my opinion if an application uses reflection a lot, there's something wrong with the application design. The next question is: What C# application that will be compiled from C# to JavaScript will use the full reflection data. My application/library is quite huge for being compiled from C# to JavaScript and doesn't use reflection and only some minor type checking in the platform layer.

kswoll commented 10 years ago

Also, another factor is that I'm baking in all the cultures provided by .NET at compile time. This is a pretty big payload, and even after minification would be approximately 300k in size. I can add an option for that feature as not including it only impacts having an extensive set of culture information for your runtime.

kswoll commented 10 years ago

I'll see what impact not including reflection has -- what tests fail, and will consider alternate options for language-level features.

I'd politely disagree that an app that uses reflection a lot is intrinsically flawed -- I think expression trees are a useful mechanism -- especially for referencing properties at compile-time in a type-safe way (rather than using strings) -- and they depend in great detail on the underlying reflection API in order to function.

That being said, I am sympathetic to your concerns of adding such a significant payload cost for a feature you avoid. So after seeing what happens with removing reflection en masse, I'll let you know what I find. (It's a bit tricky, because even my unit test runner uses reflection to get all the test fixtures)

kswoll commented 10 years ago

I've addressed these concerns mostly as suggested. You can customize the assembly-level configuration of these properties via JsCompilerOptionsAttribute which surfaces the following properties:

You should probably consider these features somewhat experimental, but so far all the unit tests pass (after I modified some of them a bit to not depend on reflection info for mscorlib) with all of these options enabled for mscorlib. For your own testing purposes, please see AssemblyInfo in WootzJs.Runtime where you can see some examples commented out:

//[assembly: JsCompilerOptions(AreAutoPropertiesMinimized = true)] //[assembly: JsCompilerOptions(AreDelegatesMinimized = true)] //[assembly: JsCompilerOptions(IsReflectionMinimized = true, IsCultureInfoExportDisabled = true)]

Feel free to configure this as you desire. It would probably be ideal if you could configure this without modifying the AssemblyInfo, but for the time being, it would be helpful if you could try this out and give me some feedback.

kswoll commented 10 years ago

Also, support for per-class and per-property auto-property minimization is configurable via JsAttribute.AreAutoPropertiesMinimized by applying this attribute to classes and properties respectively.

Danielku15 commented 10 years ago

Awesome. I'll look into the improved generation soon (probably in the weekend). I'm sure this will help minimizing the footprint of my library.

kswoll commented 10 years ago

@Danielku15 just a heads up, I've converted over to using the latest version of Microsoft.CodeAnalysis (Roslyn) and it has new dependencies that weren't required before. Thus you'll need to download (and install) Microsoft Build Tools 2015 Preview or you'll get weird runtime errors complaining about not finding Microsoft.Build. Hopefully that's the only thing you have to do, but obviously let me know if you run into any issues.

If for some reason you encounter new errors that are blocking you, you can try checking out the "oldroslyn2" branch which is the last version of the codebase that targeted the old version.

Danielku15 commented 10 years ago

Thanks for the Info. I've already discovered this dependency when I tried to use Roslyn for an other project.