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

JS-interop example produces > 670kb .js file #11218

Closed
sethladd opened this issue Jun 11, 2013 · 19 comments
Assignees
Labels
Milestone

Comments

@sethladd
Copy link
Member

@sethladd sethladd commented Jun 11, 2013

This example:

import 'dart:html';
import 'package:js/js.dart' as js;

void main() {
  var context = js.context;
  js.scoped(() {
    var hug = new js.Proxy(context.Hug);
    var result = hug.embrace(10);
    query('#output').text = result;
  });
}

Calling this JS file:

function Hug(strength) {
  this.strength = strength;
}

Hug.prototype.embrace = function(length) {
  return 'thanks, that was a good hug for ' + length + ' minutes!';
}

Hug.prototype.patBack = function(onDone) {
  onDone('all done');
}

produces a 676kb .js file.

I am using Dart Editor version 0.5.16_r23799

Note: minifying the output code breaks Dart-JS interop.

@sethladd

This comment has been minimized.

Copy link
Member Author

@sethladd sethladd commented Jun 11, 2013

Attaching the sample app.


Attachment:
dart_js_interop_example.zip (192.08 KB)

@sethladd

This comment has been minimized.

Copy link
Member Author

@sethladd sethladd commented Jun 11, 2013

@peter-ahe-google

This comment has been minimized.

Copy link
Contributor

@peter-ahe-google peter-ahe-google commented Jun 11, 2013

I assume the problem is that js-interop imports 'dart:mirrors', and that it only uses InstanceMirror.delegate.

Vijay, can you confirm this? If so, I think I know how to fix it.


Set owner to @vsmenon.
Added Waiting label.

@sethladd

This comment has been minimized.

Copy link
Member Author

@sethladd sethladd commented Jun 11, 2013

FWIW the version of the code w/out the js-interop bits compiles to 95k (unminified).

@sethladd

This comment has been minimized.

Copy link
Member Author

@sethladd sethladd commented Jun 11, 2013

And, adding the following lines increases size to 686k:

    hug.patBack(new js.Callback.once((msg) {
      query('#output').appendText(' This just in: $msg');
    }));

@sethladd

This comment has been minimized.

Copy link
Member Author

@sethladd sethladd commented Jun 11, 2013

Removed Type-Enhancement label.
Added Type-Bug label.

@vsmenon

This comment has been minimized.

Copy link
Member

@vsmenon vsmenon commented Jun 11, 2013

Yes, it imports dart:mirrors. I believe the only usage is MirrorSystem.getName.

@peter-ahe-google

This comment has been minimized.

Copy link
Contributor

@peter-ahe-google peter-ahe-google commented Jun 11, 2013

Vijay: I'll add an annotation to dart:mirrors called something like: @­EnableTreeShaking. You'll have to use this annotation like this:

@EnableTreeShaking
import 'dart:mirrors';

Does that sound doable?


Set owner to @peter-ahe-google.
Added Accepted label.

@peter-ahe-google

This comment has been minimized.

Copy link
Contributor

@peter-ahe-google peter-ahe-google commented Jun 11, 2013

Added this to the M6 milestone.
Removed Priority-Unassigned label.
Added Priority-High label.

@peter-ahe-google

This comment has been minimized.

Copy link
Contributor

@peter-ahe-google peter-ahe-google commented Jun 11, 2013

Removed Type-Bug label.
Added Type-Defect label.

@sethladd

This comment has been minimized.

Copy link
Member Author

@sethladd sethladd commented Jun 11, 2013

Thanks for taking a look!

@peter-ahe-google

This comment has been minimized.

Copy link
Contributor

@peter-ahe-google peter-ahe-google commented Jun 11, 2013

Actually, since you only use MirrorSystem.getName, I should be able to detect that and preserve tree-shaking transparently.

@peter-ahe-google

This comment has been minimized.

Copy link
Contributor

@peter-ahe-google peter-ahe-google commented Jun 12, 2013

Oops: I was planning on fixing the problem that because JS-interop imports 'dart:mirrors' tree shaking is disabled. However, Seth hadn't even noticed that. When I compile the example today, the size is 2,6M and it takes almost 12 seconds to compile.

I can fix that regression and bring the size back down to 656K and compile time down to 5 seconds.

I have one further trick in my bag, but that will only get us down to 587K. With minification, it looks like I might be able to bring it down to 323K (this number includes a mapping from minified names to unminified names, so with a little more programming, I should be able to get JS-interop working in minified code).

I don't know if 323K is small enough. If not, it is great that Vijay is working on a leaner alternative to JS-interop.

@peter-ahe-google

This comment has been minimized.

Copy link
Contributor

@peter-ahe-google peter-ahe-google commented Jun 12, 2013

https://codereview.chromium.org/16817002/ provides the first part I described in #­13 (getting the size down to 656K).

@peter-ahe-google

This comment has been minimized.

Copy link
Contributor

@peter-ahe-google peter-ahe-google commented Jun 12, 2013

I plan these further actions on this bug:

  1. Restore global type inference when importing dart:mirrors, but not using problematic features (should bring the size down to 587K).
  2. Get JS-interop working with minification. This should bring the size down to 323K (79K gzipped).
  3. Close this bug as fixed.

Please let me know if you have any concerns about this plan.

@peter-ahe-google

This comment has been minimized.

Copy link
Contributor

@peter-ahe-google peter-ahe-google commented Jun 12, 2013

Added Started label.

@peter-ahe-google

This comment has been minimized.

Copy link
Contributor

@peter-ahe-google peter-ahe-google commented Jun 12, 2013

https://codereview.chromium.org/16830002/ restores global type inference (bringing the size down to 587K).

@peter-ahe-google

This comment has been minimized.

Copy link
Contributor

@peter-ahe-google peter-ahe-google commented Jun 12, 2013

I have a working prototype implementation that seems to work with dart_js_interop_example.zip in minified code. Currently, the size is 327K (80K gzipped), but I should be able shave off a few more Ks.

@peter-ahe-google

This comment has been minimized.

Copy link
Contributor

@peter-ahe-google peter-ahe-google commented Jun 12, 2013

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.