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

Incorrect treatment of constants [was: Inlining interferes with minification] #16600

Closed
stevemessick opened this issue Feb 6, 2014 · 6 comments
Assignees
Labels
dart2js-optimization type-enhancement A request for a change that isn't a bug web-dart2js

Comments

@stevemessick
Copy link
Contributor

Dart2Js should not inline double.MAX_FINITE as it add over 300 bytes with every inline...

179769313486231570814527423731704356798070567525844996598917476803157260780028538760589558632766878171540458953514382464234321326889464182768467546703537516986049910576551282076245490090389328944075868508455133942304583236903222948165808559332123348274797826204144723168738177180919299881250404026184124858368

I use that constant 4 time in my program, I could save almost 1kb by not inlining it (but my program would still be over 1mb, but I hope you remove the mirrors usage from polymer, looks like nearly all my classes are inlcuded in mirrors because they are somehow connected over private variables to my polymer element)
////////////////////////////////////////////////////////////////////////////////////
Editor: 1.2.0.dev_02_04 (2014-01-31)
OS: Windows 8 - amd64 (6.2)
JVM: 1.7.0_21

projects: 1

open dart files: 2

auto-run pub: true
localhost resolves to: 127.0.0.1
mem max/total/free: 1778 / 603 / 29 MB
thread count: 31
index: 723814 relationships in 140305 keys in 15993 sources

SDK installed: true
Dartium installed: true

@floitschG
Copy link
Contributor

We intentionally decided to inline even bigger constants, because zip is really good at compressing them.
We will run more tests to make sure that this assumption still holds.


Added Optimization label.

@rakudrama
Copy link
Member

The literal will be interpreted by JavaScript as a double, so only ~17 digit are needed to get the same bit pattern.

There is something more serious going on here, which causes erroneous constant folding in interpolation:

main() {
  print('${double.MAX_FINITE}');
  print('${[double.MAX_FINITE]}');
}

--->
  main: function() {
    P.print("179769313486231570814527423731704356798070567525844996598917476803157260780028538760589558632766878171540458953514382464234321326889464182768467546703537516986049910576551282076245490090389328944075868508455133942304583236903222948165808559332123348274797826204144723168738177180919299881250404026184124858368");
    P.print(H.S([179769313486231570814527423731704356798070567525844996598917476803157260780028538760589558632766878171540458953514382464234321326889464182768467546703537516986049910576551282076245490090389328944075868508455133942304583236903222948165808559332123348274797826204144723168738177180919299881250404026184124858368]));
  }

===>
179769313486231570814527423731704356798070567525844996598917476803157260780028538760589558632766878171540458953514382464234321326889464182768467546703537516986049910576551282076245490090389328944075868508455133942304583236903222948165808559332123348274797826204144723168738177180919299881250404026184124858368
[1.7976931348623157e+308]

The HConstant has type [subclass=JSPositiveInt], which seems wrong.
I guess the value does satisfy x == Math.floor(x)
But the form of the literal is clearly double:

    static const double MAX_FINITE = 1.7976931348623157e+308;

Another example:

   print('${1000000 * 1000000 * 1000000 * 1000000}');

--->
    P.print("999999999999999983222784");

===>
999999999999999983222784


Removed Type-Enhancement, Priority-Unassigned labels.
Added Type-Defect, Priority-High labels.
Changed the title to: "Incorrect teat[Inlining interferes with minification]".

@rakudrama
Copy link
Member

Changed the title to: "Incorrect treatment of constants [was: Inlining interferes with minification]".

@floitschG
Copy link
Contributor

I agree that we should use exponential representations for integers that are too big.

I disagree with your assessment that this is a defect. The form of the literal doesn't have any influence on its value and for dart2js we just say that x == Math.floor(x) implies that x is an integer. After all we also say that 1.0 is an integer.


Removed Type-Defect, Priority-High labels.
Added Type-Enhancement, Priority-Medium labels.

@rakudrama
Copy link
Member

I split out the constant-folding issue into issue #18103

@floitschG
Copy link
Contributor

Fixed in r43913.


Added Fixed label.

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dart2js-optimization type-enhancement A request for a change that isn't a bug web-dart2js
Projects
None yet
Development

No branches or pull requests

4 participants