Clangd + XPC

Hi all,

Based on our discussion with Sam in https://reviews.llvm.org/D48559 we talked about our requirements internally and ultimately decided to change our approach.

We are going to use serialized JSON LSP over XPC instead of custom transport layer and we aren’t going to indroduce XPC-specific code to clangd binary itself which has several advantages.

I put our current design for review to continue the discussion.
https://reviews.llvm.org/D50452

I would like to thank you all for the great feedback you gave us! It helped us to precisely pinpoint our requirements.

Thanks.

Jan

Hi Jan,

Thanks for the update! It’s great that we’re after a solution that works at the protocol boundary, that does sound like something that should be easier to maintain and not too complicated.
I have skimmed through the patch, and as far as I can tell the idea is to forward raw JSON messages back and forth via XPC without actually converting from JSON to XPC and back.

Am I reading it correctly?

Hi Jan,

Thanks for the update! It’s great that we’re after a solution that works at the protocol boundary, that does sound like something that should be easier to maintain and not too complicated.
I have skimmed through the patch, and as far as I can tell the idea is to forward raw JSON messages back and forth via XPC without actually converting from JSON to XPC and back.

Am I reading it correctly?

Hi Ilya,

Yes that’s correct - we did some performance measurements and it turned out that for JSON containing a lot of sub-objects (think 40k CompletionItem[]) the cost of conversion to XPC dictionary is actually higher than what we gain at IPC. Just sending string over XPC is faster.
Since we might possibly need to use ranking and filtering logic on client side this use-case is interesting for us.

Cheers,

Jan

Thanks for the details.
The “json-inside-xpc” looks a bit hacky on first notice, but the adapter is actually trying to solve a different problem of dispatching requests to the correct clangd instance based on the workspace.
In that case, it does make sense to avoid losing time converting between the protocols.

One thing that might turn out to be problematic with the ‘clangd-per-workspace’ approach is that clangd manages some resource (ASTs, worker threads) and, obviously, can only do that inside a single process, so there are possibilities of resource starvation if too many clangd instances are running at the same time and don’t know about each other.

Thanks for the details.
The “json-inside-xpc” looks a bit hacky on first notice, but the adapter is actually trying to solve a different problem of dispatching requests to the correct clangd instance based on the workspace.
In that case, it does make sense to avoid losing time converting between the protocols.

Right, the other motivation is that XPC is a prescribed form of IPC for client-service on macOS and actually provides more user-friendly API than POSIX. So we need to use it but there’s no specific requirement how to use it and actually this form of optimization is also used by some other services.
Anyway, we could consider this just an implementation detail and add a minimal JSON string → xpc_dictionary conversion for LSP as a library for clangd XPC consumers to use. Then we could label the XPC representation as a stable API. Not sure if that’s necessary though. What do you think?

One thing that might turn out to be problematic with the ‘clangd-per-workspace’ approach is that clangd manages some resource (ASTs, worker threads) and, obviously, can only do that inside a single process, so there are possibilities of resource starvation if too many clangd instances are running at the same time and don’t know about each other.

We were also discussing that and our main concern were possibly duplicate ASTs in different clangd processes’ memory. Since our supported use-case is "just a few workspaces” (think 1-3 with let’s say 5 being already an extreme case) we concluded that for the time being it’s fine. In general we’d just use some reasonable limits for resources (e. g. -j 10 or whatever value we find optimal for worker threads). For general memory consumption we’ll need to implement some strategy anyway since in our case clangd would be a long-lived process running on developer’s machine. We were thinking about keeping N least recently used ASTs in memory and dropping the rest.

We have actually already implemented it in clangd: we store up to 3 last used ASTs and drop the rest at the moment.

It’s possible to make this configurable, if necessary.

In general, I guess only one clangd instance will be actively processing requests (i.e. dealing with user Interactions) at any given time, while others will only be handling background tasks.

It means we’re should probably be fine resource-wise.

Another thing about the xpc adapter is that it’s Mac-only, right?
Therefore, we can potentially break you without knowing about it. Even though that should be rare, given the design, what’s our plan there? Having a buildbot running Mac? Relying on you and the other Mac users nagging us when it’s broken?