[#412] cache enablement for URI

[ghentschke]

part of #412

[ruspl-afed]

LGTM
So, only restart will clear the cache and this is desired, correct?

[ghentschke]

LGTM So, only restart will clear the cache and this is desired, correct?

Yes. But I would like to add a clear mechanism which would be triggered when the preferLspEditor has been modifed. I'll wait until your PR #410 has been merged.

[ruspl-afed]

I'll wait until your PR #410 has been merged.

Please fill free to merge both when needed.

[jonahgraham]

I can see that the PropertyTester's test method is called a lot so speeding that up has great value. Here are some comments.

• The title of this PR and commit need cash -> cache
• PropertyTester must be stateless, see javadoc - the new cache is kindof stateful, or rather you may end up with multiple caches the way it is done now.
• The cache is accessed from multiple threads, so some consideration to concurrency is needed.
• Should this be doing LRU style so that items don't stay and this grows forever. CDT has one org.eclipse.cdt.internal.core.LRUCache<K, V>
  • As it happens @ramele1907 is looking at some concurrency + LRU cache issues in Synchronize the access to LRUCache cdt#978
  • That code in CDT deals with a similar problem you have in LSP4E which is URI -> IReource caches. See eclipse-cdt/cdt@9e7b5be for when that was introduced
• I think the discussions in [#1207] cache IResource in EnablementTester eclipse-lsp4e/lsp4e#1208 are relevant to this PR too so I am cross referencing them here
• Cache invalidation may be needed, see longer comment below

Cache invalidation

My concern with this change is rather a corner case, but that there should be logic to clear the cache as some of the tests can return different results. For example you are checking content types and caching that result. In that case you need to add a listener org.eclipse.core.runtime.content.IContentTypeManager.addContentTypeChangeListener(IContentTypeChangeListener) so that you find out about content type changes.

A simple manual test:

1. Create a file called "file.jonah"
2. Double-click to open, will not be opened in CDT LSP because it isn't a C/C++ file
3. Add jonah as a C file extension in preferences:
   
4. Make sure the file now opens as an LSP editor

Doing a addContentTypeChangeListener will be sufficient to invalidate cache for content type changes, similar mechanism may be needed for other cases.

I mentioned LRU above, but time based caching may also be useful.

[ghentschke]

Thank you @jonahgraham for the good input I'll consider it!

[ghentschke]

Ok, this is more a reminder to me:
The cache shall be cleared if:

• the Prefer LSP Editor setting has been changed (workspace or project level) or
• on changes in the c/c++ content types for which we enable LSP4E

A resource shall be removed form the cache if it's getting closed in the editor.

The cache shall be implemented as FIFO buffer with n entries which considers concurrency and stateless aspects