Community
Participate
Working Groups
I'm currently seeing the following behavior: 1) my codemining is called three times in different threads for the same document change 2) each call originates from CodeMiningReconciler as multiple ones are installed on the viewer 3) even worse this leads to situations where my codeminings are partly not show, because these three instances of CodeMiningReconciler compete for the same CodeMiningManager.run(), that run cancels the monitor and that leads to the situation where after the computation the codeminings are thrown away, only those survive that are fast enough before one of the other threads cancels them out 4) Codeminingprovers are asked for codeminings regardless of the content type (e.g. AnnotationCodeMiningProvider is called for my custom content type) 5) instead of using CompletableFuture.cancel(boolean) it uses a monitor for cancellation / check for cancel but only at the end of the computation so even if this cancelation would be useful the computation is performed until the end to throw away the result. Anyone knows why this approach was chosen? Are there good (concurrent) test for this so it is safe to try improve in that area?
For 1) I found out that there re the following extensions: - one from genericeditor itself (for content type org.eclipse.core.runtime.text) - one from org.eclipse.lsp4e (for content type org.eclipse.core.runtime.text) - one for my content type in ExtensionBasedTextViewerConfiguration.getReconciler(ISourceViewer) these are collected and combined into a CompositeReconciler. org.eclipse.core.runtime.text seems to be the default content-type for the generic text-editor and thus a total of three consilers are registered. The obvious solution seems to only use the CodeMiningReconciler of the most specific content type but that would require special CodeMiningReconciler specific handling. Another option would probably be to only allow the most specific Reconciler of a given class-type. As we are using extension points it seems reasonable to assume that each instance is "the same" as its always default constructed. Is this a valid assumption?
It seems I misinterpreted missing minings to be consequence of the cancel handling. That seems not to be true but Bug 570740 is the cause for this, so I lower the importance and only focus on the first issue 'Codeminings computed multiple times'.
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.text/+/175492
(In reply to Christoph Laeubrich from comment #1) > The obvious solution seems to only use the CodeMiningReconciler of the most > specific content type but that would require special CodeMiningReconciler > specific handling. > > Another option would probably be to only allow the most specific Reconciler > of a given class-type. As we are using extension points it seems reasonable > to assume that each instance is "the same" as its always default constructed. > > Is this a valid assumption? Not really, all code mining providers should participate; the overall approach of generic editor extensions is that they are "cumulative" as much as possible. I think we instead need to implement deeper in CodeMiningManager some caching so we don't invoke the same code mining participant multiple times if no change has happened on the document, but we'd still need a way for external services (eg a debugger) to trigger a refresh of code minings explicitly.
(In reply to Mickael Istria from comment #4) > Not really, all code mining providers should participate; Please note that the problem is that the same CodeMining*Reconciler* [1] is added/executed, each of them calls *all* CodeMining*Providers* [2]. [1] org.eclipse.jface.text.codemining.CodeMiningReconciler [2] org.eclipse.jface.text.codemining.ICodeMiningProvider
and all of the call the same CodeMining*Manager* that's why the computation of the last call cancels out all previous computions (as if I have made three text changes not one.
> it seems reasonable > to assume that each instance is "the same" as its always default constructed I partly agree and partly disagree... Relying only on classname is a very involved assumption. I'm wondering whether it would be safer to instantiate the reconcilers anyway and only keep `.distinct()` ones. Then it would be a matter of implementing `.equals()` in the (CodeMining)reconciliers. (In reply to Christoph Laeubrich from comment #5) > Please note that the problem is that the same CodeMining*Reconciler* [1] is > added/executed, each of them calls *all* CodeMining*Providers* [2]. Right, but we still want that if some user specify mulitple reconcilers, they're all placed onto the editor unless there is a good reason not to. As mentioned above, I'm a bit uncertain the same type is a good enough reason, but a .equals() can be.
(In reply to Mickael Istria from comment #7) > I'm wondering whether it would be safer to instantiate the reconcilers > anyway and only keep `.distinct()` ones. Then it would be a matter of > implementing `.equals()` in the (CodeMining)reconciliers. good idea! I'll try that out.
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.text/+/175492 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.text.git/commit/?id=013925a7e22618bef7ed8e968ee4a935d8a61d68
Thanks Christian!
