node-chakracore icon indicating copy to clipboard operation
node-chakracore copied to clipboard

Stylus tests failing in Node-ChakraCore

Open digitalinfinity opened this issue 7 years ago • 6 comments

  • Version: 10.0.0-nightly
  • Platform: all
  • Subsystem: chakracore

Repro steps (attempted on Linux):

  • Get latest node-ChakraCore
  • git clone https://github.com/stylus/stylus
  • cd stylus
  • npm install
  • npm test

Result: 26 pass, 1 fail

  1) integration
       bifs floats:

      AssertionError: expected 'body {\n  foo: 0rem;\n  foo: 0.1rem;\n  foo: 0.2rem;\n  foo: 0.3rem;\n  foo: 0.4rem;\n  foo: 0.5rem;\n  foo: 0.6rem;\n  foo: 0.7rem;\n  foo: 0.8rem;\n  foo: 0.9rem;\n  foo: 1rem;\n  foo: 1.1rem;\n  foo: 1.2rem;\n  foo: 1.3rem;\n  foo: 1.4rem;\n  foo: 1.5rem;\n  foo: 1.6rem;\n  foo: 1.7rem;\n  foo: 1.8rem;\n  foo: 1.9rem;\n  foo: 2rem;\n  foo: 50%;\n  foo: 33.333333333333336%;\n  foo: 25%;\n  foo: 20%;\n  foo: 16.666666666666668%;\n  foo: 14.285714285714286%;\n  foo: 12.5%;\n  foo: 11.11111111111111%;\n  foo: 10%;\n  foo: 9.090909090909091%;\n  foo: 8.333333333333334%;\n  foo: 1;\n  foo: 1;\n  foo: 1;\n}' to be 'body {\n  foo: 0rem;\n  foo: 0.1rem;\n  foo: 0.2rem;\n  foo: 0.3rem;\n  foo: 0.4rem;\n  foo: 0.5rem;\n  foo: 0.6rem;\n  foo: 0.7rem;\n  foo: 0.8rem;\n  foo: 0.9rem;\n  foo: 1rem;\n  foo: 1.1rem;\n  foo: 1.2rem;\n  foo: 1.3rem;\n  foo: 1.4rem;\n  foo: 1.5rem;\n  foo: 1.6rem;\n  foo: 1.7rem;\n  foo: 1.8rem;\n  foo: 1.9rem;\n  foo: 2rem;\n  foo: 50%;\n  foo: 33.333333333333336%;\n  foo: 25%;\n  foo: 20%;\n  foo: 16.666666666666668%;\n  foo: 14.285714285714286%;\n  foo: 12.5%;\n  foo: 11.11111111111111%;\n  foo: 10%;\n  foo: 9.090909090909092%;\n  foo: 8.333333333333334%;\n  foo: 1;\n  foo: 1;\n  foo: 1;\n}'
      + expected - actual

         foo: 14.285714285714286%;
         foo: 12.5%;
         foo: 11.11111111111111%;
         foo: 10%;
      -  foo: 9.090909090909091%;
      +  foo: 9.090909090909092%;
         foo: 8.333333333333334%;
         foo: 1;
         foo: 1;
         foo: 1;

digitalinfinity avatar Mar 13 '18 19:03 digitalinfinity

I believe this is the same as the chakracore issue https://github.com/Microsoft/ChakraCore/issues/2512

MSLaguana avatar Mar 13 '18 21:03 MSLaguana

Isolated repro: (100.0/11).toString(10);

They expect output: 9.090909090909092 We output: 9.090909090909091

Debugger shows all three numbers as 9.0909090909090917165 a2e8ba2f 40222e8b 00101111 10111010 11101000 10100010 2f ba e8 a2 10001011 00101110 00100010 01000000 8b 2e 22 40

Note: we have separate code path for decimal conversion.

IrinaYatsenko avatar Mar 22 '18 19:03 IrinaYatsenko

FDblToRgbPrecise produces 9.090909090909092 representation. FDblToRgbFast (which we end up using) produces 9.090909090909091 representation.

FDblToRgbFast hasn't changed since 2006 (other than handling errors and moving helpers into namespace).

Hiding FDblToRgbPrecise behind FDblToRgbFast goes as far back as 2002 (without any interesting comments on why it's this way).

Did a mini-benchmark: in x86 Release build FDblToRgbPrecise is about 2.8 times slower than FDblToRgbFast (a loop over 10^6 doubles: 733949944 & 263994008 nanoseconds).

IrinaYatsenko avatar Mar 26 '18 21:03 IrinaYatsenko

Replacing FDblToRgbFast with FDblToRgbPrecise regressed the following benchmarks: Kraken: Json-stringify-tinderbox (19%) Octane: Splay (14%), Splay latency (6%) SunSpider: String-tagcloud (5-17%)

Discussed with Louis and he thinks regressing perf of JSON.stringify like this isn't acceptable: JSON.stringify({ 'pi': 3.14 }) calls ToString and it's not inconceivable to have real life scenarios with json describing objects with many floating point properties.

IrinaYatsenko avatar Mar 27 '18 21:03 IrinaYatsenko

JSON I can see- why does splay regress with that change? I thought Splay was memory intensive. Is the String-tagcloud case also using JSON stringify in the hot path?

digitalinfinity avatar Mar 28 '18 01:03 digitalinfinity

Splay test generates a new random number and then converts it to string in a loop (see InsertNewNode()). Ends up calling ToString 10K+ times.

String-tagcloud creates "a href" tag using popularity numbres, which are converted to string. Calls ToString 2.5K+ times.

IrinaYatsenko avatar Mar 28 '18 16:03 IrinaYatsenko