Compile Twitter Bootstrap 4

connorskees commented 4 years ago

A 1.0 release of grass should be able to compile bootstrap identically to dart-sass. The features currently blocking Bootstrap from even compiling are

[x] @extend
[x] @keyframes
[x] @supports
[x] map as value in map
[x] strange issue involving color functions and precision (this turned out to be due to a lack of rounding in the output of builtin functions red, green, and blue)
[x] short circuiting of and operator
[x] short circuiting of or operator
[x] control flow inside style inside control flow
[x] better handling of @import
[x] self referential default function arguments

The places where grass and dart-sass differ in output are

[x] attribute quoting is incorrect (e.g. output should be ["foo"] => [foo], but we retain quotes). dart-sass has complex rules for determining whether quotes should be emitted or not
[x] newlines after multiline comments are retained for some reason in certain conditions. The newlines should be stripped.
[x] extended selectors should be wrapped in Rc<RefCell<Selector>>
[x] all sorts of media query features
- [x] variables are evaluated when inside parens
- [x] validation
- [x] we emit too many linebreaks inside @media
- [x] complex rules for emitting newlines around @media
[x] line breaks after comments in selectors are not retained
[x] division with / should only occur in special cases

MartinKavik commented 4 years ago

Is possible that these improvements allow to compile also Bulma .scss files?

pickfire commented 4 years ago

@MartinKavik @connorskees Is still working on @extend which bulma uses.

From what I see from the source code, scss is supported too.

connorskees commented 4 years ago

@MartinKavik @pickfire yes that's correct, Bulma is blocked largely on @extend. Bulma also appears to use @keyframes in 1 place, which is trivially possible to implement right now, but would need to be completely rewritten in the course of implementing @extend, so I am holding off on implementing it at the moment.

A possibility could be silently eating @extend (maybe print a warning that it was eaten) rather than panicking with todo!(). I'll test doing this tomorrow and and release it to crates.io if it works well.

At the moment, only scss is supported. When the indented syntax, sass, is implemented, it will only ever be a second class citizen.

MartinKavik commented 4 years ago

Thanks for answer. I will be updating Seed template for new apps in next weeks so I'm just exploring options - there can be Bulma, Bootstrap or nothing and I can change it in the future - don't hurry, great job!

pickfire commented 4 years ago

@connorskees Thanks for the work on this. By the way, are you planning to add a benchmark with the official dart implementation later on?

connorskees commented 4 years ago

@pickfire Is this in reference to https://github.com/connorskees/grass/issues/5? Yes, ideally we would benchmark against dart-sass, rsass, node-sass, and sassc.

I've been doing very rough, unscientific comparisons to rsass and dart-sass using hyperfine during development. grass seems to be very fast on some inputs, despite me deliberately ignoring performance at this stage. That is to say, I've no idea why it is fast.

I've benchmarked against the test case found here generated just using the python repl

with open("input.scss", "w") as f:
    f.write(".foo {a: b}" * 2**17)

These were all performed immediately after each other just now on my Windows 10 machine with Intel i5-8250U CPU @ 1.60GHz, 1800 Mhz. They are not scientific and there is only 1 run, but I find they are illustrative of the performance I've been seeing. (NUL on Windows is analogous to /dev/null)

hyperfine "sass.bat input.scss>NUL" --warmup 5
Benchmark #1: sass.bat input.scss>NUL
  Time (mean ± σ):      3.194 s ±  0.373 s    [User: 9.1 ms, System: 8.6 ms]
  Range (min … max):    2.627 s …  3.658 s    10 runs

hyperfine "grass input.scss>NUL" --warmup 5
Benchmark #1: grass input.scss>NUL
  Time (mean ± σ):     908.3 ms ±  35.4 ms    [User: 1.3 ms, System: 8.2 ms]
  Range (min … max):   862.9 ms … 989.6 ms    10 runs

hyperfine "rsass input.scss>NUL" --warmup 5
Benchmark #1: rsass input.scss>NUL
  Time (mean ± σ):      2.923 s ±  0.037 s    [User: 4.0 ms, System: 5.2 ms]
  Range (min … max):    2.877 s …  2.993 s    10 runs

Version info for reference

sass.bat --version
1.26.3

grass --version
grass 0.8.1

rsass --version
rsass 0.12.3-PRE

I would like to be clear that at this stage performance is not a focus and grass will sacrifice performance in order to better conform to the reference implementation. It also may very well be that grass is faster here simply because of an unimplemented feature -- @extend should push the time to well above a second for that input, though it should be noted that rsass does not yet implement @extend either.

I should also note that this benchmark is misleading; rsass does beat grass with regard to control flow and floats. Though I believe the disparity can in part be attributed to the fact that grass treats all numbers as bigints, while rsass uses usize to represent them (rsass does not have feature-complete number parsing as a result of this).

For an input of

@for $i from 0 to 99999 {
  a {
    color: $i;
  }
}

rsass takes the lead,

hyperfine "sass.bat input.scss>NUL" --warmup 5
Benchmark #1: sass.bat input.scss>NUL
  Time (mean ± σ):      1.744 s ±  0.045 s    [User: 1.2 ms, System: 9.2 ms]
  Range (min … max):    1.683 s …  1.823 s    10 runs

hyperfine "grass input.scss>NUL" --warmup 5
Benchmark #1: grass input.scss>NUL
  Time (mean ± σ):      1.038 s ±  0.122 s    [User: 3.8 ms, System: 4.1 ms]
  Range (min … max):    0.918 s …  1.288 s    10 runs

hyperfine "rsass input.scss>NUL" --warmup 5
Benchmark #1: rsass input.scss>NUL
  Time (mean ± σ):     843.2 ms ±  18.9 ms    [User: 4.2 ms, System: 6.5 ms]
  Range (min … max):   811.9 ms … 865.6 ms    10 runs

When this library becomes more mature, I do intend to add performance to CI, benchmark with criterion, and properly profile+optimize. For right now, I would say as a rule that grass is not the fastest.

connorskees commented 4 years ago

@pickfire as an update to this, I have finally gotten the chance to implement smallint optimization!

This has been something I've wanted to implement for a very long time, and the improvements from it are very clear. I've done some benchmarking with hyperfine on the same machine as my comment above.

hyperfine "grass benches\many_floats.scss>NUL" --warmup 5
Benchmark #1: grass benches\many_floats.scss>NUL
  Time (mean ± σ):      22.3 ms ±   2.2 ms    [User: 4.0 ms, System: 5.7 ms]
  Range (min … max):    18.2 ms …  31.3 ms    78 runs

hyperfine "sassc benches\many_floats.scss>NUL" --warmup 5
Benchmark #1: sassc benches\many_floats.scss>NUL
  Time (mean ± σ):      32.6 ms ±   3.3 ms    [User: 3.7 ms, System: 4.8 ms]
  Range (min … max):    27.3 ms …  39.5 ms    53 runs

hyperfine "dart-sass benches\many_floats.scss>NUL" --warmup 5
Benchmark #1: dart-sass benches\many_floats.scss>NUL
  Time (mean ± σ):     445.9 ms ±  13.7 ms    [User: 4.0 ms, System: 16.4 ms]
  Range (min … max):   427.4 ms … 474.0 ms    10 runs

hyperfine "rsass benches\many_floats.scss>NUL" --warmup 5
Benchmark #1: rsass benches\many_floats.scss>NUL
  Time (mean ± σ):      25.3 ms ±   2.5 ms    [User: 3.7 ms, System: 8.3 ms]
  Range (min … max):    21.4 ms …  32.2 ms    74 runs

and if you prefer a table,

	Time	Language	Version
grass	22.3 ms ±2.2 ms	Rust	master (c42fdc5)
rsass	25.3 ms ±2.5 ms	Rust	rsass 0.12.3-PRE
sassc	32.6 ms ±3.3 ms	C/C++	libsass master (`c3c1c0e`), sassc master (`3e73bae`)
dart-sass	445.9 ms ±13.7 ms	Dart	1.26.3

The file tested contained 250 lines of large floats generated with python's random.random()

You can find the full file here.

It should be noted that rsass by default uses a precision of 5, rather than 10 like the other implementations. Using the flag rsass --precision 10 puts it at around 75 seconds.

In addition, these are the results from the @for test case from my comment above,

hyperfine "grass big_for.scss>NUL" --warmup 5
Benchmark #1: grass big_for.scss>NUL
  Time (mean ± σ):     774.3 ms ±  51.2 ms    [User: 4.0 ms, System: 5.5 ms]
  Range (min … max):   714.7 ms … 862.6 ms    10 runs

hyperfine "sassc big_for.scss>NUL" --warmup 5
Benchmark #1: sassc big_for.scss>NUL
  Time (mean ± σ):      1.496 s ±  0.031 s    [User: 2.7 ms, System: 2.9 ms]
  Range (min … max):    1.439 s …  1.546 s    10 runs

hyperfine "dart-sass big_for.scss>NUL" --warmup 5
Benchmark #1: dart-sass big_for.scss>NUL
  Time (mean ± σ):      1.299 s ±  0.029 s    [User: 2.4 ms, System: 16.3 ms]
  Range (min … max):    1.245 s …  1.332 s    10 runs

hyperfine "rsass big_for.scss>NUL" --warmup 5
Benchmark #1: rsass big_for.scss>NUL
  Time (mean ± σ):     807.0 ms ±  46.7 ms    [User: 3.9 ms, System: 7.8 ms]
  Range (min … max):   741.2 ms … 865.6 ms    10 runs

and in table format,

	Time
grass	774.3 ms ±51.2 ms
rsass	807.0 ms ±46.7 ms
dart-sass	1.299 s ±0.029 s
sassc	1.496 s ±0.031 s

I want to clarify that again this is not enough to definitively prove anything with regard to performance and is benchmarking something very specific. Having said that, for every benchmark I have tried, the ordering you see in the charts above has been roughly the same. Most notably, for loops in grass are now just as fast as those in rsass. Interestingly, dart-sass manages to beat sassc when compiling for loops.

There is still a lot of room for optimization within grass. I am really excited to try out an idea I had for interning style properties as well as identifiers. I tested this initially with interning of all strings, and there were significant (~20%+) improvements, but I had to revert this change due to issues related to multithreading. It also made the code handling strings more complicated than it should be, so I found it easier to just revert the entire feature.

I would be interested to see how much performance we lose as a result of implementing @extend. After that, I would like to do more scientific benchmarking to definitively claim grass is the fastest.

connorskees commented 4 years ago

@MartinKavik grass is now able to compile the most recent verison of bulma-scss.

I'm still trying to track down an @extend bug that results in a combinatorial explosion when extending certain pseudoselectors, but other than that and some extra newlines the output is the exact same as dart-sass.

Performance has regressed slightly as a result of implementing @extend, but grass still appears to beat sassc and dart-sass in most benchmarks. Due to the bug I've mentioned, sassc beats grass in compiling bulma, but I would expect that disparity to disappear once I resolve it.

I'll be releasing a new bugfix version to crates.io once I can resolve the @extend issue.

connorskees commented 4 years ago

It is now possible to compile bulma-scss 100% accurately!*

* ignoring some newlines

You can play around with it by running,

git clone https://github.com/j1mc/bulma-scss
cargo install grass --force
grass bulma-scss/bulma.scss>out.css

There are currently 1 bug and 1 feature blocking bootstrap. The bug being something related to colors and number precision (I think grass is too precise!) and the feature being more robust parsing of @keyframes (specifically percent selectors). After these two are resolved, I have confirmed that grass will be able to compile the most recent version of bootstrap.

MartinKavik commented 4 years ago

It is now possible to compile bulma-scss 100% accurately!

Awesome news! I'm just writing the third and the biggest Seed tutorial and I want to use bulma and grass. It will be published on seed-rs.org.

connorskees commented 4 years ago

I've just released grass v0.9.3, which includes a 3x improvement for bulma compile time. With these optimizations, grass is now roughly 25% faster than libsass+sassc

Benchmark results

``` hyperfine "grass-new bulma-scss\bulma.scss" --warmup 5 Benchmark #1: grass bulma-scss\bulma.scss Time (mean ± σ): 186.7 ms ± 5.6 ms [User: 6.4 ms, System: 5.0 ms] Range (min … max): 176.0 ms … 200.6 ms 15 runs hyperfine "sassc bulma-scss\bulma.scss" --warmup 5 Benchmark #1: sassc bulma-scss\bulma.scss Time (mean ± σ): 244.1 ms ± 16.2 ms [User: 5.7 ms, System: 5.3 ms] Range (min … max): 218.1 ms … 263.3 ms 10 runs ```

(to anyone subscribed to this issue, I'll try to avoid pinging this thread again until bootstrap is able to be compiled)

connorskees commented 4 years ago

Sorry to alert everyone so soon, but I was not expecting to finish this tonight!

grass v.0.9.4 is able to compile the most recent version of twitter bootstrap.

To test it out,

git clone https://github.com/twbs/bootstrap --depth 1
cargo install grass --force
grass bootstrap/scss/bootstrap.scss>out.css

I'll be leaving this issue open until I can resolve the extra newlines and special cased division.

pickfire commented 4 years ago

@MartinKavik Off-topic, I am wondering if seed will ever look into not using virtual DOM?

MidasLamb commented 4 years ago

For me, bootstrap 4.5.2 doesn't compile with version 0.10.0, nor with 0.9.4:

Error: Expected "important".
  ╷
9 │ $gray-100: #f8f9fa !default;
  │                    ^^^^^^^^
  ╵
././scss/_variables.scss:9:20

connorskees commented 4 years ago

@MidasLamb You're right! Thank you for bringing this up. The issue is almost certainly related to a bug recently introduced in one of our dependencies. The error was introduced in a bugfix release so cargo is pulling in the broken version. This also broke CI. See https://github.com/connorskees/grass/commit/0c7e2017d5f3e6a1f540d746a138659741e44671 for a recent fix for this. I will release 0.10.1 today and have the Cargo.lock included on crates.io in order to avoid this occurring in the future.

MidasLamb commented 4 years ago

@connorskees , great, thanks for the quick response!

connorskees commented 4 years ago

@MidasLamb for now unfortunately it seems that publishing 0.10.1 is blocked on our dependency publishing the patch to crates.io. I've opened an issue there so hopefully we won't have to wait too long.

If you'd like to use grass now, you can install it with cargo install --git https://github.com/connorskees/grass, which I have confirmed to work with the version of Bootstrap on my machine.

connorskees commented 3 years ago

The next release of grass will have byte-for-byte accurate output for bootstrap v5.0.2, verified by CI.

Additionally, I have run a script to perform this check for the last 2,500 commits of bootstrap and have resolved all the differences (largely strange handling of newlines). This commit range includes all commits of bootstrap 4 and 5.

I am considering the work necessary to compile bootstrap done. Remaining features/bugs are tracked in #19.

connorskees / grass

Compile Twitter Bootstrap 4 #4