`doc-get-definition-by-id` can be improved

[dannypsnl]

doc-get-definition-by-id is prepared for jump to definition in the other file, when we give xxx.rkt to doc-get-definition-by-id which means we cannot find a definition in the current file, instead, we find it's located at xxx.rkt so we need to jump to there.

In doc-get-definition-by-id we create an isolated syntax check (super expensive operation), consider the followings scenario this is a waste

1. We had open xxx.rkt and never modify it, then lookup Doc-trace of xxx.rkt can complete this task without a new syntax check
2. If we never open xxx.rkt, but then because we will open xxx.rkt, we will do syntax check again rather than reuse the valid syntax check we just did

[6cdh]

Yeah, but this would require doclib layer uses data in lsp layer. Maybe just cache result of expand in expand-source? And limit the size of cache.

[dannypsnl]

I think there are two options: lift this function up to higher level; give it a callback to access higher level.

[dannypsnl]

Compute sha1 for each file as address <sha1>, store expand result as .racket-lsp/cache/<sha1>/expand.rktd.

When file are not changed, we can reuse .racket-lsp/cache/<sha1>/expand.rktd.

[6cdh]

So we finally got 4 options:

Option A: Resolver Callback

Change doc-definition API in doc.rkt, to let it accepts an optional resolver callback for cross file definitions. That callback resolver receives a path and a id. LSP layer implement the resolver by checking whether the path is opened, and reading its doc-trace data.

Option B: Lift Logic To LSP layer

Change doc-definition to let it returns path and id. Then lsp layer handles cross file resolution, because it can access opened docs.

Option C: In Memory Expansion Cache

Cache results from expand-source. doc-definition can reuse data without knowing about current opened docs.

Option D: Disk Cache

Hash file content and store something under .racket-lsp/cache/<sha1>/expand.rktd.

Compare

Option A and B are similar, they both need to put logic at LSP layer. Option A is a bit easier and less damage than Option B. Option B is more cleaner.

Option C and D are similar, they are both cache, the difference is whether they are persistent. They are useful, but the cache invalidation strategy or stale cache can be problems. And they do not solve this specific problem elegantly, see below.

After some thinking, I think the root reason is that doclib is a single document abstraction. The cross file definition resolution requires it to know other documents, especially to know other opened documents for removing repeated expansion. So what we need is another abstraction for multiple documents, and put logic there. Right now, LSP layer is the place. A problem is that this abstraction is not clear and explicit now.

So option A and B look better, A is a fast fix, B is a clean long term fix.

What do you think?

[dannypsnl]

I prefer option B.