Make scry faster and lightweight

[faustinoaq]

Hi, this PR implements lightweight features:

• Lightweight analyzer and implementations
• Avoid heavy compiler dependencies
• Use external process to free up memory

Before:

With this PR changes:

Same features and behaviors, more RAM friendly and lightweight binary 19.2MB vs 4.7MB

Oh, also compilation time has been reduced drastically:

$ time crystal build src/scry.cr -s -p --no-debug
Parse:                             00:00:00.000316202 (   0.25MB)
Semantic (top level):              00:00:00.404959721 (  52.33MB)
Semantic (new):                    00:00:00.002962556 (  52.33MB)
Semantic (type declarations):      00:00:00.032710793 (  52.33MB)
Semantic (abstract def check):     00:00:00.002463025 (  52.33MB)
Semantic (ivars initializers):     00:00:00.001851865 (  52.33MB)
Semantic (cvars initializers):     00:00:00.240542928 (  68.33MB)
Semantic (main):                   00:00:01.663311243 ( 196.77MB)
Semantic (cleanup):                00:00:00.006314623 ( 196.77MB)
Semantic (recursive struct check): 00:00:00.002034422 ( 196.77MB)
Codegen (crystal):                 00:00:01.197870686 ( 228.77MB)
Codegen (bc+obj):                  00:00:05.765841086 ( 228.77MB)
Codegen (linking):                 00:00:00.761140190 ( 228.77MB)
                                          
Codegen (bc+obj):
 - no previous .o files were reused
crystal build src/scry.cr -s -p --no-debug  14.42s user 0.81s system 149% cpu 10.159 total

Previously scry compilation used a huge amout of RAM (above 1GB) and took several minutes to compile in my pc 😓

Fixes #30

Related comment on #48 (comment)

[bew]

I wonder how the completion system will work with this

[faustinoaq]

I wonder how the completion system will work with this

@bew auto-completion feature will keep working as usual

[laginha87]

Completion just uses the compiler/syntax from crystal to build the ast trees.
Glad to see the reduction in compilation time.
I don't how I feel about passing it off to the crystal tools though, seems like it might create a bigger dependency on what version of crystal the user is using.

[faustinoaq]

Completion just uses the compiler/syntax from crystal to build the ast trees. Glad to see the reduction in compilation time.

👍

I don't how I feel about passing it off to the crystal tools though, seems like it might create a bigger dependency on what version of crystal the user is using.

@laginha87 Then, a developer using scry will see the errors and implementation behaviors according to the crystal version that he/she is using. Will not depend on the crystal version that scry was compiled, so finally, is another benefit for using an external crystal process 😄

[keplersj]

Like I said here I'm still not sure this is a good idea.

If this PR included README changes stating that this is dependent on the user having the Crystal compiler installed in their PATH and included a mechanism to alert the user if they don't when the server starts I would be more onboard with a change like this.

[faustinoaq]

If this PR included README changes stating that this is dependent on the user having the Crystal compiler installed in their PATH and included a mechanism to alert the user if they don't when the server starts I would be more onboard with a change like this.

Hi @keplersj 😄

We're already dependent on crystal command installed and avaliable on PATH. Scry is already executing crystal here:

scry/src/scry/environment_config.cr

Line 25 in 1bff42e

Process.run("crystal", ["env"], output: io)

So currently if a user doesn't have crystal installed, then scry won't work

This PR just take advantage of using crystal command to be memory friendly, to compile faster and to generate a smaller binary.

[keplersj]

@faustinoaq Hmm... Okay. I still can't shake this feeling that relying on the Crystal compiler command line is going to be a bad idea in the long run and probably breaks with the norms of other LSP servers. But, you have good points. I'm okay with this for now.

I do want to hear @bcardiff's opinion on this though.

[faustinoaq]

I still can't shake this feeling that relying on the Crystal compiler command line is going to be a bad idea in the long run and probably breaks with the norms of other LSP servers.

The src/protocol folder will work as usual, this PR just modifies the way to process 2 heavy features (diagnostics and implementations) and keeps other things untouched.

In fact the only disadvantage of using this approach is that compiler could output extra characters to stdio in some special situations: like Using bin/crystal at commit kjdf6zx (can be disabled) commonly shown in local crystal builds from git repo or benchmark warning like Using benchmark without --release flag, bla, bla... or some crystal bug. because it will break json output parsing.

[bmulvihill]

@faustinoaq How will this affect CI? We should be able to remove any LLVM dev dependencies?

[faustinoaq]

How will this affect CI? We should be able to remove any LLVM dev dependencies?>

@bmulvihill Oh yeah, another advantage of this PR, LLVM won't be a requirement to compile scry! 🌮

➜  scry git:(fa/lightweight-scry-speedup) ✗ sudo pacman -Rns llvm
[sudo] password for main: 
checking dependencies...

Packages (1) llvm-5.0.1-2

Total Removed Size:  159.96 MiB

:: Do you want to remove these packages? [Y/n] Y
:: Processing package changes...
(1/1) removing llvm               [#####################################################] 100%
:: Running post-transaction hooks...
(1/1) Arming ConditionNeedsUpdate...
➜  scry git:(fa/lightweight-scry-speedup) ✗ time crystal build src/scry.cr -s -p --no-debug
Parse:                             00:00:00.000311398 (   0.25MB)
Semantic (top level):              00:00:00.379528606 (  52.33MB)
Semantic (new):                    00:00:00.002840074 (  52.33MB)
Semantic (type declarations):      00:00:00.034088660 (  52.33MB)
Semantic (abstract def check):     00:00:00.002330909 (  52.33MB)
Semantic (ivars initializers):     00:00:00.001950688 (  52.33MB)
Semantic (cvars initializers):     00:00:00.250472900 (  68.33MB)
Semantic (main):                   00:00:01.612165740 ( 196.77MB)
Semantic (cleanup):                00:00:00.006179480 ( 196.77MB)
Semantic (recursive struct check): 00:00:00.001546292 ( 196.77MB)
Codegen (crystal):                 00:00:01.210651356 ( 228.77MB)
Codegen (bc+obj):                  00:00:00.387764860 ( 228.77MB)
Codegen (linking):                 00:00:00.714893504 ( 228.77MB)
                                          
Codegen (bc+obj):
 - all previous .o files were reused
crystal build src/scry.cr -s -p --no-debug  4.80s user 0.47s system 112% cpu 4.674 total
➜  scry git:(fa/lightweight-scry-speedup) ✗ rm -rf ~/.cache/crystal                        
➜  scry git:(fa/lightweight-scry-speedup) ✗ time crystal build src/scry.cr -s -p --no-debug
Parse:                             00:00:00.000323728 (   0.25MB)
Semantic (top level):              00:00:00.395329747 (  52.33MB)
Semantic (new):                    00:00:00.002867334 (  52.33MB)
Semantic (type declarations):      00:00:00.036601810 (  52.33MB)
Semantic (abstract def check):     00:00:00.002567500 (  52.33MB)
Semantic (ivars initializers):     00:00:00.002118485 (  52.33MB)
Semantic (cvars initializers):     00:00:00.246402066 (  68.33MB)
Semantic (main):                   00:00:01.628563404 ( 196.77MB)
Semantic (cleanup):                00:00:00.005707169 ( 196.77MB)
Semantic (recursive struct check): 00:00:00.001652108 ( 196.77MB)
Codegen (crystal):                 00:00:01.205903900 ( 228.77MB)
Codegen (bc+obj):                  00:00:06.013242583 ( 228.77MB)
Codegen (linking):                 00:00:00.712104028 ( 228.77MB)
                                          
Codegen (bc+obj):
 - no previous .o files were reused
crystal build src/scry.cr -s -p --no-debug  14.66s user 0.71s system 148% cpu 10.323 total

😄

[faustinoaq]

How will this affect CI? We should be able to remove any LLVM dev dependencies?

Well, now CI is 3X faster 🚀

Before: 5.13s

Now 1.44s 💯

Memory usage has changed as well, 1GB vs 250MB 😅

[bmulvihill]

@faustinoaq Originally I was against this idea, but I think the benefit of not needing to package the entire Crystal compiler is a big win.

[keplersj]

@bmulvihill I second your comments. I've been swayed. This is a good idea. If there were a way to interact with the compiler programmatically without all the weight I'd be for that, but this works well in this form.

[faustinoaq]

So, I think this is ready, WDYT about merging this?

[keplersj]