Limit Jedi RAM consumption

[MikhailArkhipov]

Fixes #263

Introduces setting for the Jedi RAM limit. Default (0) means 1024 MB. Max 8GB.
RAM consumption is checked every 2 seconds and completion.py process is killed if threshold is exceeded. Process is then auto-restarted.

[MikhailArkhipov]

Not sure why diff is so large

[DonJayamanne]

White spaces, I think you have re-formatted package.json (you're probably using tabs instead of spaces). Do you have the the editor config extension installed?

[codecov]

Codecov Report

Merging #744 into master will increase coverage by 0.01%.
The diff coverage is 71.87%.

@@            Coverage Diff             @@
##           master     #744      +/-   ##
==========================================
+ Coverage   62.63%   62.65%   +0.01%     
==========================================
  Files         242      242              
  Lines       11236    11256      +20     
  Branches     2033     2037       +4     
==========================================
+ Hits         7038     7052      +14     
- Misses       4190     4196       +6     
  Partials        8        8

Impacted Files	Coverage Δ
src/client/common/types.ts	100% <ø> (ø)	⬆️
src/client/common/configSettings.ts	91.71% <100%> (+0.05%)	⬆️
src/client/common/extensions.ts	88.23% <100%> (+1.56%)	⬆️
src/client/common/logger.ts	53.33% <33.33%> (-10.31%)	⬇️
src/client/providers/jediProxy.ts	72.15% <78.26%> (+0.15%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 012985c...e78e311. Read the comment docs.

[DonJayamanne]

Shouldn't we be capturing the reason and passing that through to the onError function

[MikhailArkhipov]

Which logger should I use

[DonJayamanne]

What's the purpose of doNotWait. If i'm not mistaken, we'd only use this in case where we don't care about any exceptions raised at all. Hence I'd say, we don't care about the errors, that's why we didn't have any error handlers in the first place. If we do want them, then i'd put the logging in the code itself, rather than creating an extension method.

So, all we need is:

/**
 * Explicitly tells that promise should be run asynchonously.
 */
Promise.prototype.doNotWait = function(this: Promise) {
    this.catch(()=>{});
    // or     this.catch(nop);
};

Now, when you look at the code, this extension feels kind of redundant or we should rename it to something such as ignoreErrors().

Because technically this still returns a promise that will get resolved. So it not about waiting or not. We just want to ignore the errors.

[MikhailArkhipov]

The purpose is to explicitly state that we don't want to await here by design. Generally caller doesn't care about exceptions, but may appreciate if they are logged.

[MikhailArkhipov]

In C# we have SilenceException and DoNotWait. The latter indicates that task runs freely and is not awaited, but it does rethrow exceptions.

[MikhailArkhipov]

Let's make this one into ignoreErrors. We can discuss rethrowing separately

[DonJayamanne]

I'd removed all the parameters completely. If the purpose of this extension is to let promises not to wait, then all we need is a catch inside this and it should log if necessary.
Else this function is identical to a standard promise, apart from the fact that we have a default catch handler.

Ie this extension should do what it says, not to wait. However we are adding callbacks for when it completes. I.e. we are waiting.

[MikhailArkhipov]

Yes, you are right. I confused C# 'ContinueWith' with 'then'. Primary purpose is to make tslint happy.

[DonJayamanne]

I don't l like this, we're just creating a water for promises.
Ideally we need to handle the then the normal way, then after that we can call the doNotWit as follows:

this.startL...().then(()=> this.initialized.resolve()).doNotWait()

[DonJayamanne]

I.e. doNotWait should not require any args, else it's same as a promise but epmty default catch handler.

[MikhailArkhipov]

OK. So this.startLanguageServer().catch(() => { }).then(() => this.initialized.resolve()); actually waits?

[DonJayamanne]

this.startLanguageServer().catch(() => { }).then(() => this.initialized.resolve());

no, the problem is the last method this.initialized.resolve() invoked in the chain could fail, hence tslint wants us to handle errors at the end.
So, the solution is:

this.startLanguageServer()
    .catch(() => { })
    .then(() => this.initialized.resolve())
    .catch(() => { });

Or:

this.startLanguageServer()
    .catch(() => { })
    .then(() => this.initialized.resolve())
    .doNotWait();

I think we need a global const for nop (where const nop = ()=> {};)

this.startLanguageServer()
    .catch(nop)
    .then(() => this.initialized.resolve())
    .catch(nop)

Again, all the promises are getting chained, and TS lint wants us to catch an exception from the last method in the chain.