sass / libsass-net

A lightweight wrapper around libsass
MIT License
94 stars 35 forks source link

Large project performance #25

Closed ghost closed 7 years ago

ghost commented 9 years ago

I am contemplating replacing a node based build process with libsass-net, however am seeing some strange performance issues.

For example, to build bootstrap can sometimes take 20+ seconds with libsass-net whereas node-sass does this in a consistent 1.5s.

One thing I did notice is that compile times can vary between 12s and 20s for libsass-net so maybe it's just a setting I have which is forcing more IO?

An example repo is here, with both node and c# runners: https://github.com/amar-tcpl/libsass-perf

janzii commented 9 years ago

This might very well be because the AppDomain has been restarted causing the unmanaged code freak out. Restarting the Application Pool should solve the problem (temporarily, until next build, web.config edit or other things that restart the AppDomain...)

As far as I understand, this seems to be a problem with any unmanaged code that uses static objects. It would be awesome to be able to completely dispose of those. See for example: http://forums.asp.net/t/1250907.aspx?Unmanaged+Resource+considerations+when+using+static+objects+in+an+HttpHandler

aviatrix commented 9 years ago

@amar-tcpl I am aware of the performance issue, and am looking into how we can fix that.

@janzii Interesting info. I will do some perf testing by making the http handler non reusable and see if that helps. or maybe dispose the sass C/C++ code manually after each scss compile.

ghost commented 9 years ago

@aviatrix Great, thanks!

@janzii very interesting! So in my example, it was a console application (20+ seconds), however in a web process, the same code fragment was reaching more than double that

aviatrix commented 9 years ago

@darrenkopp @janzii @amar-tcpl if any of you guys has c++ experience and is willing to track down the problem, i would be more than gratefull.

I'm testing stuff out, but since my c++ knowledge is 0, i'm pretty much in the dark here.

ghost commented 9 years ago

@aviatrix Thanks for reopening, I closed it by accident!

Unfortunately my c++ is non-existent, however I will have a play around and see if I can spot some patterns atleast

darrenkopp commented 9 years ago

I just updated libsass-net to use the new API, perhaps it was a regression when using the old API. can you benchmark the speed again using the new code?

aviatrix commented 9 years ago

@darrenkopp The issue still persist, after iis site ( not app pool) restart the compile takes ~2-3 minutes i even did some profiling on the thing but i'm at work now, will post the details later on.

ghost commented 9 years ago

@darrenkopp thanks for looking. I have tried the benchmarks on my test project, and the times are looking the same I'm afraid. (Note, this was a console app, not via IIS)

driekus77 commented 9 years ago

We are having the same problems with our HttpHandler and libsassnet. After a change in e.g. web.config or a (re)build, the compiler could take more than 2 minutes to succeed. Command line I can't reproduce this. I do have C/C++ experience so when I have time I look into it.

ghost commented 9 years ago

@driekus77 thanks for looking into it. As for the command line app, can you reproduce the issue with the example project in this ticket?

driekus77 commented 9 years ago

@amar-tcpl thanks for pointing me to the example project I will try it later today. I was testing it with Foundation SCSS but ran into assertion very quickly: https://github.com/sass/libsass/issues/1307 After my fix I could not reproduce it using libsass 3.2.5 with foundation. Again I will try your project later on today. Thanks

driekus77 commented 9 years ago

Could not reproduce stalling with above test program so I tweaked it a bit by adding multi threading. Then I could reprocedue it and run it through Intel VTune: Top Hotspots Function CPU Time operator delete 53.131s operator new 52.727s [Outside any known module] 26.496s func@0x18003b19f 20.326s Sass::Context::compile_file 17.035s [Others] 77.946s

So lots of time is spend in deallocating and allocating which is of course normal. Maybe caching can be introduced. Can't find a quick fix right away...

Any idea's are helpfull.

aviatrix commented 9 years ago

@driekus77 IMO, a quick and dirty fix would be to destroy the whole compile object and recreate it each time and/or use a separate instance for each thread. this could help with another bug that is triggered by IIS restarting the process that hosts the c++ dll.

New instance of the code compile foundation 5.5 for 1.2-2.2 seconds from scratch. for development purposes this is ideal.

driekus77 commented 9 years ago

Debugged and Hacked arround a little bit with libsass, libsassnet and libsasstest all in debug.

I found following remarkable things: file.cpp --> no try/catch in read_file's WIN32 part. Also if checking file length doesn't check and clear the file HANDLE if needed.

SassCompiler doesn't have IDisposable implemented SassInterface doesn't have IDisposable implemented Its discussable if these IDisposable implementations are realy needed...

After implementening IDisposable for both classes and rebuilding all in Release I got: 30 threads in Parallel:

  1. 9258 (ms)
  2. 9187 (ms)
  3. 9336 (ms) Witch looks kind of stable.

Used: libsass 64bits 3.2.5 release code libsass-net 3.2.2 64 bits release code libsasstest 64 bits release code

Test code can be found at: https://github.com/driekus77/SassDebugTests/blob/master/libsassTests/Program.cs

driekus77 commented 8 years ago

Finally I got an Stalling libsass-net with VTune attached.

See screenshot for top-down tree with timings: vtune_libsass-net_stalling

Unfortunatelly it doesn't tell me much. Only thing I can think of is if the libsass dll's are properly signed?

The most self time is burned in PEDecoder::IsILOnly ( clr.dll ) which looks strange to me. Googling on this doesn't return anything usefull.

Anybody has an idea when looking at the timing screenshots?

Kind regards,

Henry

driekus77 commented 8 years ago

After searching and reading on internet e.g.: https://msdn.microsoft.com/en-us/library/ms173266.aspx

I took another look at the C++ code and compiler/project settings and found:

Without /clr for native code I can't reproduce stalling anymore. If anybody want's to check for themselve: https://github.com/driekus77/SassDebugTests/releases/download/3.2.3/resources.zip

Kind regards,

Henry

ghost commented 8 years ago

@driekus77 running your DLLs brings the bootstrap compile time down to about 1.5s on my box (same as node-libsass), rather than the 20+ seconds from 3.2.4.1 so it looks very good to me!

I also confirmed both versions output identical files.

Thanks for looking into this!

driekus77 commented 8 years ago

Some test results:

Testing 30 threads with bootstrap:

  1. Total time: 3148.
  2. Total time: 3401.
  3. Total time: 3131 Average: 3226,667 ==> Divided by 30: 107,5556 ms per Bootstrap compile to file.

Testing 30 threads with foundation:

  1. Total time: 4751.
  2. Total time: 4599.
  3. Total time: 4550. Average: 4633,333 ==> Divided by 30: 154,444 ms per Foundation compile to file.

To run the tests yourself see code at: https://github.com/driekus77/SassDebugTests

Kind regards, Henry

driekus77 commented 8 years ago

Some food for toughts:

@amar-tcpl Thanks for youre quick feedback! It was a releave because I could not always reproduce this stalling/performance bug.

I hope this bug will be fixed soon in the nuget-package version of libsass-net?

Thanks for the help and feedback. Kind regards, Henry

driekus77 commented 8 years ago

Used the weekend to create one C++.NET dll that contains unmanaged libsass as static library. For results see: https://github.com/driekus77/libsass.net.mixed

API is changed a little bit because C++.Net does not allow default argument values.

Is this something people would/could be interested in or am I the only one?

Notes: Speed tests above and on the mixed repository are done with different hosts so can not be compared.

Kind regards, Henry

ghost commented 8 years ago

@driekus77 unfortunately I can't build your libsass.net.mixed to test at the moment, however it looks good from your numbers! If your recommendations from the comment 7 days ago (sorry, can't see how to link it!) are applied to this repo then what other differences or benefits will there be?

/cc @darrenkopp @aviatrix

driekus77 commented 8 years ago

@amar-tcpl I've just update the repo with VS 2013 project files. Under this solution / projects everything should work. Also added libsass as git submodule so that its buildable from within my VS solution and project files.

I think the number, 35 ms per foundation compilation, is wrong. When I run just one thread/job I get +/- 110 ms on the same host as the "154,444 ms per Foundation compile to file." tests. Somehow the Parallel.ForEach timing does not work properly in the test program.

Benefits:

Also there are Cons:

ashwin027 commented 8 years ago

Hello!...I've had similar performance problems recently and the dlls provided by @driekus77 fixed these issues. Is there a new nuget package available with this fix?

tyleregeto commented 8 years ago

Hi, we were seeing the same issues as well. We swapped out the Nuget package for the DLLs posted by @driekus77 and saw significantly faster compile times and no more issues after website restarts. We'd love to see this merged into the main repo and available through Nuget. Any chance that this could happen? Thanks

aviatrix commented 8 years ago

me and @am11 are working on a new version which has good perf (200-400 ms compile time ) and no memory leaks that crash IIS, please stay tight for few days !

darrenkopp commented 7 years ago

the alpha of the new version has been pushed to nuget.

joaope commented 7 years ago

@darrenkopp are you pushing correspondent x64 versions (for both libsassnet and libsassweb) as well? I really want to make a demo of it to the design team but web app is x64 only.

darrenkopp commented 7 years ago

no. the packages with the .x64 suffix will be going away because the libsassnet and libsassnet.Web packages now can power both 32-bit and 64-bit.

aviatrix commented 7 years ago

I'm closing this since the latest alpha packages are blazing fast @joaope @tyleregeto @ashwin027 @driekus77 @amar-tcpl @janzii