Skip to content
This repository has been archived by the owner on Oct 31, 2021. It is now read-only.

Restructure VFPT.Core for Out of Process Execution #1318

Closed
cloudRoutine opened this issue Jan 14, 2016 · 28 comments
Closed

Restructure VFPT.Core for Out of Process Execution #1318

cloudRoutine opened this issue Jan 14, 2016 · 28 comments

Comments

@cloudRoutine
Copy link
Contributor

cloudRoutine commented Jan 14, 2016

@dungpa @duckmatt earlier today @vasily-kirichenko ir and I were discussing how to improve VFPT so that it can handle the load of being employed in a production environment managing Solutions with dozens to hundreds of projects.

One way to achieve this is by moving the majority of VFPT's functionality into VFPT.Core Then we can run VFPT.Core as an independent 64bit process and send the results of its computations over sockets via protocol buffers/binary protocol/etc to a much lighter weight VSIX implementation

We also discussed building dependency graphs and symbol and reference caches to speed up
refactoring and to enable jump to anywhere

This architecture will also allow VFPT to play a more important role in F# tooling across the
various editors. In which case I think it would be prudent to rename at least the core to FSharpPowerTools.Core

Information from Core -> Editor

  • Outlining Spans (int * int) list
  • Depth Spans (int * int) list (^ often the same but not always)
  • Syntax Construct Spans (int * int * enum) list
  • Code Generations (int * int * string) list
  • Tooltips - string list
  • XMl Generation (int * int * string) list
  • Highlight Usage (int * int) list
  • Navigate to Position path * position
  • QuickInfo Sig string
  • Generation Available
  • Refactor Options Available

Information from Editor -> Core

  • Editor Events
    • cursor position change
    • active file change
    • build initiated
    • active solution configuration change
    • file open / close
  • Commands
    • code gen
    • xml gen / sig gen
    • find references generate
    • do rename

Open Questions

  • How will formatting work in this new system?
  • Will metada conventions be supported? e.g. TODOs in comments
  • The slow speed of compilation is parially do to msbuild making poor evaluations of
    when things need to be recompiled. Should we take command over this process?
@vasily-kirichenko
Copy link
Contributor

I'm completely for this (surprise :) )

@cloudRoutine remember what happened to your very large PR where you rewrite all C# code to F#? That's why I suggest the following steps:

  1. Move as much VS independent logic from Logic project to Core (a good question: why and how that logic was added there?)
  2. Little-by-little, keeping VFPT fully functional all the time, define a thing, clean, FCS independent API between Logic and Core
  3. Remove Logic -> FCS dependency
  4. Create a separate project, like VFPT.Service which is a thin proxy between Core and a pluggable transport layer
  5. We should support:
    • The fastest protocol available, like protobuf/FsPickler over TCP
    • JSON over HTTP (like what's used by FSAC) to be consumed by Atom, VS Code and other non-.NET editors

I strongly believe that API should not be thought out first, it should be discovered as we implement p. 1 and 2. So, we should not spend time thinking how we will support formatting, it will be done much easier when some other features have ported to that API.

@dsyme
Copy link
Contributor

dsyme commented Jan 14, 2016

Well, this would be great. Some thoughts

  • If we could structure this at the FCS boundary, and make it an FCS feature, then all F# tooling in all editors could benefit.
  • Do you think it's easier to make VFPT.Core the boundary? If so, do you think other cross-platform editors (Xamarin etc.) should be building on VFPT.Core instead of FCS?
  • Do you think the API to core is suitable to take out-of-process? Object model APIs generally need to be converted to "service" APIs structured much more like a set of database calls
  • What would you do about reactive callbacks from out-of-process?

thanks!

@7sharp9
Copy link

7sharp9 commented Jan 14, 2016

Less code that does the same thing is always better, consolidating the 7 different consumers of FCS into an all powerful language service has always been my goal, its been rather difficult to make it come about though.

@7sharp9
Copy link

7sharp9 commented Jan 14, 2016

oop callbacks should be relatively easy with websockets, which is the big thing we would get for free from http.

@vasily-kirichenko
Copy link
Contributor

If we could structure this at the FCS boundary, and make it an FCS feature, then all F# tooling in all editors could benefit.

Great idea, I think. Just merge Core and FCS and done all further work there. The less moving parts the better.

Do you think it's easier to make VFPT.Core the boundary? If so, do you think other cross-platform editors (Xamarin etc.) should be building on VFPT.Core instead of FCS?

See ^. All the editors win. Cool.

Do you think the API to core is suitable to take out-of-process? Object model APIs generally need to be converted to "service" APIs structured much more like a set of database calls

Of course as it is, Logic -> Core API is not suitable for service at all.

What would you do about reactive callbacks from out-of-process?

In case of "fast TCP" transport callbacks would be done by establishing another TCP connection, in opposite direction (I think this should be done with robust, battle tested (by me as well) libraries like 0MQ).

In case of HTTP, I've no idea :(

@vasily-kirichenko
Copy link
Contributor

oop callbacks should be relatively easy with websockets, which is the big thing we would get for free from http.

Oh, great. Is it possible to use compact protocol like ProtocolBuffers over HTTP?

@7sharp9
Copy link

7sharp9 commented Jan 14, 2016

Yes, you just use the binary output from protobuf and set application/octet-stream

@vasily-kirichenko
Copy link
Contributor

Great. What about suave as a server? Too slow?

@cloudRoutine
Copy link
Contributor Author

@vasily-kirichenko I think it'd be useful to have a feature branch for this objective

@7sharp9
Copy link

7sharp9 commented Jan 14, 2016

And what of the core? A fork of FCS?

@enricosada
Copy link
Contributor

Great idea.

I'll add a link https://github.com/OmniSharp/omnisharp-roslyn because it's the same effort for c# based on roslyn ( instead of FCS ). Maybe it's possible to learn about api, tradeoff, mistake because it's a good cross ide story

@7sharp9
Copy link

7sharp9 commented Jan 14, 2016

@vasily-kirichenko
Just tested and Suaves latency is on average 165ms For comparison node.js has a latency of 38ms and iron in rust has a latency of 1.59ms

Suave:

Running 10s test @ http://127.0.0.1:8083/
  12 threads and 900 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency   165.10ms  176.52ms   1.99s    94.46%
    Req/Sec   251.60     93.32   777.00     80.75%
  27823 requests in 10.06s, 3.94MB read
  Socket errors: connect 0, read 1063, write 1, timeout 60
Requests/sec:   2764.77
Transfer/sec:    401.12KB

Node.js

Running 10s test @ http://localhost:8000/
  12 threads and 900 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency    38.91ms   23.17ms 300.64ms   67.03%
    Req/Sec     0.90k   286.63     2.34k    79.36%
  104253 requests in 10.05s, 15.51MB read
  Socket errors: connect 0, read 872, write 0, timeout 0
Requests/sec:  10373.39
Transfer/sec:      1.54MB

Rust - Iron:

Running 10s test @ http://localhost:3000/
  12 threads and 900 connections
  Thread Stats   Avg      Stdev     Max   +/- Stdev
    Latency     1.59ms    1.10ms 117.24ms   99.57%
    Req/Sec     3.37k     1.51k    6.69k    64.48%
  399493 requests in 10.03s, 43.43MB read
  Socket errors: connect 0, read 626, write 0, timeout 0
Requests/sec:  39847.95
Transfer/sec:      4.33MB

@vasily-kirichenko
Copy link
Contributor

wow. 200 ms is too large.

@dungpa
Copy link
Contributor

dungpa commented Jan 15, 2016

Let's do this. I'm not sure how it turns out to be, but I can play a supporting role in this initiative.

@dungpa
Copy link
Contributor

dungpa commented Jan 15, 2016

/cc @nosami for his insights from OmniSharp.

@enricosada
Copy link
Contributor

as the service backend host, an option i think it's a good candidate is https://github.com/aspnet/KestrelHttpServer. xplat, http, perf oriented (they try really hard), libuv based like node, active and mantained. For development you can use owin directly, or some library compatible with owin.

Another feedback i want to add it's to really try hard to use http directly instead of tcp if performance is enough ( https://github.com/helios-io/helios it's good but tcp oriented, like 0MQ ) . That's remove a needed wrapper for some ide extensions. If http/json performance it's ok for vscode, i think it's ok for others ide (it's possible to plug multiple response content format like protobuff)

@nosami
Copy link

nosami commented Jan 15, 2016

I would recommend not to use a HTTP server and use stdio instead for the following reasons :-

  • A HTTP server can't push information to the editor without it originating from a request. This may or may not be an issue. It was useful in OmniSharp to raise events that originate from the server (project loaded etc) that the editor could act on.
  • HTTP is more difficult to manage. You have to handle finding free ports etc

omnisharp-roslyn supports both HTTP and stdio - but HTTP is only there for legacy support to retain compatibility with omnisharp-server. VSCode uses stdio/json (for the omnisharp stuff). Performance wise, there's no noticeable difference.

@nosami
Copy link

nosami commented Jan 15, 2016

Another point - any performance drops from using an external process are likely to be insignificant compared to the CPU time required by the compiler.

You should enable the server to support text changes (textrange * oldtext * newtext) for input rather than sending the entire buffer across and also for any code generation output.

@7sharp9
Copy link

7sharp9 commented Jan 15, 2016

FCS often requires full text input rather than a diff, a diff could be calculated on the server though prior to feeding FCS.

@nosami
Copy link

nosami commented Jan 15, 2016

Yeah. Roslyn accepts full text or a list of text changes. The purpose being to reduce transportation and serialisation costs.

OmniSharp uses something similar to the V8 debugger protocol. Each request gets an incrementing ID from the editor and the server echoes the ID on the response so that the editor can dispatch a callback.

It might be an idea to support the fsautocomplete protocol too for use with other editors and let them adapt to a new protocol in their own time. Anything other than json here could be problematic.

Also, I think this is something that me and @7sharp9 could get behind to use in Xamarin Studio.

@nosami
Copy link

nosami commented Jan 15, 2016

Stdio is extremely easy to implement for both the server and the clients (which could be written in any language). Providing that none of the requests are blocking, there shouldn't be any issues.

The OmniSharp request handler is more complex than it needs to be due to DNX weirdness.

@cloudRoutine
Copy link
Contributor Author

We now have a feature branch dedicated to this endeavor
https://github.com/fsprojects/VisualFSharpPowerTools/tree/out-of-process-core

@Krzysztof-Cieslak
Copy link

Definitely 👍.

For the HTTP - I'm using it for both Atom and VS Code right now. Suave is bit slow (But I haven't experienced as bad performance as Dave is suggesting - maybe that's mono problem? Can You link me benchmark You were running, @7sharp9? ). Probably JSON serialization have bigger impact.

Lack of pushing from server is not a problem ( what may be connected with limited set of features we have in FSAC)

Also vim, emacs and sublime (?) are using old FSAC protocol and stdio. @rneatherway can tell us more about it.

While it would be great to have super fast communication layer, I still think performance gains would be small - our main performance problem is compiler itself. So I would more focus on making it usable in easy way from all interested clients.

@vasily-kirichenko
Copy link
Contributor

So I would more focus on making it usable in easy way from all interested clients.

I disagree. Any comm layer which impacts overall performance more that say 5% should not be used (in VFPT I mean. If other clients can live with it, plug whatever they want).

@rneatherway
Copy link

Lack of pushing from server is not a problem ( what may be connected with limited set of features we have in FSAC)

Not sure exactly what you mean here, because FSAC currently returns the results of some requests asynchronously, such as errors and colorization information.

@Krzysztof-Cieslak
Copy link

Suave version doesn't support it

@rneatherway
Copy link

OK, understood. I thought you were talking about the whole app.
On 16 Jan 2016 3:43 pm, "Krzysztof Cieślak" [email protected]
wrote:

Suave version doesn't support it


Reply to this email directly or view it on GitHub
#1318 (comment)
.

@cloudRoutine
Copy link
Contributor Author

cloudRoutine commented Jul 15, 2016

The first stage of accomplishing this goal is underway and being tracked in VFPT.Core Refactoring #1406

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

9 participants