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

sassc using large amount of memory with libsass 3.6.3 #3033

Closed
phmccarty opened this issue Nov 5, 2019 · 18 comments
Closed

sassc using large amount of memory with libsass 3.6.3 #3033

phmccarty opened this issue Nov 5, 2019 · 18 comments

Comments

@phmccarty
Copy link

I am attempting to build the latest release of arc-theme, which has a build dependency on sassc/libsass.

With libsass 3.6.2, the build was hanging, very likely due to this issue. With libsass 3.6.3, sassc starts to consume large amounts of memory. I don't know if the build would successfully complete, because my system was about to run out of memory; sassc was using about 14 GB of RAM when I killed the process.

Steps to reproduce:

git clone https://github.com/arc-design/arc-theme
cd arc-theme/common/gtk-3.0/3.20
sassc sass/gtk.scss gtk.css
@saper
Copy link
Member

saper commented Nov 6, 2019

node-sass using libsass 3.5.5 compiled it under 2 seconds:

> time node-sass sass/gtk.scss sass/gtk.css
Rendering Complete, saving .css file...
Wrote CSS to /home/saper/sw/arc-theme/common/gtk-3.0/3.20/sass/gtk.css

real    0m2,02s
user    0m1,60s
sys     0m0,19s

> node-sass -v
node-sass       4.13.0  (Wrapper)       [JavaScript]
libsass         3.5.5   (Sass Compiler) [C/C++]

@saper
Copy link
Member

saper commented Nov 6, 2019

With libsass master (d91105 which is 3.6.3 plus 4 commit) there are lots of warnings:

Compound selectors may no longer be extended.

3.6.1 seems to be fine.

@xzyfer
Copy link
Contributor

xzyfer commented Nov 6, 2019 via email

@saper
Copy link
Member

saper commented Nov 6, 2019

@xzyfer but is this feature still supported? it seems that extension process is causing trouble here (according to the debugger)

@mgreter
Copy link
Contributor

mgreter commented Nov 6, 2019

Has anyone tried it with dart sass, I suspect it will run into the same issue.

@mgreter
Copy link
Contributor

mgreter commented Nov 6, 2019

AFAIR that gtk theme has quite a few @extend * which seems to cause long runtime.

@saper
Copy link
Member

saper commented Nov 6, 2019

Nope, dart is not even running on my system.

@saper
Copy link
Member

saper commented Nov 6, 2019

Does it ever finish? I poked around it with a debugger and it seems to jump around extending stuff; it also does not seem to endlessly recurse.

@mgreter
Copy link
Contributor

mgreter commented Nov 6, 2019

I believe it will finish if you have an absurd amount of RAM. It seems to even exhaust 64GB. But the real issue is that the extend rules for this theme are really getting out of hand and probably don't work as they have been intended. Not sure exactly how or why older libsass extend was working but I suspect that we didn't obey the 3rd law of extend (It trims redundant selectors as much as possible, while still ensuring that the specificity is greater than or equal to that of the extender).
Dart-sass seems to behave exactly the same, albeit slower accumulating memory.

FWIW I was able to compile _common.scss and it alone produced a 6MB big css file.

image

@lazka
Copy link

lazka commented Nov 8, 2019

Similar problem with the default gtk CSS theme: https://gitlab.gnome.org/GNOME/gtk/issues/2237

sassc used to create a 192kb CSS file and starting with 3.6.3 (3.6.2 got into an endless loop) the resulting CSS is 70MB large.

@mgreter
Copy link
Contributor

mgreter commented Nov 9, 2019

It seems to boil down to some usage pattern like these:

%undecorated_button {
  box-shadow: none;
}

button:link:hover, button:visited:hover,
button:link:active, button:visited:active,
button:link:checked, button:visited:checked {
  @extend %undecorated_button;
}

headerbar {
  @extend %header_widgets;
}

%header_widgets {

  &.selection-mode button {

    &, &.flat {
      asd: qwe;
    }
    &:active, &:checked { @extend :active; }

    &:disabled {
      @extend :disabled;

      &:checked, &:active { @extend :disabled, :checked; }
    }
  }
}

I would suggest to take this upstream to dart-sass, as it suffers from the same issue.

@lazka
Copy link

lazka commented Nov 10, 2019

Here is an example which produces a bogus result starting with 3.6.2:

*:hover {
  *:visited {
    color: green;
  }
}

button {
  @extend *:hover;
}

@saper
Copy link
Member

saper commented Nov 10, 2019

Is there a relatively current sass language specification? I think we have reached the point here where just following the reference implementation leads to wrong results.

@nex3
Copy link
Contributor

nex3 commented Nov 12, 2019

@saper There's a partial spec in https://github.com/sass/sass/tree/master/spec. It gets filled in lazily as necessary when writing proposals for new language features.

@bdkjones
Copy link

Is there a proposed resolution for this? I shipped Libsass 3.6.2 but had to rollback after many reports of hung compiling. It seems this issue is likely to affect many people, so I’m hesitant to ship 3.6.3 now.

@mgreter
Copy link
Contributor

mgreter commented Nov 26, 2019

No, there hasn't, but the hung compilation was another issue that should be addressed correctly in 3.6.3. The issue here is how the exend logic of dart sass works. I think if you don't use @extend * it should be safe (and I think one really shouldn't use that pattern). Any chance you can put out a beta version for further testing?

Edit: it seems the main problem is the difference between e.g. @extend *:hover; vs @extend *, :hover;. I was under the impression both should produce the same result, but a check with older ruby sass shows that even old ruby sass has the same behavior. So the question might be if dart sass should re-add extending of complex selectors in order to support this better. But I can't make that call, that's why I suggested to take this upstream to dart-sass.

sass/sass#1599

gnomesysadmins pushed a commit to GNOME/gtk that referenced this issue Nov 27, 2019
They are no longer supported by sass and broken with libsass 3.6.3
(sass/libsass#3033)

This removes some of them by replacing them with a placeholder selector.
This at least brings the resulting CSS size down a bit so gtk can be build
again.

The remaining cases I don't know how to convert because I haven't found a way to
reproduce the old output.

The CSS was generated with libsass 3.5.5.

See #2237
gnomesysadmins pushed a commit to GNOME/gtk that referenced this issue Dec 1, 2019
They are no longer supported by sass and broken with libsass 3.6.3
(sass/libsass#3033)

This removes them by replacing them with a placeholder selector. This at
least brings the resulting CSS size down a bit so gtk can be build
again.

`%button.flat.suggested-action` has been replaced by
`%selection_mode_button_flat`, which is a more appropriate selector for
`.selection-mode button.titlebutton`.

The CSS was generated with libsass 3.5.5.

Co-authored-by: Christoph Reiter <reiter.christoph@gmail.com>

See https://gitlab.gnome.org/GNOME/gtk/issues/2237
@q66
Copy link

q66 commented Dec 16, 2019

I can confirm that 3.6.3 hangs and keeps eating memory until it invokes OOM killer. Void Linux ppc64le, trying to build arc-theme.

Ponce added a commit to Ponce/slackbuilds that referenced this issue Feb 13, 2020
sass/libsass#3033

Signed-off-by: Matteo Bernardini <ponce@slackbuilds.org>
Ponce added a commit to Ponce/slackbuilds that referenced this issue Feb 15, 2020
sass/libsass#3033

Signed-off-by: Matteo Bernardini <ponce@slackbuilds.org>
Ponce added a commit to Ponce/slackbuilds that referenced this issue Feb 24, 2020
sass/libsass#3033

Signed-off-by: Matteo Bernardini <ponce@slackbuilds.org>
Ponce added a commit to Ponce/slackbuilds that referenced this issue Feb 29, 2020
sass/libsass#3033

Signed-off-by: Matteo Bernardini <ponce@slackbuilds.org>
@mgreter
Copy link
Contributor

mgreter commented Mar 4, 2021

Closing this out as it is as it is. Since LibSass follows the lead of now official dart sass, and this same problem exists there, we can't do much to fix. This behavior is intrinsically related to how dart sass now handles @extend and any future LibSass release will suffer from this issue. If you believe this is wrong please take your arguments upstream to https://github.com/sass/sass. Please re-open with further info, if you find a case where dart-sass compiles OK but LibSass doesn't.

@mgreter mgreter closed this as completed Mar 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants